diff mbox

[2/6] libceph: fix authorizer invalidation

Message ID 1363734486-26879-2-git-send-email-sage@inktank.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sage Weil March 19, 2013, 11:08 p.m. UTC
We were invalidating the authorizer by removing the ticket handler
entirely.  This was effective in inducing us to request a new authorizer,
but in the meantime it mean that any authorizer we generated would get a
new and initialized handler with secret_id=0, which would always be
rejected by the server side with a confusing error message:

 auth: could not find secret_id=0
 cephx: verify_authorizer could not get service secret for service osd secret_id=0

Instead, simply clear the validity field.  This will still induce the auth
code to request a new secret, but will let us continue to use the old
ticket in the meantime.  The messenger code will probably continue to fail,
but the exponential backoff will kick in, and eventually the we will get a
new (hopefully more valid) ticket from the mon and be able to continue.

Signed-off-by: Sage Weil <sage@inktank.com>
---
 net/ceph/auth_x.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Alex Elder March 25, 2013, 1:39 p.m. UTC | #1
On 03/19/2013 06:08 PM, Sage Weil wrote:
> We were invalidating the authorizer by removing the ticket handler
> entirely.  This was effective in inducing us to request a new authorizer,
> but in the meantime it mean that any authorizer we generated would get a
> new and initialized handler with secret_id=0, which would always be
> rejected by the server side with a confusing error message:
> 
>  auth: could not find secret_id=0
>  cephx: verify_authorizer could not get service secret for service osd secret_id=0
> 
> Instead, simply clear the validity field.  This will still induce the auth
> code to request a new secret, but will let us continue to use the old
> ticket in the meantime.  The messenger code will probably continue to fail,
> but the exponential backoff will kick in, and eventually the we will get a
> new (hopefully more valid) ticket from the mon and be able to continue.

This does seem like a smaller hammer way of invalidating
the authorizer, namely making its validity (time after
which it is no longer valid) be a time in the past.

I am not well versed in the bigger picture of this
mechanism, but this change looks good to me.

Reviewed-by: Alex Elder <elder@inktank.com>

> Signed-off-by: Sage Weil <sage@inktank.com>
> ---
>  net/ceph/auth_x.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c
> index a16bf14..bd8758d 100644
> --- a/net/ceph/auth_x.c
> +++ b/net/ceph/auth_x.c
> @@ -630,7 +630,7 @@ static void ceph_x_invalidate_authorizer(struct ceph_auth_client *ac,
>  
>  	th = get_ticket_handler(ac, peer_type);
>  	if (!IS_ERR(th))
> -		remove_ticket_handler(ac, th);
> +		memset(&th->validity, 0, sizeof(th->validity));
>  }
>  
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sage Weil March 25, 2013, 3:51 p.m. UTC | #2
On Mon, 25 Mar 2013, Alex Elder wrote:
> On 03/19/2013 06:08 PM, Sage Weil wrote:
> > We were invalidating the authorizer by removing the ticket handler
> > entirely.  This was effective in inducing us to request a new authorizer,
> > but in the meantime it mean that any authorizer we generated would get a
> > new and initialized handler with secret_id=0, which would always be
> > rejected by the server side with a confusing error message:
> > 
> >  auth: could not find secret_id=0
> >  cephx: verify_authorizer could not get service secret for service osd secret_id=0
> > 
> > Instead, simply clear the validity field.  This will still induce the auth
> > code to request a new secret, but will let us continue to use the old
> > ticket in the meantime.  The messenger code will probably continue to fail,
> > but the exponential backoff will kick in, and eventually the we will get a
> > new (hopefully more valid) ticket from the mon and be able to continue.
> 
> This does seem like a smaller hammer way of invalidating
> the authorizer, namely making its validity (time after
> which it is no longer valid) be a time in the past.

Yeah.  It seemed simpler than adding a new field that we have to store, 
initalize, and check, though, and it doesn't wrap so no real danger.

Thanks-

> I am not well versed in the bigger picture of this
> mechanism, but this change looks good to me.
> 
> Reviewed-by: Alex Elder <elder@inktank.com>
> 
> > Signed-off-by: Sage Weil <sage@inktank.com>
> > ---
> >  net/ceph/auth_x.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c
> > index a16bf14..bd8758d 100644
> > --- a/net/ceph/auth_x.c
> > +++ b/net/ceph/auth_x.c
> > @@ -630,7 +630,7 @@ static void ceph_x_invalidate_authorizer(struct ceph_auth_client *ac,
> >  
> >  	th = get_ticket_handler(ac, peer_type);
> >  	if (!IS_ERR(th))
> > -		remove_ticket_handler(ac, th);
> > +		memset(&th->validity, 0, sizeof(th->validity));
> >  }
> >  
> >  
> > 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/ceph/auth_x.c b/net/ceph/auth_x.c
index a16bf14..bd8758d 100644
--- a/net/ceph/auth_x.c
+++ b/net/ceph/auth_x.c
@@ -630,7 +630,7 @@  static void ceph_x_invalidate_authorizer(struct ceph_auth_client *ac,
 
 	th = get_ticket_handler(ac, peer_type);
 	if (!IS_ERR(th))
-		remove_ticket_handler(ac, th);
+		memset(&th->validity, 0, sizeof(th->validity));
 }