| Message ID | 20210623151352.18840-1-idryomov@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | libceph: set global_id as soon as we get an auth ticket | expand |
On Wed, 2021-06-23 at 17:13 +0200, Ilya Dryomov wrote: > Commit 61ca49a9105f ("libceph: don't set global_id until we get an > auth ticket") delayed the setting of global_id too much. It is set > only after all tickets are received, but in pre-nautilus clusters an > auth ticket and the service tickets are obtained in separate steps > (for a total of three MAuth replies). When the service tickets are > requested, global_id is used to build an authorizer; if global_id is > still 0 we never get them and fail to establish the session. > > Moving the setting of global_id into protocol implementations. This > way global_id can be set exactly when an auth ticket is received, not > sooner nor later. > > Fixes: 61ca49a9105f ("libceph: don't set global_id until we get an auth ticket") > Signed-off-by: Ilya Dryomov <idryomov@gmail.com> > --- > include/linux/ceph/auth.h | 4 +++- > net/ceph/auth.c | 13 +++++-------- > net/ceph/auth_none.c | 3 ++- > net/ceph/auth_x.c | 11 ++++++----- > 4 files changed, 16 insertions(+), 15 deletions(-) > > diff --git a/include/linux/ceph/auth.h b/include/linux/ceph/auth.h > index 39425e2f7cb2..6b138fa97db8 100644 > --- a/include/linux/ceph/auth.h > +++ b/include/linux/ceph/auth.h > @@ -50,7 +50,7 @@ struct ceph_auth_client_ops { > * another request. > */ > int (*build_request)(struct ceph_auth_client *ac, void *buf, void *end); > - int (*handle_reply)(struct ceph_auth_client *ac, > + int (*handle_reply)(struct ceph_auth_client *ac, u64 global_id, > void *buf, void *end, u8 *session_key, > int *session_key_len, u8 *con_secret, > int *con_secret_len); > @@ -104,6 +104,8 @@ struct ceph_auth_client { > struct mutex mutex; > }; > > +void ceph_auth_set_global_id(struct ceph_auth_client *ac, u64 global_id); > + > struct ceph_auth_client *ceph_auth_init(const char *name, > const struct ceph_crypto_key *key, > const int *con_modes); > diff --git a/net/ceph/auth.c b/net/ceph/auth.c > index d07c8cd6cb46..d38c9eadbe2f 100644 > --- a/net/ceph/auth.c > +++ b/net/ceph/auth.c > @@ -36,7 +36,7 @@ static int init_protocol(struct ceph_auth_client *ac, int proto) > } > } > > -static void set_global_id(struct ceph_auth_client *ac, u64 global_id) > +void ceph_auth_set_global_id(struct ceph_auth_client *ac, u64 global_id) > { > dout("%s global_id %llu\n", __func__, global_id); > > @@ -262,7 +262,7 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac, > goto out; > } > > - ret = ac->ops->handle_reply(ac, payload, payload_end, > + ret = ac->ops->handle_reply(ac, global_id, payload, payload_end, > NULL, NULL, NULL, NULL); > if (ret == -EAGAIN) { > ret = build_request(ac, true, reply_buf, reply_len); > @@ -271,8 +271,6 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac, > goto out; > } > > - set_global_id(ac, global_id); > - > out: > mutex_unlock(&ac->mutex); > return ret; > @@ -480,7 +478,7 @@ int ceph_auth_handle_reply_more(struct ceph_auth_client *ac, void *reply, > int ret; > > mutex_lock(&ac->mutex); > - ret = ac->ops->handle_reply(ac, reply, reply + reply_len, > + ret = ac->ops->handle_reply(ac, 0, reply, reply + reply_len, > NULL, NULL, NULL, NULL); Won't this trigger a KERN_ERR message? The the handle_reply routines call ceph_auth_set_global_id unconditionally, which will fire off the pr_err message and not do anything in this case. > if (ret == -EAGAIN) > ret = build_request(ac, false, buf, buf_len); > @@ -498,11 +496,10 @@ int ceph_auth_handle_reply_done(struct ceph_auth_client *ac, > int ret; > > mutex_lock(&ac->mutex); > - ret = ac->ops->handle_reply(ac, reply, reply + reply_len, > + ret = ac->ops->handle_reply(ac, global_id, reply, reply + reply_len, > session_key, session_key_len, > con_secret, con_secret_len); > - if (!ret) > - set_global_id(ac, global_id); > + WARN_ON(ret == -EAGAIN || ret > 0); > mutex_unlock(&ac->mutex); > return ret; > } > diff --git a/net/ceph/auth_none.c b/net/ceph/auth_none.c > index 533a2d85dbb9..77b5519bc45f 100644 > --- a/net/ceph/auth_none.c > +++ b/net/ceph/auth_none.c > @@ -69,7 +69,7 @@ static int build_request(struct ceph_auth_client *ac, void *buf, void *end) > * the generic auth code decode the global_id, and we carry no actual > * authenticate state, so nothing happens here. > */ > -static int handle_reply(struct ceph_auth_client *ac, > +static int handle_reply(struct ceph_auth_client *ac, u64 global_id, > void *buf, void *end, u8 *session_key, > int *session_key_len, u8 *con_secret, > int *con_secret_len) > @@ -77,6 +77,7 @@ static int handle_reply(struct ceph_auth_client *ac, > struct ceph_auth_none_info *xi = ac->private; > > xi->starting = false; > + ceph_auth_set_global_id(ac, global_id); > return 0; > } > > diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c > index cab99c5581b0..b71b1635916e 100644 > --- a/net/ceph/auth_x.c > +++ b/net/ceph/auth_x.c > @@ -597,7 +597,7 @@ static int decode_con_secret(void **p, void *end, u8 *con_secret, > return -EINVAL; > } > > -static int handle_auth_session_key(struct ceph_auth_client *ac, > +static int handle_auth_session_key(struct ceph_auth_client *ac, u64 global_id, > void **p, void *end, > u8 *session_key, int *session_key_len, > u8 *con_secret, int *con_secret_len) > @@ -613,6 +613,7 @@ static int handle_auth_session_key(struct ceph_auth_client *ac, > if (ret) > return ret; > > + ceph_auth_set_global_id(ac, global_id); > if (*p == end) { > /* pre-nautilus (or didn't request service tickets!) */ > WARN_ON(session_key || con_secret); > @@ -661,7 +662,7 @@ static int handle_auth_session_key(struct ceph_auth_client *ac, > return -EINVAL; > } > > -static int ceph_x_handle_reply(struct ceph_auth_client *ac, > +static int ceph_x_handle_reply(struct ceph_auth_client *ac, u64 global_id, > void *buf, void *end, > u8 *session_key, int *session_key_len, > u8 *con_secret, int *con_secret_len) > @@ -695,9 +696,9 @@ static int ceph_x_handle_reply(struct ceph_auth_client *ac, > switch (op) { > case CEPHX_GET_AUTH_SESSION_KEY: > /* AUTH ticket + [connection secret] + service tickets */ > - ret = handle_auth_session_key(ac, &p, end, session_key, > - session_key_len, con_secret, > - con_secret_len); > + ret = handle_auth_session_key(ac, global_id, &p, end, > + session_key, session_key_len, > + con_secret, con_secret_len); > break; > > case CEPHX_GET_PRINCIPAL_SESSION_KEY: -- Jeff Layton <jlayton@kernel.org>
On Thu, Jun 24, 2021 at 6:57 PM Jeff Layton <jlayton@kernel.org> wrote: > > On Wed, 2021-06-23 at 17:13 +0200, Ilya Dryomov wrote: > > Commit 61ca49a9105f ("libceph: don't set global_id until we get an > > auth ticket") delayed the setting of global_id too much. It is set > > only after all tickets are received, but in pre-nautilus clusters an > > auth ticket and the service tickets are obtained in separate steps > > (for a total of three MAuth replies). When the service tickets are > > requested, global_id is used to build an authorizer; if global_id is > > still 0 we never get them and fail to establish the session. > > > > Moving the setting of global_id into protocol implementations. This > > way global_id can be set exactly when an auth ticket is received, not > > sooner nor later. > > > > Fixes: 61ca49a9105f ("libceph: don't set global_id until we get an auth ticket") > > Signed-off-by: Ilya Dryomov <idryomov@gmail.com> > > --- > > include/linux/ceph/auth.h | 4 +++- > > net/ceph/auth.c | 13 +++++-------- > > net/ceph/auth_none.c | 3 ++- > > net/ceph/auth_x.c | 11 ++++++----- > > 4 files changed, 16 insertions(+), 15 deletions(-) > > > > diff --git a/include/linux/ceph/auth.h b/include/linux/ceph/auth.h > > index 39425e2f7cb2..6b138fa97db8 100644 > > --- a/include/linux/ceph/auth.h > > +++ b/include/linux/ceph/auth.h > > @@ -50,7 +50,7 @@ struct ceph_auth_client_ops { > > * another request. > > */ > > int (*build_request)(struct ceph_auth_client *ac, void *buf, void *end); > > - int (*handle_reply)(struct ceph_auth_client *ac, > > + int (*handle_reply)(struct ceph_auth_client *ac, u64 global_id, > > void *buf, void *end, u8 *session_key, > > int *session_key_len, u8 *con_secret, > > int *con_secret_len); > > @@ -104,6 +104,8 @@ struct ceph_auth_client { > > struct mutex mutex; > > }; > > > > +void ceph_auth_set_global_id(struct ceph_auth_client *ac, u64 global_id); > > + > > struct ceph_auth_client *ceph_auth_init(const char *name, > > const struct ceph_crypto_key *key, > > const int *con_modes); > > diff --git a/net/ceph/auth.c b/net/ceph/auth.c > > index d07c8cd6cb46..d38c9eadbe2f 100644 > > --- a/net/ceph/auth.c > > +++ b/net/ceph/auth.c > > @@ -36,7 +36,7 @@ static int init_protocol(struct ceph_auth_client *ac, int proto) > > } > > } > > > > -static void set_global_id(struct ceph_auth_client *ac, u64 global_id) > > +void ceph_auth_set_global_id(struct ceph_auth_client *ac, u64 global_id) > > { > > dout("%s global_id %llu\n", __func__, global_id); > > > > @@ -262,7 +262,7 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac, > > goto out; > > } > > > > - ret = ac->ops->handle_reply(ac, payload, payload_end, > > + ret = ac->ops->handle_reply(ac, global_id, payload, payload_end, > > NULL, NULL, NULL, NULL); > > if (ret == -EAGAIN) { > > ret = build_request(ac, true, reply_buf, reply_len); > > @@ -271,8 +271,6 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac, > > goto out; > > } > > > > - set_global_id(ac, global_id); > > - > > out: > > mutex_unlock(&ac->mutex); > > return ret; > > @@ -480,7 +478,7 @@ int ceph_auth_handle_reply_more(struct ceph_auth_client *ac, void *reply, > > int ret; > > > > mutex_lock(&ac->mutex); > > - ret = ac->ops->handle_reply(ac, reply, reply + reply_len, > > + ret = ac->ops->handle_reply(ac, 0, reply, reply + reply_len, > > NULL, NULL, NULL, NULL); > > Won't this trigger a KERN_ERR message? The the handle_reply routines > call ceph_auth_set_global_id unconditionally, which will fire off the > pr_err message and not do anything in this case. No, it won't get to calling ceph_auth_set_global_id() in this case because we are handling the "more" frame. An auth ticket is shared in the "done" frame, anything before that can't end up in handle_auth_session_key(). Thanks, Ilya
On Thu, 2021-06-24 at 19:16 +0200, Ilya Dryomov wrote: > On Thu, Jun 24, 2021 at 6:57 PM Jeff Layton <jlayton@kernel.org> wrote: > > > > On Wed, 2021-06-23 at 17:13 +0200, Ilya Dryomov wrote: > > > Commit 61ca49a9105f ("libceph: don't set global_id until we get an > > > auth ticket") delayed the setting of global_id too much. It is set > > > only after all tickets are received, but in pre-nautilus clusters an > > > auth ticket and the service tickets are obtained in separate steps > > > (for a total of three MAuth replies). When the service tickets are > > > requested, global_id is used to build an authorizer; if global_id is > > > still 0 we never get them and fail to establish the session. > > > > > > Moving the setting of global_id into protocol implementations. This > > > way global_id can be set exactly when an auth ticket is received, not > > > sooner nor later. > > > > > > Fixes: 61ca49a9105f ("libceph: don't set global_id until we get an auth ticket") > > > Signed-off-by: Ilya Dryomov <idryomov@gmail.com> > > > --- > > > include/linux/ceph/auth.h | 4 +++- > > > net/ceph/auth.c | 13 +++++-------- > > > net/ceph/auth_none.c | 3 ++- > > > net/ceph/auth_x.c | 11 ++++++----- > > > 4 files changed, 16 insertions(+), 15 deletions(-) > > > > > > diff --git a/include/linux/ceph/auth.h b/include/linux/ceph/auth.h > > > index 39425e2f7cb2..6b138fa97db8 100644 > > > --- a/include/linux/ceph/auth.h > > > +++ b/include/linux/ceph/auth.h > > > @@ -50,7 +50,7 @@ struct ceph_auth_client_ops { > > > * another request. > > > */ > > > int (*build_request)(struct ceph_auth_client *ac, void *buf, void *end); > > > - int (*handle_reply)(struct ceph_auth_client *ac, > > > + int (*handle_reply)(struct ceph_auth_client *ac, u64 global_id, > > > void *buf, void *end, u8 *session_key, > > > int *session_key_len, u8 *con_secret, > > > int *con_secret_len); > > > @@ -104,6 +104,8 @@ struct ceph_auth_client { > > > struct mutex mutex; > > > }; > > > > > > +void ceph_auth_set_global_id(struct ceph_auth_client *ac, u64 global_id); > > > + > > > struct ceph_auth_client *ceph_auth_init(const char *name, > > > const struct ceph_crypto_key *key, > > > const int *con_modes); > > > diff --git a/net/ceph/auth.c b/net/ceph/auth.c > > > index d07c8cd6cb46..d38c9eadbe2f 100644 > > > --- a/net/ceph/auth.c > > > +++ b/net/ceph/auth.c > > > @@ -36,7 +36,7 @@ static int init_protocol(struct ceph_auth_client *ac, int proto) > > > } > > > } > > > > > > -static void set_global_id(struct ceph_auth_client *ac, u64 global_id) > > > +void ceph_auth_set_global_id(struct ceph_auth_client *ac, u64 global_id) > > > { > > > dout("%s global_id %llu\n", __func__, global_id); > > > > > > @@ -262,7 +262,7 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac, > > > goto out; > > > } > > > > > > - ret = ac->ops->handle_reply(ac, payload, payload_end, > > > + ret = ac->ops->handle_reply(ac, global_id, payload, payload_end, > > > NULL, NULL, NULL, NULL); > > > if (ret == -EAGAIN) { > > > ret = build_request(ac, true, reply_buf, reply_len); > > > @@ -271,8 +271,6 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac, > > > goto out; > > > } > > > > > > - set_global_id(ac, global_id); > > > - > > > out: > > > mutex_unlock(&ac->mutex); > > > return ret; > > > @@ -480,7 +478,7 @@ int ceph_auth_handle_reply_more(struct ceph_auth_client *ac, void *reply, > > > int ret; > > > > > > mutex_lock(&ac->mutex); > > > - ret = ac->ops->handle_reply(ac, reply, reply + reply_len, > > > + ret = ac->ops->handle_reply(ac, 0, reply, reply + reply_len, > > > NULL, NULL, NULL, NULL); > > > > Won't this trigger a KERN_ERR message? The the handle_reply routines > > call ceph_auth_set_global_id unconditionally, which will fire off the > > pr_err message and not do anything in this case. > > No, it won't get to calling ceph_auth_set_global_id() in this case because we > are handling the "more" frame. An auth ticket is shared in the "done" frame, > anything before that can't end up in handle_auth_session_key(). > Ok, so ceph_x_handle_reply only calls that in the CEPHX_GET_AUTH_SESSION_KEY case... I suppose we can't get any of this in the auth_none case, correct? If so, then I think this looks good: Reviewed-by: Jeff Layton <jlayton@kernel.org>
On Thu, Jun 24, 2021 at 7:30 PM Jeff Layton <jlayton@kernel.org> wrote: > > On Thu, 2021-06-24 at 19:16 +0200, Ilya Dryomov wrote: > > On Thu, Jun 24, 2021 at 6:57 PM Jeff Layton <jlayton@kernel.org> wrote: > > > > > > On Wed, 2021-06-23 at 17:13 +0200, Ilya Dryomov wrote: > > > > Commit 61ca49a9105f ("libceph: don't set global_id until we get an > > > > auth ticket") delayed the setting of global_id too much. It is set > > > > only after all tickets are received, but in pre-nautilus clusters an > > > > auth ticket and the service tickets are obtained in separate steps > > > > (for a total of three MAuth replies). When the service tickets are > > > > requested, global_id is used to build an authorizer; if global_id is > > > > still 0 we never get them and fail to establish the session. > > > > > > > > Moving the setting of global_id into protocol implementations. This > > > > way global_id can be set exactly when an auth ticket is received, not > > > > sooner nor later. > > > > > > > > Fixes: 61ca49a9105f ("libceph: don't set global_id until we get an auth ticket") > > > > Signed-off-by: Ilya Dryomov <idryomov@gmail.com> > > > > --- > > > > include/linux/ceph/auth.h | 4 +++- > > > > net/ceph/auth.c | 13 +++++-------- > > > > net/ceph/auth_none.c | 3 ++- > > > > net/ceph/auth_x.c | 11 ++++++----- > > > > 4 files changed, 16 insertions(+), 15 deletions(-) > > > > > > > > diff --git a/include/linux/ceph/auth.h b/include/linux/ceph/auth.h > > > > index 39425e2f7cb2..6b138fa97db8 100644 > > > > --- a/include/linux/ceph/auth.h > > > > +++ b/include/linux/ceph/auth.h > > > > @@ -50,7 +50,7 @@ struct ceph_auth_client_ops { > > > > * another request. > > > > */ > > > > int (*build_request)(struct ceph_auth_client *ac, void *buf, void *end); > > > > - int (*handle_reply)(struct ceph_auth_client *ac, > > > > + int (*handle_reply)(struct ceph_auth_client *ac, u64 global_id, > > > > void *buf, void *end, u8 *session_key, > > > > int *session_key_len, u8 *con_secret, > > > > int *con_secret_len); > > > > @@ -104,6 +104,8 @@ struct ceph_auth_client { > > > > struct mutex mutex; > > > > }; > > > > > > > > +void ceph_auth_set_global_id(struct ceph_auth_client *ac, u64 global_id); > > > > + > > > > struct ceph_auth_client *ceph_auth_init(const char *name, > > > > const struct ceph_crypto_key *key, > > > > const int *con_modes); > > > > diff --git a/net/ceph/auth.c b/net/ceph/auth.c > > > > index d07c8cd6cb46..d38c9eadbe2f 100644 > > > > --- a/net/ceph/auth.c > > > > +++ b/net/ceph/auth.c > > > > @@ -36,7 +36,7 @@ static int init_protocol(struct ceph_auth_client *ac, int proto) > > > > } > > > > } > > > > > > > > -static void set_global_id(struct ceph_auth_client *ac, u64 global_id) > > > > +void ceph_auth_set_global_id(struct ceph_auth_client *ac, u64 global_id) > > > > { > > > > dout("%s global_id %llu\n", __func__, global_id); > > > > > > > > @@ -262,7 +262,7 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac, > > > > goto out; > > > > } > > > > > > > > - ret = ac->ops->handle_reply(ac, payload, payload_end, > > > > + ret = ac->ops->handle_reply(ac, global_id, payload, payload_end, > > > > NULL, NULL, NULL, NULL); > > > > if (ret == -EAGAIN) { > > > > ret = build_request(ac, true, reply_buf, reply_len); > > > > @@ -271,8 +271,6 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac, > > > > goto out; > > > > } > > > > > > > > - set_global_id(ac, global_id); > > > > - > > > > out: > > > > mutex_unlock(&ac->mutex); > > > > return ret; > > > > @@ -480,7 +478,7 @@ int ceph_auth_handle_reply_more(struct ceph_auth_client *ac, void *reply, > > > > int ret; > > > > > > > > mutex_lock(&ac->mutex); > > > > - ret = ac->ops->handle_reply(ac, reply, reply + reply_len, > > > > + ret = ac->ops->handle_reply(ac, 0, reply, reply + reply_len, > > > > NULL, NULL, NULL, NULL); > > > > > > Won't this trigger a KERN_ERR message? The the handle_reply routines > > > call ceph_auth_set_global_id unconditionally, which will fire off the > > > pr_err message and not do anything in this case. > > > > No, it won't get to calling ceph_auth_set_global_id() in this case because we > > are handling the "more" frame. An auth ticket is shared in the "done" frame, > > anything before that can't end up in handle_auth_session_key(). > > > > Ok, so ceph_x_handle_reply only calls that in the > CEPHX_GET_AUTH_SESSION_KEY case... > > I suppose we can't get any of this in the auth_none case, correct? If > so, then I think this looks good: Correct. In the auth_none case the "done" frame is sent right away. Thanks, Ilya
diff --git a/include/linux/ceph/auth.h b/include/linux/ceph/auth.h index 39425e2f7cb2..6b138fa97db8 100644 --- a/include/linux/ceph/auth.h +++ b/include/linux/ceph/auth.h @@ -50,7 +50,7 @@ struct ceph_auth_client_ops { * another request. */ int (*build_request)(struct ceph_auth_client *ac, void *buf, void *end); - int (*handle_reply)(struct ceph_auth_client *ac, + int (*handle_reply)(struct ceph_auth_client *ac, u64 global_id, void *buf, void *end, u8 *session_key, int *session_key_len, u8 *con_secret, int *con_secret_len); @@ -104,6 +104,8 @@ struct ceph_auth_client { struct mutex mutex; }; +void ceph_auth_set_global_id(struct ceph_auth_client *ac, u64 global_id); + struct ceph_auth_client *ceph_auth_init(const char *name, const struct ceph_crypto_key *key, const int *con_modes); diff --git a/net/ceph/auth.c b/net/ceph/auth.c index d07c8cd6cb46..d38c9eadbe2f 100644 --- a/net/ceph/auth.c +++ b/net/ceph/auth.c @@ -36,7 +36,7 @@ static int init_protocol(struct ceph_auth_client *ac, int proto) } } -static void set_global_id(struct ceph_auth_client *ac, u64 global_id) +void ceph_auth_set_global_id(struct ceph_auth_client *ac, u64 global_id) { dout("%s global_id %llu\n", __func__, global_id); @@ -262,7 +262,7 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac, goto out; } - ret = ac->ops->handle_reply(ac, payload, payload_end, + ret = ac->ops->handle_reply(ac, global_id, payload, payload_end, NULL, NULL, NULL, NULL); if (ret == -EAGAIN) { ret = build_request(ac, true, reply_buf, reply_len); @@ -271,8 +271,6 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac, goto out; } - set_global_id(ac, global_id); - out: mutex_unlock(&ac->mutex); return ret; @@ -480,7 +478,7 @@ int ceph_auth_handle_reply_more(struct ceph_auth_client *ac, void *reply, int ret; mutex_lock(&ac->mutex); - ret = ac->ops->handle_reply(ac, reply, reply + reply_len, + ret = ac->ops->handle_reply(ac, 0, reply, reply + reply_len, NULL, NULL, NULL, NULL); if (ret == -EAGAIN) ret = build_request(ac, false, buf, buf_len); @@ -498,11 +496,10 @@ int ceph_auth_handle_reply_done(struct ceph_auth_client *ac, int ret; mutex_lock(&ac->mutex); - ret = ac->ops->handle_reply(ac, reply, reply + reply_len, + ret = ac->ops->handle_reply(ac, global_id, reply, reply + reply_len, session_key, session_key_len, con_secret, con_secret_len); - if (!ret) - set_global_id(ac, global_id); + WARN_ON(ret == -EAGAIN || ret > 0); mutex_unlock(&ac->mutex); return ret; } diff --git a/net/ceph/auth_none.c b/net/ceph/auth_none.c index 533a2d85dbb9..77b5519bc45f 100644 --- a/net/ceph/auth_none.c +++ b/net/ceph/auth_none.c @@ -69,7 +69,7 @@ static int build_request(struct ceph_auth_client *ac, void *buf, void *end) * the generic auth code decode the global_id, and we carry no actual * authenticate state, so nothing happens here. */ -static int handle_reply(struct ceph_auth_client *ac, +static int handle_reply(struct ceph_auth_client *ac, u64 global_id, void *buf, void *end, u8 *session_key, int *session_key_len, u8 *con_secret, int *con_secret_len) @@ -77,6 +77,7 @@ static int handle_reply(struct ceph_auth_client *ac, struct ceph_auth_none_info *xi = ac->private; xi->starting = false; + ceph_auth_set_global_id(ac, global_id); return 0; } diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c index cab99c5581b0..b71b1635916e 100644 --- a/net/ceph/auth_x.c +++ b/net/ceph/auth_x.c @@ -597,7 +597,7 @@ static int decode_con_secret(void **p, void *end, u8 *con_secret, return -EINVAL; } -static int handle_auth_session_key(struct ceph_auth_client *ac, +static int handle_auth_session_key(struct ceph_auth_client *ac, u64 global_id, void **p, void *end, u8 *session_key, int *session_key_len, u8 *con_secret, int *con_secret_len) @@ -613,6 +613,7 @@ static int handle_auth_session_key(struct ceph_auth_client *ac, if (ret) return ret; + ceph_auth_set_global_id(ac, global_id); if (*p == end) { /* pre-nautilus (or didn't request service tickets!) */ WARN_ON(session_key || con_secret); @@ -661,7 +662,7 @@ static int handle_auth_session_key(struct ceph_auth_client *ac, return -EINVAL; } -static int ceph_x_handle_reply(struct ceph_auth_client *ac, +static int ceph_x_handle_reply(struct ceph_auth_client *ac, u64 global_id, void *buf, void *end, u8 *session_key, int *session_key_len, u8 *con_secret, int *con_secret_len) @@ -695,9 +696,9 @@ static int ceph_x_handle_reply(struct ceph_auth_client *ac, switch (op) { case CEPHX_GET_AUTH_SESSION_KEY: /* AUTH ticket + [connection secret] + service tickets */ - ret = handle_auth_session_key(ac, &p, end, session_key, - session_key_len, con_secret, - con_secret_len); + ret = handle_auth_session_key(ac, global_id, &p, end, + session_key, session_key_len, + con_secret, con_secret_len); break; case CEPHX_GET_PRINCIPAL_SESSION_KEY:
Commit 61ca49a9105f ("libceph: don't set global_id until we get an auth ticket") delayed the setting of global_id too much. It is set only after all tickets are received, but in pre-nautilus clusters an auth ticket and the service tickets are obtained in separate steps (for a total of three MAuth replies). When the service tickets are requested, global_id is used to build an authorizer; if global_id is still 0 we never get them and fail to establish the session. Moving the setting of global_id into protocol implementations. This way global_id can be set exactly when an auth ticket is received, not sooner nor later. Fixes: 61ca49a9105f ("libceph: don't set global_id until we get an auth ticket") Signed-off-by: Ilya Dryomov <idryomov@gmail.com> --- include/linux/ceph/auth.h | 4 +++- net/ceph/auth.c | 13 +++++-------- net/ceph/auth_none.c | 3 ++- net/ceph/auth_x.c | 11 ++++++----- 4 files changed, 16 insertions(+), 15 deletions(-)