GNOME Bugzilla – Bug 756495
Fix kerberos renewal feature
Last modified: 2016-04-11 15:47:38 UTC
Some KDCs allow renewal of the kerberos ticket granting ticket, just by asking for a renewal, without having to resubmit a password (within defined limits). the gnome-online-accounts kerberos code tries to support that, when available, but fails in most cases due to a couple of bugs in the code. These patches get things working.
Created attachment 313177 [details] [review] identity: don't ignore almost all renewal requests There's a bug in the code where we ignore all renewal requests for objects we already know about. This means we'll only ever honor renewal requests that happen almost immediately after start up. This commit fixes the bug, so the only renewal requests that aren't honored, are those that have been specifically disabled by the user. This was clearly the original intention of the buggy code.
Created attachment 313178 [details] [review] identity: consider ticket start time when calculating renewal time Right now we try to renew the kerberos ticket at the midpoint between when we first set up the renewal alarm and when the ticket expires. Unfortunately, this doesn't work for kernel keyring based kerberos tickets, since the alarms are reset on a regular interval (as part of the polling process to check for updates, since kernel keyring doesn't have changed notification). Since the alarms are reset regularly, the midpoint converges toward the end of the ticket lifetime. Instead, we should consider the ticket start time, which remains constant. This commit tracks the ticket start time, and changes the code renewal alarm to use the mid point between the start time and expiration time of the ticket.
Cool I wrote very similar patch yesterday and I wanted to file a bug report today :) Thanks for taking care of this. I see two points: 1) credentials->times.starttime = credentials->times.authtime or at least use authtime as failback: http://web.mit.edu/kerberos/krb5-current/doc/appldev/refs/types/krb5_ticket_times.html#krb5_ticket_times (this mentions starttime is optional) 2) you should take into account also renew_till from credentials->times
Created attachment 323890 [details] [review] identity: rework how renewal time is calculated Right now we try to renew the kerberos ticket at the midpoint between when we first set up the renewal alarm and when the ticket expires. Unfortunately, this doesn't work for kernel keyring based kerberos tickets, since the alarms are reset on a regular interval (as part of the polling process to check for updates, since kernel keyring doesn't have changed notification). Since the alarms are reset regularly, the midpoint converges toward the end of the ticket lifetime. Instead, we should consider the ticket start time, which remains constant. This commit tracks the ticket start time, and changes the code renewal alarm to use the mid point between the start time and renewal time of the ticket.
Comment on attachment 313177 [details] [review] identity: don't ignore almost all renewal requests (pushing the first patch now, since it's small, and non-controversial) Attachment 313177 [details] pushed as 09e1103 - identity: don't ignore almost all renewal requests
Thanks Luf, attachment 323890 [details] [review] incorporates your feedback. I still need to some testing of it.
Review of attachment 323890 [details] [review]: Submodule alert! This hunk is spurious: diff --git a/telepathy-account-widgets b/telepathy-account-widgets index eea6395..b838c54 160000 --- a/telepathy-account-widgets +++ b/telepathy-account-widgets @@ -1 +1 @@ -Subproject commit eea639500689a9d9cb52a22fd4788e735002acac +Subproject commit b838c54c673165a9d948b18f21580a0ca94f8cd5
eek, hate that.
Created attachment 324042 [details] [review] identity: rework how renewal time is calculated Right now we try to renew the kerberos ticket at the midpoint between when we first set up the renewal alarm and when the ticket expires. Unfortunately, this doesn't work for kernel keyring based kerberos tickets, since the alarms are reset on a regular interval (as part of the polling process to check for updates, since kernel keyring doesn't have changed notification). Since the alarms are reset regularly, the midpoint converges toward the end of the ticket lifetime. Instead, we should consider the ticket start time, which remains constant. This commit tracks the ticket start time, and changes the code renewal alarm to use the mid point between the start time and renewal time of the ticket.
Comment on attachment 324042 [details] [review] identity: rework how renewal time is calculated after doing some testing on this, i've discovered it needs some more work.
So the scoop is that renew_till doesn't help us figure out a good time to renew. The renew_till time will always, of course, be after the expiration date of the ticket, so the expiration time is all we need to look at to figure out a renewal time. Knowing if the renew_till time is 0 or not, though, will tell use whether we should even bother trying to renew, so it's useful for that (though I guess we could have looked at the flags on the ticket too). Will post an updated patch and push in the next day or too unless Luf or rishi want to give it a review.
Created attachment 325044 [details] [review] identity: don't reverify identity when doing update At the moment, the way we refresh an identity is by creating a new, fresh, transient identity, and then update the existing identity instance with the state of the transient instance. When the fresh identity is instantiated we validate its credentials, and then immediately afterward, when the identity state is transferred we revalidate the credentials, which is needless double work. This commit changes goa_kerberos_identity_update to stop validiating the credentials of the source identity, and instead just copy the state over wholesale.
Created attachment 325045 [details] [review] identity: don't reset alarm if time unchanged Right now we always reset the expiration alarm, any time there is an update. This commit changes the code to reset the alarm only if the update resulted in the expiration time changing.
Created attachment 325046 [details] [review] identity: rework how renewal time is calculated Right now we try to renew the kerberos ticket at the midpoint between when we first set up the renewal alarm and when the ticket expires. Unfortunately, this doesn't work for kernel keyring based kerberos tickets, since the alarms are reset on a regular interval (as part of the polling process to check for updates, since kernel keyring doesn't have changed notification). Since the alarms are reset regularly, the midpoint converges toward the end of the ticket lifetime. Instead, we should consider the ticket start time, which remains constant. This commit tracks the ticket start time, and changes the code renewal alarm to use the mid point between the start time and expiration time of the ticket. We also now only set a renewal alarm if there's a valid renewal lifetime.
Attachment 325044 [details] pushed as 1db6741 - identity: don't reverify identity when doing update Attachment 325045 [details] pushed as 770732c - identity: don't reset alarm if time unchanged Attachment 325046 [details] pushed as 684f588 - identity: rework how renewal time is calculated