ehlo,
Function krb5_cc_get_full_name is called only as a way to validate that, we have the right cache. Instead of returned name, location will be returned from function cc_dir_cache_for_princ.
https://fedorahosted.org/sssd/ticket/1936
Patch is attached.
LS
On (29/05/13 11:07), Lukas Slebodnik wrote:
ehlo,
Function krb5_cc_get_full_name is called only as a way to validate that, we have the right cache. Instead of returned name, location will be returned from function cc_dir_cache_for_princ.
https://fedorahosted.org/sssd/ticket/1936
Patch is attached.
LS
self NACK
this patch store to cache DIR:/run/user/325600000/krb5cc ^^^^ missing colon?
LS
On Wed, 2013-05-29 at 11:30 +0200, Lukas Slebodnik wrote:
On (29/05/13 11:07), Lukas Slebodnik wrote:
ehlo,
Function krb5_cc_get_full_name is called only as a way to validate that, we have the right cache. Instead of returned name, location will be returned from function cc_dir_cache_for_princ.
https://fedorahosted.org/sssd/ticket/1936
Patch is attached.
LS
self NACK
this patch store to cache DIR:/run/user/325600000/krb5cc ^^^^ missing colon?
No, this is the correct form.
Simo.
On (29/05/13 08:44), Simo Sorce wrote:
On Wed, 2013-05-29 at 11:30 +0200, Lukas Slebodnik wrote:
On (29/05/13 11:07), Lukas Slebodnik wrote:
ehlo,
Function krb5_cc_get_full_name is called only as a way to validate that, we have the right cache. Instead of returned name, location will be returned from function cc_dir_cache_for_princ.
https://fedorahosted.org/sssd/ticket/1936
Patch is attached.
LS
self NACK
this patch store to cache DIR:/run/user/325600000/krb5cc ^^^^ missing colon?
No, this is the correct form.
I found out, that it is a correct form. Problem was with checking ccname in function sss_krb5_cc_file_path.
New patches attached.
LS
On Wed, 2013-05-29 at 16:09 +0200, Lukas Slebodnik wrote:
On (29/05/13 08:44), Simo Sorce wrote:
On Wed, 2013-05-29 at 11:30 +0200, Lukas Slebodnik wrote:
On (29/05/13 11:07), Lukas Slebodnik wrote:
ehlo,
Function krb5_cc_get_full_name is called only as a way to validate that, we have the right cache. Instead of returned name, location will be returned from function cc_dir_cache_for_princ.
https://fedorahosted.org/sssd/ticket/1936
Patch is attached.
LS
self NACK
this patch store to cache DIR:/run/user/325600000/krb5cc ^^^^ missing colon?
No, this is the correct form.
I found out, that it is a correct form. Problem was with checking ccname in function sss_krb5_cc_file_path.
New patches attached.
LS
Sorry for the reply from gmail. My OTP token decided the best password for me on a Friday evening is "Err", so I can't access my redhat.com account at the moment.
These patches break one assumption we want to keep -- if there is a user logged in and the same user logs in for example from another terminal, they should have the same ccache. With your patches, I'm getting a new one when I log in simultaneously.
I haven't tested that, but I guess this is because path to collection is always passed to the krb5_child now. I think that in the case user is already logged in (in krb5 code we denote this with "ccache is active"), then you should pass the full path to the ccache to the krb5_child.
On (31/05/13 21:29), Jakub Hrozek wrote:
On Wed, 2013-05-29 at 16:09 +0200, Lukas Slebodnik wrote:
On (29/05/13 08:44), Simo Sorce wrote:
On Wed, 2013-05-29 at 11:30 +0200, Lukas Slebodnik wrote:
On (29/05/13 11:07), Lukas Slebodnik wrote:
ehlo,
Function krb5_cc_get_full_name is called only as a way to validate that, we have the right cache. Instead of returned name, location will be returned from function cc_dir_cache_for_princ.
https://fedorahosted.org/sssd/ticket/1936
Patch is attached.
LS
self NACK
this patch store to cache DIR:/run/user/325600000/krb5cc ^^^^ missing colon?
No, this is the correct form.
I found out, that it is a correct form. Problem was with checking ccname in function sss_krb5_cc_file_path.
New patches attached.
LS
Sorry for the reply from gmail. My OTP token decided the best password for me on a Friday evening is "Err", so I can't access my redhat.com account at the moment.
These patches break one assumption we want to keep -- if there is a user logged in and the same user logs in for example from another terminal, they should have the same ccache. With your patches, I'm getting a new one when I log in simultaneously.
I haven't tested that, but I guess this is because path to collection is always passed to the krb5_child now. I think that in the case user is already logged in (in krb5 code we denote this with "ccache is active"), then you should pass the full path to the ccache to the krb5_child.
Simo wrote in ticket comment (https://fedorahosted.org/sssd/ticket/1936#comment:10)
Do we really want to store only DIR:/run/user/$uid/krb5cc/ to cache?
Yes, this is exactly what we want as a ccache.
LS
On Sat, 2013-06-01 at 11:43 +0200, Lukas Slebodnik wrote:
On (31/05/13 21:29), Jakub Hrozek wrote:
On Wed, 2013-05-29 at 16:09 +0200, Lukas Slebodnik wrote:
On (29/05/13 08:44), Simo Sorce wrote:
On Wed, 2013-05-29 at 11:30 +0200, Lukas Slebodnik wrote:
On (29/05/13 11:07), Lukas Slebodnik wrote:
ehlo,
Function krb5_cc_get_full_name is called only as a way to validate that, we have the right cache. Instead of returned name, location will be returned from function cc_dir_cache_for_princ.
https://fedorahosted.org/sssd/ticket/1936
Patch is attached.
LS
self NACK
this patch store to cache DIR:/run/user/325600000/krb5cc ^^^^ missing colon?
No, this is the correct form.
I found out, that it is a correct form. Problem was with checking ccname in function sss_krb5_cc_file_path.
New patches attached.
LS
Sorry for the reply from gmail. My OTP token decided the best password for me on a Friday evening is "Err", so I can't access my redhat.com account at the moment.
These patches break one assumption we want to keep -- if there is a user logged in and the same user logs in for example from another terminal, they should have the same ccache. With your patches, I'm getting a new one when I log in simultaneously.
I haven't tested that, but I guess this is because path to collection is always passed to the krb5_child now. I think that in the case user is already logged in (in krb5 code we denote this with "ccache is active"), then you should pass the full path to the ccache to the krb5_child.
Simo wrote in ticket comment (https://fedorahosted.org/sssd/ticket/1936#comment:10)
Do we really want to store only DIR:/run/user/$uid/krb5cc/ to cache?
Yes, this is exactly what we want as a ccache.
LS
Yes, I saw that comment and I agree with Simo.
But I think we should examine the ccache collection and in the case there already is a valid cache present there, we should reuse it just like we did before.
On Sun, Jun 02, 2013 at 11:14:31PM +0200, Jakub Hrozek wrote:
On Sat, 2013-06-01 at 11:43 +0200, Lukas Slebodnik wrote:
On (31/05/13 21:29), Jakub Hrozek wrote:
On Wed, 2013-05-29 at 16:09 +0200, Lukas Slebodnik wrote:
On (29/05/13 08:44), Simo Sorce wrote:
On Wed, 2013-05-29 at 11:30 +0200, Lukas Slebodnik wrote:
On (29/05/13 11:07), Lukas Slebodnik wrote: >ehlo, > >Function krb5_cc_get_full_name is called only as a way to validate that, >we have the right cache. Instead of returned name, location will be returned >from function cc_dir_cache_for_princ. > >https://fedorahosted.org/sssd/ticket/1936 > >Patch is attached. > >LS
self NACK
this patch store to cache DIR:/run/user/325600000/krb5cc ^^^^ missing colon?
No, this is the correct form.
I found out, that it is a correct form. Problem was with checking ccname in function sss_krb5_cc_file_path.
New patches attached.
LS
Sorry for the reply from gmail. My OTP token decided the best password for me on a Friday evening is "Err", so I can't access my redhat.com account at the moment.
These patches break one assumption we want to keep -- if there is a user logged in and the same user logs in for example from another terminal, they should have the same ccache. With your patches, I'm getting a new one when I log in simultaneously.
I haven't tested that, but I guess this is because path to collection is always passed to the krb5_child now. I think that in the case user is already logged in (in krb5 code we denote this with "ccache is active"), then you should pass the full path to the ccache to the krb5_child.
Simo wrote in ticket comment (https://fedorahosted.org/sssd/ticket/1936#comment:10)
Do we really want to store only DIR:/run/user/$uid/krb5cc/ to cache?
Yes, this is exactly what we want as a ccache.
LS
Yes, I saw that comment and I agree with Simo.
But I think we should examine the ccache collection and in the case there already is a valid cache present there, we should reuse it just like we did before.
btw this actually sounds like something the krb5 libs should be doing on their own, I can't think of a situation where you'd want two ccaches for exactly the same principals in the same collection..
On (02/06/13 23:14), Jakub Hrozek wrote:
On Sat, 2013-06-01 at 11:43 +0200, Lukas Slebodnik wrote:
On (31/05/13 21:29), Jakub Hrozek wrote:
On Wed, 2013-05-29 at 16:09 +0200, Lukas Slebodnik wrote:
On (29/05/13 08:44), Simo Sorce wrote:
On Wed, 2013-05-29 at 11:30 +0200, Lukas Slebodnik wrote:
On (29/05/13 11:07), Lukas Slebodnik wrote: >ehlo, > >Function krb5_cc_get_full_name is called only as a way to validate that, >we have the right cache. Instead of returned name, location will be returned >from function cc_dir_cache_for_princ. > >https://fedorahosted.org/sssd/ticket/1936 > >Patch is attached. > >LS
self NACK
this patch store to cache DIR:/run/user/325600000/krb5cc ^^^^ missing colon?
No, this is the correct form.
I found out, that it is a correct form. Problem was with checking ccname in function sss_krb5_cc_file_path.
New patches attached.
LS
Sorry for the reply from gmail. My OTP token decided the best password for me on a Friday evening is "Err", so I can't access my redhat.com account at the moment.
These patches break one assumption we want to keep -- if there is a user logged in and the same user logs in for example from another terminal, they should have the same ccache. With your patches, I'm getting a new one when I log in simultaneously.
I haven't tested that, but I guess this is because path to collection is always passed to the krb5_child now. I think that in the case user is already logged in (in krb5 code we denote this with "ccache is active"), then you should pass the full path to the ccache to the krb5_child.
Simo wrote in ticket comment (https://fedorahosted.org/sssd/ticket/1936#comment:10)
Do we really want to store only DIR:/run/user/$uid/krb5cc/ to cache?
Yes, this is exactly what we want as a ccache.
LS
Yes, I saw that comment and I agree with Simo.
But I think we should examine the ccache collection and in the case there already is a valid cache present there, we should reuse it just like we did before.
I did not realize, that new ccache is created after another login. Thank you for review.
Updated patches are attached.
LS
On Wed, Jun 05, 2013 at 09:37:28AM +0200, Lukas Slebodnik wrote:
On (02/06/13 23:14), Jakub Hrozek wrote:
On Sat, 2013-06-01 at 11:43 +0200, Lukas Slebodnik wrote:
On (31/05/13 21:29), Jakub Hrozek wrote:
On Wed, 2013-05-29 at 16:09 +0200, Lukas Slebodnik wrote:
On (29/05/13 08:44), Simo Sorce wrote:
On Wed, 2013-05-29 at 11:30 +0200, Lukas Slebodnik wrote: > On (29/05/13 11:07), Lukas Slebodnik wrote: > >ehlo, > > > >Function krb5_cc_get_full_name is called only as a way to validate that, > >we have the right cache. Instead of returned name, location will be returned > >from function cc_dir_cache_for_princ. > > > >https://fedorahosted.org/sssd/ticket/1936 > > > >Patch is attached. > > > >LS > > self NACK > > this patch store to cache DIR:/run/user/325600000/krb5cc > ^^^^ > missing colon?
No, this is the correct form.
I found out, that it is a correct form. Problem was with checking ccname in function sss_krb5_cc_file_path.
New patches attached.
LS
Sorry for the reply from gmail. My OTP token decided the best password for me on a Friday evening is "Err", so I can't access my redhat.com account at the moment.
These patches break one assumption we want to keep -- if there is a user logged in and the same user logs in for example from another terminal, they should have the same ccache. With your patches, I'm getting a new one when I log in simultaneously.
I haven't tested that, but I guess this is because path to collection is always passed to the krb5_child now. I think that in the case user is already logged in (in krb5 code we denote this with "ccache is active"), then you should pass the full path to the ccache to the krb5_child.
Simo wrote in ticket comment (https://fedorahosted.org/sssd/ticket/1936#comment:10)
Do we really want to store only DIR:/run/user/$uid/krb5cc/ to cache?
Yes, this is exactly what we want as a ccache.
LS
Yes, I saw that comment and I agree with Simo.
But I think we should examine the ccache collection and in the case there already is a valid cache present there, we should reuse it just like we did before.
I did not realize, that new ccache is created after another login. Thank you for review.
Updated patches are attached.
LS
I'm afraid the detection works too well :-)
I tested with an expired ccache: $ sudo klist /run/user/208800000/ccdir/tktGihEUV Ticket cache: FILE:/run/user/208800000/ccdir/tktGihEUV Default principal: admin@EXAMPLE.COM
Valid starting Expires Service principal 06/07/2013 14:57:16 06/08/2013 14:57:16 krbtgt/EXAMPLE.COM@EXAMPLE.COM ^^^^^^^^^^^^^^^^^^ only valid until 8th of July
renew until 06/14/2013 14:57:16
And SSSD assigned me this ccache. I would expect that if there already is a ccache for the user, but it is expired (there is a function in the krb5 provider that checks if ccache is valid and active), then it would be overwritten with a fresh one.
On (12/06/13 16:05), Jakub Hrozek wrote:
On Wed, Jun 05, 2013 at 09:37:28AM +0200, Lukas Slebodnik wrote:
On (02/06/13 23:14), Jakub Hrozek wrote:
On Sat, 2013-06-01 at 11:43 +0200, Lukas Slebodnik wrote:
On (31/05/13 21:29), Jakub Hrozek wrote:
On Wed, 2013-05-29 at 16:09 +0200, Lukas Slebodnik wrote:
On (29/05/13 08:44), Simo Sorce wrote: >On Wed, 2013-05-29 at 11:30 +0200, Lukas Slebodnik wrote: >> On (29/05/13 11:07), Lukas Slebodnik wrote: >> >ehlo, >> > >> >Function krb5_cc_get_full_name is called only as a way to validate that, >> >we have the right cache. Instead of returned name, location will be returned >> >from function cc_dir_cache_for_princ. >> > >> >https://fedorahosted.org/sssd/ticket/1936 >> > >> >Patch is attached. >> > >> >LS >> >> self NACK >> >> this patch store to cache DIR:/run/user/325600000/krb5cc >> ^^^^ >> missing colon? > >No, this is the correct form. I found out, that it is a correct form. Problem was with checking ccname in function sss_krb5_cc_file_path.
New patches attached.
LS
Sorry for the reply from gmail. My OTP token decided the best password for me on a Friday evening is "Err", so I can't access my redhat.com account at the moment.
These patches break one assumption we want to keep -- if there is a user logged in and the same user logs in for example from another terminal, they should have the same ccache. With your patches, I'm getting a new one when I log in simultaneously.
I haven't tested that, but I guess this is because path to collection is always passed to the krb5_child now. I think that in the case user is already logged in (in krb5 code we denote this with "ccache is active"), then you should pass the full path to the ccache to the krb5_child.
Simo wrote in ticket comment (https://fedorahosted.org/sssd/ticket/1936#comment:10)
Do we really want to store only DIR:/run/user/$uid/krb5cc/ to cache?
Yes, this is exactly what we want as a ccache.
LS
Yes, I saw that comment and I agree with Simo.
But I think we should examine the ccache collection and in the case there already is a valid cache present there, we should reuse it just like we did before.
I did not realize, that new ccache is created after another login. Thank you for review.
Updated patches are attached.
LS
I'm afraid the detection works too well :-)
I tested with an expired ccache: $ sudo klist /run/user/208800000/ccdir/tktGihEUV Ticket cache: FILE:/run/user/208800000/ccdir/tktGihEUV Default principal: admin@EXAMPLE.COM
Valid starting Expires Service principal 06/07/2013 14:57:16 06/08/2013 14:57:16 krbtgt/EXAMPLE.COM@EXAMPLE.COM ^^^^^^^^^^^^^^^^^^ only valid until 8th of July
renew until 06/14/2013 14:57:16
And SSSD assigned me this ccache. I would expect that if there already is a ccache for the user, but it is expired (there is a function in the krb5 provider that checks if ccache is valid and active), then it would be overwritten with a fresh one.
With new patches you will get only valid cache.
LS
On Mon, Jun 17, 2013 at 12:02:02PM +0200, Lukas Slebodnik wrote:
On (12/06/13 16:05), Jakub Hrozek wrote:
On Wed, Jun 05, 2013 at 09:37:28AM +0200, Lukas Slebodnik wrote:
On (02/06/13 23:14), Jakub Hrozek wrote:
On Sat, 2013-06-01 at 11:43 +0200, Lukas Slebodnik wrote:
On (31/05/13 21:29), Jakub Hrozek wrote:
On Wed, 2013-05-29 at 16:09 +0200, Lukas Slebodnik wrote: > On (29/05/13 08:44), Simo Sorce wrote: > >On Wed, 2013-05-29 at 11:30 +0200, Lukas Slebodnik wrote: > >> On (29/05/13 11:07), Lukas Slebodnik wrote: > >> >ehlo, > >> > > >> >Function krb5_cc_get_full_name is called only as a way to validate that, > >> >we have the right cache. Instead of returned name, location will be returned > >> >from function cc_dir_cache_for_princ. > >> > > >> >https://fedorahosted.org/sssd/ticket/1936 > >> > > >> >Patch is attached. > >> > > >> >LS > >> > >> self NACK > >> > >> this patch store to cache DIR:/run/user/325600000/krb5cc > >> ^^^^ > >> missing colon? > > > >No, this is the correct form. > I found out, that it is a correct form. > Problem was with checking ccname in function sss_krb5_cc_file_path. > > New patches attached. > > LS
Sorry for the reply from gmail. My OTP token decided the best password for me on a Friday evening is "Err", so I can't access my redhat.com account at the moment.
These patches break one assumption we want to keep -- if there is a user logged in and the same user logs in for example from another terminal, they should have the same ccache. With your patches, I'm getting a new one when I log in simultaneously.
I haven't tested that, but I guess this is because path to collection is always passed to the krb5_child now. I think that in the case user is already logged in (in krb5 code we denote this with "ccache is active"), then you should pass the full path to the ccache to the krb5_child.
Simo wrote in ticket comment (https://fedorahosted.org/sssd/ticket/1936#comment:10)
Do we really want to store only DIR:/run/user/$uid/krb5cc/ to cache?
Yes, this is exactly what we want as a ccache.
LS
Yes, I saw that comment and I agree with Simo.
But I think we should examine the ccache collection and in the case there already is a valid cache present there, we should reuse it just like we did before.
I did not realize, that new ccache is created after another login. Thank you for review.
Updated patches are attached.
LS
I'm afraid the detection works too well :-)
I tested with an expired ccache: $ sudo klist /run/user/208800000/ccdir/tktGihEUV Ticket cache: FILE:/run/user/208800000/ccdir/tktGihEUV Default principal: admin@EXAMPLE.COM
Valid starting Expires Service principal 06/07/2013 14:57:16 06/08/2013 14:57:16 krbtgt/EXAMPLE.COM@EXAMPLE.COM ^^^^^^^^^^^^^^^^^^ only valid until 8th of July
renew until 06/14/2013 14:57:16
And SSSD assigned me this ccache. I would expect that if there already is a ccache for the user, but it is expired (there is a function in the krb5 provider that checks if ccache is valid and active), then it would be overwritten with a fresh one.
With new patches you will get only valid cache.
LS
I would prefer if the checks can be done in the responder and not in the krb5_child. The current logic is documented and evaluated in krb5_auth_prepare_ccache_file(), do you think it would be possible to enhance krb5_auth_prepare_ccache_file() with the needed new checks? Additionally there is already code in the responder e.g. to check for a valid TGT which might help to simplify your patch.
The responder then can send either
DIR::/run/user/325600000/krb5cc/ to indicate that a new ccache should be created in the collection, and that existing ccaches in the collection for the principal can be removed, because they are invalid and not used
or
DIR::/run/user/325600000/krb5cc/tktt1yNP8 to indicated that the specified ccache in the collection should be reused
to the krb5_child. Does this make sense?
@@ -1100,13 +1257,19 @@ static krb5_error_code get_and_save_tgt(struct krb5_req *kr,
DEBUG(SSSDBG_TRACE_FUNC, ("Attempting kinit for realm [%s]\n",realm_name));
- kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ,
discard_const(password),sss_krb5_prompter, kr, 0,NULL, kr->options);- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);return kerr;
- have_old_cache = retrieved_valid_cred_from_cache(kr->ctx, kr->princ,
realm_name, kr->ccname,&kr->creds);- if (!have_old_cache) {
we always want to get a new TGT and update the existing cache to increase the use time of the ticket, otherwise we would be limited by the lifetime of the ticket and the use-the-screensaver-to-get-a-new-valid-ticket trick wouldn't work anymore.
kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ,discard_const(password),sss_krb5_prompter, kr, 0,NULL, kr->options);if (kerr != 0) {KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);return kerr;}}
if (kr->validate) {
bye, Sumit
On (19/06/13 12:20), Sumit Bose wrote:
On Mon, Jun 17, 2013 at 12:02:02PM +0200, Lukas Slebodnik wrote:
On (12/06/13 16:05), Jakub Hrozek wrote:
On Wed, Jun 05, 2013 at 09:37:28AM +0200, Lukas Slebodnik wrote:
On (02/06/13 23:14), Jakub Hrozek wrote:
On Sat, 2013-06-01 at 11:43 +0200, Lukas Slebodnik wrote:
On (31/05/13 21:29), Jakub Hrozek wrote: >On Wed, 2013-05-29 at 16:09 +0200, Lukas Slebodnik wrote: >> On (29/05/13 08:44), Simo Sorce wrote: >> >On Wed, 2013-05-29 at 11:30 +0200, Lukas Slebodnik wrote: >> >> On (29/05/13 11:07), Lukas Slebodnik wrote: >> >> >ehlo, >> >> > >> >> >Function krb5_cc_get_full_name is called only as a way to validate that, >> >> >we have the right cache. Instead of returned name, location will be returned >> >> >from function cc_dir_cache_for_princ. >> >> > >> >> >https://fedorahosted.org/sssd/ticket/1936 >> >> > >> >> >Patch is attached. >> >> > >> >> >LS >> >> >> >> self NACK >> >> >> >> this patch store to cache DIR:/run/user/325600000/krb5cc >> >> ^^^^ >> >> missing colon? >> > >> >No, this is the correct form. >> I found out, that it is a correct form. >> Problem was with checking ccname in function sss_krb5_cc_file_path. >> >> New patches attached. >> >> LS > >Sorry for the reply from gmail. My OTP token decided the best password >for me on a Friday evening is "Err", so I can't access my redhat.com >account at the moment. > >These patches break one assumption we want to keep -- if there is a user >logged in and the same user logs in for example from another terminal, >they should have the same ccache. With your patches, I'm getting a new >one when I log in simultaneously. > >I haven't tested that, but I guess this is because path to collection is >always passed to the krb5_child now. I think that in the case user is >already logged in (in krb5 code we denote this with "ccache is active"), >then you should pass the full path to the ccache to the krb5_child. > Simo wrote in ticket comment (https://fedorahosted.org/sssd/ticket/1936#comment:10)
> Do we really want to store only DIR:/run/user/$uid/krb5cc/ to cache? Yes, this is exactly what we want as a ccache.
LS
Yes, I saw that comment and I agree with Simo.
But I think we should examine the ccache collection and in the case there already is a valid cache present there, we should reuse it just like we did before.
I did not realize, that new ccache is created after another login. Thank you for review.
Updated patches are attached.
LS
I'm afraid the detection works too well :-)
I tested with an expired ccache: $ sudo klist /run/user/208800000/ccdir/tktGihEUV Ticket cache: FILE:/run/user/208800000/ccdir/tktGihEUV Default principal: admin@EXAMPLE.COM
Valid starting Expires Service principal 06/07/2013 14:57:16 06/08/2013 14:57:16 krbtgt/EXAMPLE.COM@EXAMPLE.COM ^^^^^^^^^^^^^^^^^^ only valid until 8th of July
renew until 06/14/2013 14:57:16
And SSSD assigned me this ccache. I would expect that if there already is a ccache for the user, but it is expired (there is a function in the krb5 provider that checks if ccache is valid and active), then it would be overwritten with a fresh one.
With new patches you will get only valid cache.
LS
I would prefer if the checks can be done in the responder and not in the krb5_child. The current logic is documented and evaluated in krb5_auth_prepare_ccache_file(), do you think it would be possible to enhance krb5_auth_prepare_ccache_file() with the needed new checks? Additionally there is already code in the responder e.g. to check for a valid TGT which might help to simplify your patch.
The responder then can send either
DIR::/run/user/325600000/krb5cc/ to indicate that a new ccache should be created in the collection, and that existing ccaches in the collection for the principal can be removed, because they are invalid and not used
or
DIR::/run/user/325600000/krb5cc/tktt1yNP8 to indicated that the specified ccache in the collection should be reused
to the krb5_child. Does this make sense?
@@ -1100,13 +1257,19 @@ static krb5_error_code get_and_save_tgt(struct krb5_req *kr,
DEBUG(SSSDBG_TRACE_FUNC, ("Attempting kinit for realm [%s]\n",realm_name));
- kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ,
discard_const(password),sss_krb5_prompter, kr, 0,NULL, kr->options);- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);return kerr;
- have_old_cache = retrieved_valid_cred_from_cache(kr->ctx, kr->princ,
realm_name, kr->ccname,&kr->creds);- if (!have_old_cache) {
we always want to get a new TGT and update the existing cache to increase the use time of the ticket, otherwise we would be limited by the lifetime of the ticket and the use-the-screensaver-to-get-a-new-valid-ticket trick wouldn't work anymore.
At first, I wanted to use krb5_get_renewed_creds to renew credential, but it became too complicated. Therefore I inspired in kinit code (new ccache is created with the same name as old ccname)
kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ,discard_const(password),sss_krb5_prompter, kr, 0,NULL, kr->options);if (kerr != 0) {KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);return kerr;}}
if (kr->validate) {
bye, Sumit
Rewritten patches are attached.
LS
On Thu, Jun 20, 2013 at 11:11:17AM +0200, Lukas Slebodnik wrote:
Rewritten patches are attached.
LS
Two nitpicks:
+static char * get_ccache_name_by_principal(TALLOC_CTX *mem_ctx,
krb5_context ctx,krb5_principal principal,const char *ccname)+{
- krb5_error_code kerr;
- krb5_ccache tmp_cc = NULL;
- char *tmp_ccname = NULL;
- char *ret_ccname = NULL;
- kerr = krb5_cc_set_default_name(ctx, ccname);
- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);return NULL;- }
- kerr = krb5_cc_cache_match(ctx, principal, &tmp_cc);
- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);return NULL;- }
- kerr = krb5_cc_get_full_name(ctx, tmp_cc, &tmp_ccname);
- if (kerr !=0) {
KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);- }
- ret_ccname = talloc_strdup(mem_ctx, tmp_ccname);
- if (ret_ccname == NULL) {
DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed (ENOMEM).\n"));- }
+done:
This done label is unused and GCC emits a warning over that. It should be used if krb5_cc_get_full_name() fails.
- if (tmp_cc != NULL) {
kerr = krb5_cc_close(ctx, tmp_cc);if (kerr != 0) {KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);}- }
- krb5_free_string(ctx, tmp_ccname);
- return ret_ccname;
+}
- kerr = krb5_unparse_name(ctx, princ, &full_name);
- if (kerr !=0) {
KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);goto done;- }
- if (0 == strcmp(default_full_name, full_name)) {
ret = true;- }
If you're changing the other patch, can you also add DEBUG message here saying what has been compared?
+done:
- if (default_cc != NULL) {
kerr = krb5_cc_close(ctx, default_cc);if (kerr != 0) {KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr);goto done;}- }
- /* all functions can be safely called with NULL. */
- krb5_free_principal(ctx, default_princ);
- krb5_free_unparsed_name(ctx, default_full_name);
- krb5_free_unparsed_name(ctx, full_name);
- return ret;
+}
So far I ran a number of tests against IPA. I should still check AD provider especially with enterprise principals to see if they still work. If they do I'll ack.
But I will also file a ticket for 1.10.1 to move the checks to the backend so they are all done on one place as Sumit suggested earlier.
I don't think it's necessary to delay 1.10.0 any longer for this issue, but it's the right thing to do eventually.
On (21/06/13 20:45), Jakub Hrozek wrote:
On Thu, Jun 20, 2013 at 11:11:17AM +0200, Lukas Slebodnik wrote:
Rewritten patches are attached.
LS
Two nitpicks:
+static char * get_ccache_name_by_principal(TALLOC_CTX *mem_ctx,
krb5_context ctx,krb5_principal principal,const char *ccname)+{
- krb5_error_code kerr;
- krb5_ccache tmp_cc = NULL;
- char *tmp_ccname = NULL;
- char *ret_ccname = NULL;
- kerr = krb5_cc_set_default_name(ctx, ccname);
- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);return NULL;- }
- kerr = krb5_cc_cache_match(ctx, principal, &tmp_cc);
- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);return NULL;- }
- kerr = krb5_cc_get_full_name(ctx, tmp_cc, &tmp_ccname);
- if (kerr !=0) {
KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);- }
- ret_ccname = talloc_strdup(mem_ctx, tmp_ccname);
- if (ret_ccname == NULL) {
DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed (ENOMEM).\n"));- }
+done:
This done label is unused and GCC emits a warning over that. It should be used if krb5_cc_get_full_name() fails.
- if (tmp_cc != NULL) {
kerr = krb5_cc_close(ctx, tmp_cc);if (kerr != 0) {KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);}- }
- krb5_free_string(ctx, tmp_ccname);
- return ret_ccname;
+}
- kerr = krb5_unparse_name(ctx, princ, &full_name);
- if (kerr !=0) {
KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);goto done;- }
- if (0 == strcmp(default_full_name, full_name)) {
ret = true;- }
If you're changing the other patch, can you also add DEBUG message here saying what has been compared?
+done:
- if (default_cc != NULL) {
kerr = krb5_cc_close(ctx, default_cc);if (kerr != 0) {KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr);goto done;}- }
- /* all functions can be safely called with NULL. */
- krb5_free_principal(ctx, default_princ);
- krb5_free_unparsed_name(ctx, default_full_name);
- krb5_free_unparsed_name(ctx, full_name);
- return ret;
+}
So far I ran a number of tests against IPA. I should still check AD provider especially with enterprise principals to see if they still work. If they do I'll ack.
But I will also file a ticket for 1.10.1 to move the checks to the backend so they are all done on one place as Sumit suggested earlier.
I removed checks in former updates. There left only mapping from collection ccache (DIR:<cc_dir_name>) to ccache (DIR::<cc_dir_name>/<ticket_name>)
I tried some modifications on backend side (with gdb), but I could not get an aim 1. store collection ccache name stored in sysdb 2. environment variable KRB5CCNAME also contains collection ccache name 3. do not have more ccaches with same principal name
I don't think it's necessary to delay 1.10.0 any longer for this issue, but it's the right thing to do eventually.
Updated patches are attached.
LS
On Sat, Jun 22, 2013 at 01:55:51PM +0200, Lukas Slebodnik wrote:
On (21/06/13 20:45), Jakub Hrozek wrote:
On Thu, Jun 20, 2013 at 11:11:17AM +0200, Lukas Slebodnik wrote:
Rewritten patches are attached.
LS
Two nitpicks:
+static char * get_ccache_name_by_principal(TALLOC_CTX *mem_ctx,
krb5_context ctx,krb5_principal principal,const char *ccname)+{
- krb5_error_code kerr;
- krb5_ccache tmp_cc = NULL;
- char *tmp_ccname = NULL;
- char *ret_ccname = NULL;
- kerr = krb5_cc_set_default_name(ctx, ccname);
- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);return NULL;- }
- kerr = krb5_cc_cache_match(ctx, principal, &tmp_cc);
- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);return NULL;- }
- kerr = krb5_cc_get_full_name(ctx, tmp_cc, &tmp_ccname);
- if (kerr !=0) {
KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);- }
- ret_ccname = talloc_strdup(mem_ctx, tmp_ccname);
- if (ret_ccname == NULL) {
DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed (ENOMEM).\n"));- }
+done:
This done label is unused and GCC emits a warning over that. It should be used if krb5_cc_get_full_name() fails.
- if (tmp_cc != NULL) {
kerr = krb5_cc_close(ctx, tmp_cc);if (kerr != 0) {KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);}- }
- krb5_free_string(ctx, tmp_ccname);
- return ret_ccname;
+}
- kerr = krb5_unparse_name(ctx, princ, &full_name);
- if (kerr !=0) {
KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);goto done;- }
- if (0 == strcmp(default_full_name, full_name)) {
ret = true;- }
If you're changing the other patch, can you also add DEBUG message here saying what has been compared?
+done:
- if (default_cc != NULL) {
kerr = krb5_cc_close(ctx, default_cc);if (kerr != 0) {KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr);goto done;}- }
- /* all functions can be safely called with NULL. */
- krb5_free_principal(ctx, default_princ);
- krb5_free_unparsed_name(ctx, default_full_name);
- krb5_free_unparsed_name(ctx, full_name);
- return ret;
+}
So far I ran a number of tests against IPA. I should still check AD provider especially with enterprise principals to see if they still work. If they do I'll ack.
But I will also file a ticket for 1.10.1 to move the checks to the backend so they are all done on one place as Sumit suggested earlier.
I removed checks in former updates. There left only mapping from collection ccache (DIR:<cc_dir_name>) to ccache (DIR::<cc_dir_name>/<ticket_name>)
I tried some modifications on backend side (with gdb), but I could not get an aim
- store collection ccache name stored in sysdb
- environment variable KRB5CCNAME also contains collection ccache name
- do not have more ccaches with same principal name
I don't think it's necessary to delay 1.10.0 any longer for this issue, but it's the right thing to do eventually.
Updated patches are attached.
LS
I still see two issues, sorry:
1) The cc_residual_is_used check seems to be broken:
[sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x1000): User [1983201109] is still active, reusing ccache [/run/user/1983201109]. [sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x0200): Cache file [/run/user/1983201109/primary] does not exist, it will be recreated ^^^^^^^^^^^^^^^^^ This path cannot exist, I think SSSD should be checking for /run/user/1983201109/krb5cc/primary instead
[sssd[be[AD.EXAMPLE.COM]]] [check_for_valid_tgt] (0x0020): krb5_cc_retrieve_cred failed. [sssd[be[AD.EXAMPLE.COM]]] [fo_resolve_service_send] (0x0100): Trying to resolve service 'AD'
2) When using enterprise credentials, multiple logins always get a new ccache. I don't see this happening with origin/master, sorry.
The first issue is probably not caused by these patches, so it's OK to just create a ticket. The second one needs to be fixed, though.
On (24/06/13 22:06), Jakub Hrozek wrote:
On Sat, Jun 22, 2013 at 01:55:51PM +0200, Lukas Slebodnik wrote:
On (21/06/13 20:45), Jakub Hrozek wrote:
On Thu, Jun 20, 2013 at 11:11:17AM +0200, Lukas Slebodnik wrote:
Rewritten patches are attached.
LS
Two nitpicks:
+static char * get_ccache_name_by_principal(TALLOC_CTX *mem_ctx,
krb5_context ctx,krb5_principal principal,const char *ccname)+{
- krb5_error_code kerr;
- krb5_ccache tmp_cc = NULL;
- char *tmp_ccname = NULL;
- char *ret_ccname = NULL;
- kerr = krb5_cc_set_default_name(ctx, ccname);
- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);return NULL;- }
- kerr = krb5_cc_cache_match(ctx, principal, &tmp_cc);
- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);return NULL;- }
- kerr = krb5_cc_get_full_name(ctx, tmp_cc, &tmp_ccname);
- if (kerr !=0) {
KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);- }
- ret_ccname = talloc_strdup(mem_ctx, tmp_ccname);
- if (ret_ccname == NULL) {
DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed (ENOMEM).\n"));- }
+done:
This done label is unused and GCC emits a warning over that. It should be used if krb5_cc_get_full_name() fails.
- if (tmp_cc != NULL) {
kerr = krb5_cc_close(ctx, tmp_cc);if (kerr != 0) {KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);}- }
- krb5_free_string(ctx, tmp_ccname);
- return ret_ccname;
+}
- kerr = krb5_unparse_name(ctx, princ, &full_name);
- if (kerr !=0) {
KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);goto done;- }
- if (0 == strcmp(default_full_name, full_name)) {
ret = true;- }
If you're changing the other patch, can you also add DEBUG message here saying what has been compared?
+done:
- if (default_cc != NULL) {
kerr = krb5_cc_close(ctx, default_cc);if (kerr != 0) {KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr);goto done;}- }
- /* all functions can be safely called with NULL. */
- krb5_free_principal(ctx, default_princ);
- krb5_free_unparsed_name(ctx, default_full_name);
- krb5_free_unparsed_name(ctx, full_name);
- return ret;
+}
So far I ran a number of tests against IPA. I should still check AD provider especially with enterprise principals to see if they still work. If they do I'll ack.
But I will also file a ticket for 1.10.1 to move the checks to the backend so they are all done on one place as Sumit suggested earlier.
I removed checks in former updates. There left only mapping from collection ccache (DIR:<cc_dir_name>) to ccache (DIR::<cc_dir_name>/<ticket_name>)
I tried some modifications on backend side (with gdb), but I could not get an aim
- store collection ccache name stored in sysdb
- environment variable KRB5CCNAME also contains collection ccache name
- do not have more ccaches with same principal name
I don't think it's necessary to delay 1.10.0 any longer for this issue, but it's the right thing to do eventually.
Updated patches are attached.
LS
I still see two issues, sorry:
- The cc_residual_is_used check seems to be broken:
[sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x1000): User [1983201109] is still active, reusing ccache [/run/user/1983201109]. [sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x0200): Cache file [/run/user/1983201109/primary] does not exist, it will be recreated ^^^^^^^^^^^^^^^^^ This path cannot exist, I think SSSD should be checking for /run/user/1983201109/krb5cc/primary instead
[sssd[be[AD.EXAMPLE.COM]]] [check_for_valid_tgt] (0x0020): krb5_cc_retrieve_cred failed. [sssd[be[AD.EXAMPLE.COM]]] [fo_resolve_service_send] (0x0100): Trying to resolve service 'AD'
- When using enterprise credentials, multiple logins always get a new
ccache. I don't see this happening with origin/master, sorry.
I thought, that it is aim of cache collection to have separate ccache for different logins.
The first issue is probably not caused by these patches, so it's OK to just create a ticket. The second one needs to be fixed, though.
LS
On Mon, Jun 24, 2013 at 10:54:30PM +0200, Lukas Slebodnik wrote:
On (24/06/13 22:06), Jakub Hrozek wrote:
On Sat, Jun 22, 2013 at 01:55:51PM +0200, Lukas Slebodnik wrote:
On (21/06/13 20:45), Jakub Hrozek wrote:
On Thu, Jun 20, 2013 at 11:11:17AM +0200, Lukas Slebodnik wrote:
Rewritten patches are attached.
LS
Two nitpicks:
+static char * get_ccache_name_by_principal(TALLOC_CTX *mem_ctx,
krb5_context ctx,krb5_principal principal,const char *ccname)+{
- krb5_error_code kerr;
- krb5_ccache tmp_cc = NULL;
- char *tmp_ccname = NULL;
- char *ret_ccname = NULL;
- kerr = krb5_cc_set_default_name(ctx, ccname);
- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);return NULL;- }
- kerr = krb5_cc_cache_match(ctx, principal, &tmp_cc);
- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);return NULL;- }
- kerr = krb5_cc_get_full_name(ctx, tmp_cc, &tmp_ccname);
- if (kerr !=0) {
KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);- }
- ret_ccname = talloc_strdup(mem_ctx, tmp_ccname);
- if (ret_ccname == NULL) {
DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed (ENOMEM).\n"));- }
+done:
This done label is unused and GCC emits a warning over that. It should be used if krb5_cc_get_full_name() fails.
- if (tmp_cc != NULL) {
kerr = krb5_cc_close(ctx, tmp_cc);if (kerr != 0) {KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);}- }
- krb5_free_string(ctx, tmp_ccname);
- return ret_ccname;
+}
- kerr = krb5_unparse_name(ctx, princ, &full_name);
- if (kerr !=0) {
KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);goto done;- }
- if (0 == strcmp(default_full_name, full_name)) {
ret = true;- }
If you're changing the other patch, can you also add DEBUG message here saying what has been compared?
+done:
- if (default_cc != NULL) {
kerr = krb5_cc_close(ctx, default_cc);if (kerr != 0) {KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr);goto done;}- }
- /* all functions can be safely called with NULL. */
- krb5_free_principal(ctx, default_princ);
- krb5_free_unparsed_name(ctx, default_full_name);
- krb5_free_unparsed_name(ctx, full_name);
- return ret;
+}
So far I ran a number of tests against IPA. I should still check AD provider especially with enterprise principals to see if they still work. If they do I'll ack.
But I will also file a ticket for 1.10.1 to move the checks to the backend so they are all done on one place as Sumit suggested earlier.
I removed checks in former updates. There left only mapping from collection ccache (DIR:<cc_dir_name>) to ccache (DIR::<cc_dir_name>/<ticket_name>)
I tried some modifications on backend side (with gdb), but I could not get an aim
- store collection ccache name stored in sysdb
- environment variable KRB5CCNAME also contains collection ccache name
- do not have more ccaches with same principal name
I don't think it's necessary to delay 1.10.0 any longer for this issue, but it's the right thing to do eventually.
Updated patches are attached.
LS
I still see two issues, sorry:
- The cc_residual_is_used check seems to be broken:
[sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x1000): User [1983201109] is still active, reusing ccache [/run/user/1983201109]. [sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x0200): Cache file [/run/user/1983201109/primary] does not exist, it will be recreated ^^^^^^^^^^^^^^^^^ This path cannot exist, I think SSSD should be checking for /run/user/1983201109/krb5cc/primary instead
[sssd[be[AD.EXAMPLE.COM]]] [check_for_valid_tgt] (0x0020): krb5_cc_retrieve_cred failed. [sssd[be[AD.EXAMPLE.COM]]] [fo_resolve_service_send] (0x0100): Trying to resolve service 'AD'
- When using enterprise credentials, multiple logins always get a new
ccache. I don't see this happening with origin/master, sorry.
I thought, that it is aim of cache collection to have separate ccache for different logins.
Yes, but these were two separate logins of the same principal. I've demonstrated the issue to Lukas in a VM to make sure we both see the problem.
On (25/06/13 11:40), Jakub Hrozek wrote:
On Mon, Jun 24, 2013 at 10:54:30PM +0200, Lukas Slebodnik wrote:
On (24/06/13 22:06), Jakub Hrozek wrote:
On Sat, Jun 22, 2013 at 01:55:51PM +0200, Lukas Slebodnik wrote:
On (21/06/13 20:45), Jakub Hrozek wrote:
On Thu, Jun 20, 2013 at 11:11:17AM +0200, Lukas Slebodnik wrote:
Rewritten patches are attached.
LS
Two nitpicks:
+static char * get_ccache_name_by_principal(TALLOC_CTX *mem_ctx,
krb5_context ctx,krb5_principal principal,const char *ccname)+{
- krb5_error_code kerr;
- krb5_ccache tmp_cc = NULL;
- char *tmp_ccname = NULL;
- char *ret_ccname = NULL;
- kerr = krb5_cc_set_default_name(ctx, ccname);
- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);return NULL;- }
- kerr = krb5_cc_cache_match(ctx, principal, &tmp_cc);
- if (kerr != 0) {
KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);return NULL;- }
- kerr = krb5_cc_get_full_name(ctx, tmp_cc, &tmp_ccname);
- if (kerr !=0) {
KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);- }
- ret_ccname = talloc_strdup(mem_ctx, tmp_ccname);
- if (ret_ccname == NULL) {
DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed (ENOMEM).\n"));- }
+done:
This done label is unused and GCC emits a warning over that. It should be used if krb5_cc_get_full_name() fails.
- if (tmp_cc != NULL) {
kerr = krb5_cc_close(ctx, tmp_cc);if (kerr != 0) {KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr);}- }
- krb5_free_string(ctx, tmp_ccname);
- return ret_ccname;
+}
- kerr = krb5_unparse_name(ctx, princ, &full_name);
- if (kerr !=0) {
KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr);goto done;- }
- if (0 == strcmp(default_full_name, full_name)) {
ret = true;- }
If you're changing the other patch, can you also add DEBUG message here saying what has been compared?
+done:
- if (default_cc != NULL) {
kerr = krb5_cc_close(ctx, default_cc);if (kerr != 0) {KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr);goto done;}- }
- /* all functions can be safely called with NULL. */
- krb5_free_principal(ctx, default_princ);
- krb5_free_unparsed_name(ctx, default_full_name);
- krb5_free_unparsed_name(ctx, full_name);
- return ret;
+}
So far I ran a number of tests against IPA. I should still check AD provider especially with enterprise principals to see if they still work. If they do I'll ack.
But I will also file a ticket for 1.10.1 to move the checks to the backend so they are all done on one place as Sumit suggested earlier.
I removed checks in former updates. There left only mapping from collection ccache (DIR:<cc_dir_name>) to ccache (DIR::<cc_dir_name>/<ticket_name>)
I tried some modifications on backend side (with gdb), but I could not get an aim
- store collection ccache name stored in sysdb
- environment variable KRB5CCNAME also contains collection ccache name
- do not have more ccaches with same principal name
I don't think it's necessary to delay 1.10.0 any longer for this issue, but it's the right thing to do eventually.
Updated patches are attached.
LS
I still see two issues, sorry:
- The cc_residual_is_used check seems to be broken:
[sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x1000): User [1983201109] is still active, reusing ccache [/run/user/1983201109]. [sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x0200): Cache file [/run/user/1983201109/primary] does not exist, it will be recreated ^^^^^^^^^^^^^^^^^ This path cannot exist, I think SSSD should be checking for /run/user/1983201109/krb5cc/primary instead
[sssd[be[AD.EXAMPLE.COM]]] [check_for_valid_tgt] (0x0020): krb5_cc_retrieve_cred failed. [sssd[be[AD.EXAMPLE.COM]]] [fo_resolve_service_send] (0x0100): Trying to resolve service 'AD'
- When using enterprise credentials, multiple logins always get a new
ccache. I don't see this happening with origin/master, sorry.
I thought, that it is aim of cache collection to have separate ccache for different logins.
Yes, but these were two separate logins of the same principal. I've demonstrated the issue to Lukas in a VM to make sure we both see the problem.
I would like to describe situation, because I am not really sure, where is the problem.
String "ptest\@FOO.BAR@SSSD-AD.TEST" is sent to krb5_child with enterprise principal. We have empty cache collection. New ccache is created (after login). This new ccache has principal name "ptest@SSSD-AD.TEST"
Output from klist -l #> Principal name Cache name #> -------------- ---------- #> ptest@SSSD-AD.TEST DIR::/run/user/947005188/krb5cc/tktceNIlD
User decides to log on machine second time. Attribute ccacheFile has value "DIR:/run/user/947005188/krb5cc/", therefore we need find ccache in collection. With my patches, sssd uses function krb5_cc_cache_match to find ccache by principal. krb5_cc_cache_match internally uses krb5_principal_compare_flags to compare two principals and here is the problem
By default, krb5_principal_compare_flags is called with flags argument 0. In this case following principals are compared
principal->data principal->realm ------------------------------ ptest SSSD-AD.TEST //Principal obtained from existing ccache ptest@FOO.BAR SSSD-AD.TEST //Principal created by parsing string
I tried to call krb5_principal_compare_flags with argument flags = KRB5_PRINCIPAL_COMPARE_ENTERPRISE and in this case following principals were compared.
principal->data principal->realm ------------------------------ ptest SSSD-AD.TEST //Principal obtained from existing ccache ptest FOO.BAR //Principal created by parsing string
This is a reason, why ccache can't be found in collection and new ccache is created every time.
LS
On Wed, Jun 26, 2013 at 10:23:59AM +0200, Lukas Slebodnik wrote:
On (25/06/13 11:40), Jakub Hrozek wrote:
On Mon, Jun 24, 2013 at 10:54:30PM +0200, Lukas Slebodnik wrote:
On (24/06/13 22:06), Jakub Hrozek wrote:
On Sat, Jun 22, 2013 at 01:55:51PM +0200, Lukas Slebodnik wrote:
On (21/06/13 20:45), Jakub Hrozek wrote:
On Thu, Jun 20, 2013 at 11:11:17AM +0200, Lukas Slebodnik wrote: > Rewritten patches are attached. > > LS
Two nitpicks:
> +static char * get_ccache_name_by_principal(TALLOC_CTX *mem_ctx, > + krb5_context ctx, > + krb5_principal principal, > + const char *ccname) > +{ > + krb5_error_code kerr; > + krb5_ccache tmp_cc = NULL; > + char *tmp_ccname = NULL; > + char *ret_ccname = NULL; > + > + kerr = krb5_cc_set_default_name(ctx, ccname); > + if (kerr != 0) { > + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); > + return NULL; > + } > + > + kerr = krb5_cc_cache_match(ctx, principal, &tmp_cc); > + if (kerr != 0) { > + KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr); > + return NULL; > + } > + > + kerr = krb5_cc_get_full_name(ctx, tmp_cc, &tmp_ccname); > + if (kerr !=0) { > + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); > + } > + > + ret_ccname = talloc_strdup(mem_ctx, tmp_ccname); > + if (ret_ccname == NULL) { > + DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed (ENOMEM).\n")); > + } > + > +done:
This done label is unused and GCC emits a warning over that. It should be used if krb5_cc_get_full_name() fails.
> + if (tmp_cc != NULL) { > + kerr = krb5_cc_close(ctx, tmp_cc); > + if (kerr != 0) { > + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); > + } > + } > + krb5_free_string(ctx, tmp_ccname); > + > + return ret_ccname; > +} > +
> > + kerr = krb5_unparse_name(ctx, princ, &full_name); > + if (kerr !=0) { > + KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr); > + goto done; > + } > + > + if (0 == strcmp(default_full_name, full_name)) { > + ret = true; > + } > +
If you're changing the other patch, can you also add DEBUG message here saying what has been compared?
> +done: > + if (default_cc != NULL) { > + kerr = krb5_cc_close(ctx, default_cc); > + if (kerr != 0) { > + KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr); > + goto done; > + } > + } > + > + /* all functions can be safely called with NULL. */ > + krb5_free_principal(ctx, default_princ); > + krb5_free_unparsed_name(ctx, default_full_name); > + krb5_free_unparsed_name(ctx, full_name); > + > + return ret; > +}
So far I ran a number of tests against IPA. I should still check AD provider especially with enterprise principals to see if they still work. If they do I'll ack.
But I will also file a ticket for 1.10.1 to move the checks to the backend so they are all done on one place as Sumit suggested earlier.
I removed checks in former updates. There left only mapping from collection ccache (DIR:<cc_dir_name>) to ccache (DIR::<cc_dir_name>/<ticket_name>)
I tried some modifications on backend side (with gdb), but I could not get an aim
- store collection ccache name stored in sysdb
- environment variable KRB5CCNAME also contains collection ccache name
- do not have more ccaches with same principal name
I don't think it's necessary to delay 1.10.0 any longer for this issue, but it's the right thing to do eventually.
Updated patches are attached.
LS
I still see two issues, sorry:
- The cc_residual_is_used check seems to be broken:
[sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x1000): User [1983201109] is still active, reusing ccache [/run/user/1983201109]. [sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x0200): Cache file [/run/user/1983201109/primary] does not exist, it will be recreated ^^^^^^^^^^^^^^^^^ This path cannot exist, I think SSSD should be checking for /run/user/1983201109/krb5cc/primary instead
[sssd[be[AD.EXAMPLE.COM]]] [check_for_valid_tgt] (0x0020): krb5_cc_retrieve_cred failed. [sssd[be[AD.EXAMPLE.COM]]] [fo_resolve_service_send] (0x0100): Trying to resolve service 'AD'
- When using enterprise credentials, multiple logins always get a new
ccache. I don't see this happening with origin/master, sorry.
I thought, that it is aim of cache collection to have separate ccache for different logins.
Yes, but these were two separate logins of the same principal. I've demonstrated the issue to Lukas in a VM to make sure we both see the problem.
I would like to describe situation, because I am not really sure, where is the problem.
String "ptest\@FOO.BAR@SSSD-AD.TEST" is sent to krb5_child with enterprise
An enterprise principal should never be send to the krb5_child. (There was an issue with AD, because AD expected the canonicalization flag explicitly set even for enterprise principals, which lead to enterprise principals stored in SSSD's cache, but this is already fixed) Either the principal from the SSSD cache or a constructed one (username@REALM) should be send to krb5_child and both should not be enterprise principals.
The principal is then expanded to an enterprise principal in krb5_child if configured.
principal. We have empty cache collection. New ccache is created (after login). This new ccache has principal name "ptest@SSSD-AD.TEST"
Output from klist -l #> Principal name Cache name #> -------------- ---------- #> ptest@SSSD-AD.TEST DIR::/run/user/947005188/krb5cc/tktceNIlD
The principal form the cache (the canonicalized principal) is returned to the krb5 provider together with the ccache path and should be saved to the cache.
User decides to log on machine second time. Attribute ccacheFile has value "DIR:/run/user/947005188/krb5cc/", therefore we need find ccache in collection. With my patches, sssd uses function krb5_cc_cache_match to find ccache by principal. krb5_cc_cache_match internally uses krb5_principal_compare_flags to compare two principals and here is the problem
The second login should pick the principal from SSSD's cache which should be the one from above and hence the one from the ccache file. If configured the principal is expanded to an enterprise principal quite early in krb5_child. So you either make the comparison before the expansion, save the original principal in the context of the krb5_child and use it for the comparison or if possible do the comparison already in the krb5 provider.
HTH
bye, Sumit
By default, krb5_principal_compare_flags is called with flags argument 0. In this case following principals are compared
principal->data principal->realm
ptest SSSD-AD.TEST //Principal obtained from existing ccache ptest@FOO.BAR SSSD-AD.TEST //Principal created by parsing string
I tried to call krb5_principal_compare_flags with argument flags = KRB5_PRINCIPAL_COMPARE_ENTERPRISE and in this case following principals were compared.
principal->data principal->realm
ptest SSSD-AD.TEST //Principal obtained from existing ccache ptest FOO.BAR //Principal created by parsing string
This is a reason, why ccache can't be found in collection and new ccache is created every time.
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On (26/06/13 10:56), Sumit Bose wrote:
On Wed, Jun 26, 2013 at 10:23:59AM +0200, Lukas Slebodnik wrote:
On (25/06/13 11:40), Jakub Hrozek wrote:
On Mon, Jun 24, 2013 at 10:54:30PM +0200, Lukas Slebodnik wrote:
On (24/06/13 22:06), Jakub Hrozek wrote:
On Sat, Jun 22, 2013 at 01:55:51PM +0200, Lukas Slebodnik wrote:
On (21/06/13 20:45), Jakub Hrozek wrote: >On Thu, Jun 20, 2013 at 11:11:17AM +0200, Lukas Slebodnik wrote: >> Rewritten patches are attached. >> >> LS > >Two nitpicks: > >> +static char * get_ccache_name_by_principal(TALLOC_CTX *mem_ctx, >> + krb5_context ctx, >> + krb5_principal principal, >> + const char *ccname) >> +{ >> + krb5_error_code kerr; >> + krb5_ccache tmp_cc = NULL; >> + char *tmp_ccname = NULL; >> + char *ret_ccname = NULL; >> + >> + kerr = krb5_cc_set_default_name(ctx, ccname); >> + if (kerr != 0) { >> + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); >> + return NULL; >> + } >> + >> + kerr = krb5_cc_cache_match(ctx, principal, &tmp_cc); >> + if (kerr != 0) { >> + KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr); >> + return NULL; >> + } >> + >> + kerr = krb5_cc_get_full_name(ctx, tmp_cc, &tmp_ccname); >> + if (kerr !=0) { >> + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); >> + } >> + >> + ret_ccname = talloc_strdup(mem_ctx, tmp_ccname); >> + if (ret_ccname == NULL) { >> + DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed (ENOMEM).\n")); >> + } >> + >> +done: > >This done label is unused and GCC emits a warning over that. It should >be used if krb5_cc_get_full_name() fails. > >> + if (tmp_cc != NULL) { >> + kerr = krb5_cc_close(ctx, tmp_cc); >> + if (kerr != 0) { >> + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); >> + } >> + } >> + krb5_free_string(ctx, tmp_ccname); >> + >> + return ret_ccname; >> +} >> + > > >> >> + kerr = krb5_unparse_name(ctx, princ, &full_name); >> + if (kerr !=0) { >> + KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr); >> + goto done; >> + } >> + >> + if (0 == strcmp(default_full_name, full_name)) { >> + ret = true; >> + } >> + > >If you're changing the other patch, can you also add DEBUG message here >saying what has been compared? > >> +done: >> + if (default_cc != NULL) { >> + kerr = krb5_cc_close(ctx, default_cc); >> + if (kerr != 0) { >> + KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr); >> + goto done; >> + } >> + } >> + >> + /* all functions can be safely called with NULL. */ >> + krb5_free_principal(ctx, default_princ); >> + krb5_free_unparsed_name(ctx, default_full_name); >> + krb5_free_unparsed_name(ctx, full_name); >> + >> + return ret; >> +} > >So far I ran a number of tests against IPA. I should still check AD >provider especially with enterprise principals to see if they still >work. If they do I'll ack. > >But I will also file a ticket for 1.10.1 to move the checks to the >backend so they are all done on one place as Sumit suggested earlier. > I removed checks in former updates. There left only mapping from collection ccache (DIR:<cc_dir_name>) to ccache (DIR::<cc_dir_name>/<ticket_name>)
I tried some modifications on backend side (with gdb), but I could not get an aim
- store collection ccache name stored in sysdb
- environment variable KRB5CCNAME also contains collection ccache name
- do not have more ccaches with same principal name
>I don't think it's necessary to delay 1.10.0 any longer for this issue, >but it's the right thing to do eventually.
Updated patches are attached.
LS
I still see two issues, sorry:
- The cc_residual_is_used check seems to be broken:
[sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x1000): User [1983201109] is still active, reusing ccache [/run/user/1983201109]. [sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x0200): Cache file [/run/user/1983201109/primary] does not exist, it will be recreated ^^^^^^^^^^^^^^^^^ This path cannot exist, I think SSSD should be checking for /run/user/1983201109/krb5cc/primary instead
[sssd[be[AD.EXAMPLE.COM]]] [check_for_valid_tgt] (0x0020): krb5_cc_retrieve_cred failed. [sssd[be[AD.EXAMPLE.COM]]] [fo_resolve_service_send] (0x0100): Trying to resolve service 'AD'
- When using enterprise credentials, multiple logins always get a new
ccache. I don't see this happening with origin/master, sorry.
I thought, that it is aim of cache collection to have separate ccache for different logins.
Yes, but these were two separate logins of the same principal. I've demonstrated the issue to Lukas in a VM to make sure we both see the problem.
I would like to describe situation, because I am not really sure, where is the problem.
String "ptest\@FOO.BAR@SSSD-AD.TEST" is sent to krb5_child with enterprise
An enterprise principal should never be send to the krb5_child. (There was an issue with AD, because AD expected the canonicalization flag explicitly set even for enterprise principals, which lead to enterprise principals stored in SSSD's cache, but this is already fixed) Either the principal from the SSSD cache or a constructed one (username@REALM) should be send to krb5_child and both should not be enterprise principals.
The principal is then expanded to an enterprise principal in krb5_child if configured.
principal. We have empty cache collection. New ccache is created (after login). This new ccache has principal name "ptest@SSSD-AD.TEST"
Output from klist -l #> Principal name Cache name #> -------------- ---------- #> ptest@SSSD-AD.TEST DIR::/run/user/947005188/krb5cc/tktceNIlD
The principal form the cache (the canonicalized principal) is returned to the krb5 provider together with the ccache path and should be saved to the cache.
User decides to log on machine second time. Attribute ccacheFile has value "DIR:/run/user/947005188/krb5cc/", therefore we need find ccache in collection. With my patches, sssd uses function krb5_cc_cache_match to find ccache by principal. krb5_cc_cache_match internally uses krb5_principal_compare_flags to compare two principals and here is the problem
The second login should pick the principal from SSSD's cache which should be the one from above and hence the one from the ccache file. If configured the principal is expanded to an enterprise principal quite early in krb5_child. So you either make the comparison before the expansion, save the original principal in the context of the krb5_child and use it for the comparison or if possible do the comparison already in the krb5 provider.
I moved searching existing ccache after obtaining cerdentials from server (krb5_get_init_creds_password) So this is the main change in the second patch. 1st, 3rd patch are untouched.
Thank you very much Sumit.
Updated patches are attached.
LS
HTH
bye, Sumit
By default, krb5_principal_compare_flags is called with flags argument 0. In this case following principals are compared
principal->data principal->realm
ptest SSSD-AD.TEST //Principal obtained from existing ccache ptest@FOO.BAR SSSD-AD.TEST //Principal created by parsing string
I tried to call krb5_principal_compare_flags with argument flags = KRB5_PRINCIPAL_COMPARE_ENTERPRISE and in this case following principals were compared.
principal->data principal->realm
ptest SSSD-AD.TEST //Principal obtained from existing ccache ptest FOO.BAR //Principal created by parsing string
This is a reason, why ccache can't be found in collection and new ccache is created every time.
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On (26/06/13 10:56), Sumit Bose wrote:
On Wed, Jun 26, 2013 at 10:23:59AM +0200, Lukas Slebodnik wrote:
On (25/06/13 11:40), Jakub Hrozek wrote:
On Mon, Jun 24, 2013 at 10:54:30PM +0200, Lukas Slebodnik wrote:
On (24/06/13 22:06), Jakub Hrozek wrote:
On Sat, Jun 22, 2013 at 01:55:51PM +0200, Lukas Slebodnik wrote:
On (21/06/13 20:45), Jakub Hrozek wrote: >On Thu, Jun 20, 2013 at 11:11:17AM +0200, Lukas Slebodnik wrote: >> Rewritten patches are attached. >> >> LS > >Two nitpicks: > >> +static char * get_ccache_name_by_principal(TALLOC_CTX *mem_ctx, >> + krb5_context ctx, >> + krb5_principal principal, >> + const char *ccname) >> +{ >> + krb5_error_code kerr; >> + krb5_ccache tmp_cc = NULL; >> + char *tmp_ccname = NULL; >> + char *ret_ccname = NULL; >> + >> + kerr = krb5_cc_set_default_name(ctx, ccname); >> + if (kerr != 0) { >> + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); >> + return NULL; >> + } >> + >> + kerr = krb5_cc_cache_match(ctx, principal, &tmp_cc); >> + if (kerr != 0) { >> + KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr); >> + return NULL; >> + } >> + >> + kerr = krb5_cc_get_full_name(ctx, tmp_cc, &tmp_ccname); >> + if (kerr !=0) { >> + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); >> + } >> + >> + ret_ccname = talloc_strdup(mem_ctx, tmp_ccname); >> + if (ret_ccname == NULL) { >> + DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed (ENOMEM).\n")); >> + } >> + >> +done: > >This done label is unused and GCC emits a warning over that. It should >be used if krb5_cc_get_full_name() fails. > >> + if (tmp_cc != NULL) { >> + kerr = krb5_cc_close(ctx, tmp_cc); >> + if (kerr != 0) { >> + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); >> + } >> + } >> + krb5_free_string(ctx, tmp_ccname); >> + >> + return ret_ccname; >> +} >> + > > >> >> + kerr = krb5_unparse_name(ctx, princ, &full_name); >> + if (kerr !=0) { >> + KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr); >> + goto done; >> + } >> + >> + if (0 == strcmp(default_full_name, full_name)) { >> + ret = true; >> + } >> + > >If you're changing the other patch, can you also add DEBUG message here >saying what has been compared? > >> +done: >> + if (default_cc != NULL) { >> + kerr = krb5_cc_close(ctx, default_cc); >> + if (kerr != 0) { >> + KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr); >> + goto done; >> + } >> + } >> + >> + /* all functions can be safely called with NULL. */ >> + krb5_free_principal(ctx, default_princ); >> + krb5_free_unparsed_name(ctx, default_full_name); >> + krb5_free_unparsed_name(ctx, full_name); >> + >> + return ret; >> +} > >So far I ran a number of tests against IPA. I should still check AD >provider especially with enterprise principals to see if they still >work. If they do I'll ack. > >But I will also file a ticket for 1.10.1 to move the checks to the >backend so they are all done on one place as Sumit suggested earlier. > I removed checks in former updates. There left only mapping from collection ccache (DIR:<cc_dir_name>) to ccache (DIR::<cc_dir_name>/<ticket_name>)
I tried some modifications on backend side (with gdb), but I could not get an aim
- store collection ccache name stored in sysdb
- environment variable KRB5CCNAME also contains collection ccache name
- do not have more ccaches with same principal name
>I don't think it's necessary to delay 1.10.0 any longer for this issue, >but it's the right thing to do eventually.
Updated patches are attached.
LS
I still see two issues, sorry:
- The cc_residual_is_used check seems to be broken:
[sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x1000): User [1983201109] is still active, reusing ccache [/run/user/1983201109]. [sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x0200): Cache file [/run/user/1983201109/primary] does not exist, it will be recreated ^^^^^^^^^^^^^^^^^ This path cannot exist, I think SSSD should be checking for /run/user/1983201109/krb5cc/primary instead
[sssd[be[AD.EXAMPLE.COM]]] [check_for_valid_tgt] (0x0020): krb5_cc_retrieve_cred failed. [sssd[be[AD.EXAMPLE.COM]]] [fo_resolve_service_send] (0x0100): Trying to resolve service 'AD'
- When using enterprise credentials, multiple logins always get a new
ccache. I don't see this happening with origin/master, sorry.
I thought, that it is aim of cache collection to have separate ccache for different logins.
Yes, but these were two separate logins of the same principal. I've demonstrated the issue to Lukas in a VM to make sure we both see the problem.
I would like to describe situation, because I am not really sure, where is the problem.
String "ptest\@FOO.BAR@SSSD-AD.TEST" is sent to krb5_child with enterprise
An enterprise principal should never be send to the krb5_child. (There was an issue with AD, because AD expected the canonicalization flag explicitly set even for enterprise principals, which lead to enterprise principals stored in SSSD's cache, but this is already fixed) Either the principal from the SSSD cache or a constructed one (username@REALM) should be send to krb5_child and both should not be enterprise principals.
You are right, only "ptest@FOO.BAR" is sent to krb5_child and stored in kr->upn. Enterprise principal is generated in k5c_setup and stored in kr->name and this is the string, which I meant. Not, it is clear to me.
LS
The principal is then expanded to an enterprise principal in krb5_child if configured.
principal. We have empty cache collection. New ccache is created (after login). This new ccache has principal name "ptest@SSSD-AD.TEST"
Output from klist -l #> Principal name Cache name #> -------------- ---------- #> ptest@SSSD-AD.TEST DIR::/run/user/947005188/krb5cc/tktceNIlD
The principal form the cache (the canonicalized principal) is returned to the krb5 provider together with the ccache path and should be saved to the cache.
User decides to log on machine second time. Attribute ccacheFile has value "DIR:/run/user/947005188/krb5cc/", therefore we need find ccache in collection. With my patches, sssd uses function krb5_cc_cache_match to find ccache by principal. krb5_cc_cache_match internally uses krb5_principal_compare_flags to compare two principals and here is the problem
The second login should pick the principal from SSSD's cache which should be the one from above and hence the one from the ccache file. If configured the principal is expanded to an enterprise principal quite early in krb5_child. So you either make the comparison before the expansion, save the original principal in the context of the krb5_child and use it for the comparison or if possible do the comparison already in the krb5 provider.
HTH
bye, Sumit
By default, krb5_principal_compare_flags is called with flags argument 0. In this case following principals are compared
principal->data principal->realm
ptest SSSD-AD.TEST //Principal obtained from existing ccache ptest@FOO.BAR SSSD-AD.TEST //Principal created by parsing string
I tried to call krb5_principal_compare_flags with argument flags = KRB5_PRINCIPAL_COMPARE_ENTERPRISE and in this case following principals were compared.
principal->data principal->realm
ptest SSSD-AD.TEST //Principal obtained from existing ccache ptest FOO.BAR //Principal created by parsing string
This is a reason, why ccache can't be found in collection and new ccache is created every time.
LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Wed, Jun 26, 2013 at 02:54:19PM +0200, Lukas Slebodnik wrote:
On (26/06/13 10:56), Sumit Bose wrote:
On Wed, Jun 26, 2013 at 10:23:59AM +0200, Lukas Slebodnik wrote:
On (25/06/13 11:40), Jakub Hrozek wrote:
On Mon, Jun 24, 2013 at 10:54:30PM +0200, Lukas Slebodnik wrote:
On (24/06/13 22:06), Jakub Hrozek wrote:
On Sat, Jun 22, 2013 at 01:55:51PM +0200, Lukas Slebodnik wrote: > On (21/06/13 20:45), Jakub Hrozek wrote: > >On Thu, Jun 20, 2013 at 11:11:17AM +0200, Lukas Slebodnik wrote: > >> Rewritten patches are attached. > >> > >> LS > > > >Two nitpicks: > > > >> +static char * get_ccache_name_by_principal(TALLOC_CTX *mem_ctx, > >> + krb5_context ctx, > >> + krb5_principal principal, > >> + const char *ccname) > >> +{ > >> + krb5_error_code kerr; > >> + krb5_ccache tmp_cc = NULL; > >> + char *tmp_ccname = NULL; > >> + char *ret_ccname = NULL; > >> + > >> + kerr = krb5_cc_set_default_name(ctx, ccname); > >> + if (kerr != 0) { > >> + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); > >> + return NULL; > >> + } > >> + > >> + kerr = krb5_cc_cache_match(ctx, principal, &tmp_cc); > >> + if (kerr != 0) { > >> + KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr); > >> + return NULL; > >> + } > >> + > >> + kerr = krb5_cc_get_full_name(ctx, tmp_cc, &tmp_ccname); > >> + if (kerr !=0) { > >> + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); > >> + } > >> + > >> + ret_ccname = talloc_strdup(mem_ctx, tmp_ccname); > >> + if (ret_ccname == NULL) { > >> + DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed (ENOMEM).\n")); > >> + } > >> + > >> +done: > > > >This done label is unused and GCC emits a warning over that. It should > >be used if krb5_cc_get_full_name() fails. > > > >> + if (tmp_cc != NULL) { > >> + kerr = krb5_cc_close(ctx, tmp_cc); > >> + if (kerr != 0) { > >> + KRB5_CHILD_DEBUG(SSSDBG_MINOR_FAILURE, kerr); > >> + } > >> + } > >> + krb5_free_string(ctx, tmp_ccname); > >> + > >> + return ret_ccname; > >> +} > >> + > > > > > >> > >> + kerr = krb5_unparse_name(ctx, princ, &full_name); > >> + if (kerr !=0) { > >> + KRB5_CHILD_DEBUG(SSSDBG_TRACE_INTERNAL, kerr); > >> + goto done; > >> + } > >> + > >> + if (0 == strcmp(default_full_name, full_name)) { > >> + ret = true; > >> + } > >> + > > > >If you're changing the other patch, can you also add DEBUG message here > >saying what has been compared? > > > >> +done: > >> + if (default_cc != NULL) { > >> + kerr = krb5_cc_close(ctx, default_cc); > >> + if (kerr != 0) { > >> + KRB5_CHILD_DEBUG(SSSDBG_OP_FAILURE, kerr); > >> + goto done; > >> + } > >> + } > >> + > >> + /* all functions can be safely called with NULL. */ > >> + krb5_free_principal(ctx, default_princ); > >> + krb5_free_unparsed_name(ctx, default_full_name); > >> + krb5_free_unparsed_name(ctx, full_name); > >> + > >> + return ret; > >> +} > > > >So far I ran a number of tests against IPA. I should still check AD > >provider especially with enterprise principals to see if they still > >work. If they do I'll ack. > > > >But I will also file a ticket for 1.10.1 to move the checks to the > >backend so they are all done on one place as Sumit suggested earlier. > > > I removed checks in former updates. There left only mapping > from collection ccache (DIR:<cc_dir_name>) to ccache > (DIR::<cc_dir_name>/<ticket_name>) > > I tried some modifications on backend side (with gdb), but I could not get an aim > 1. store collection ccache name stored in sysdb > 2. environment variable KRB5CCNAME also contains collection ccache name > 3. do not have more ccaches with same principal name > > >I don't think it's necessary to delay 1.10.0 any longer for this issue, > >but it's the right thing to do eventually. > > Updated patches are attached. > > LS
I still see two issues, sorry:
- The cc_residual_is_used check seems to be broken:
[sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x1000): User [1983201109] is still active, reusing ccache [/run/user/1983201109]. [sssd[be[AD.EXAMPLE.COM]]] [cc_residual_is_used] (0x0200): Cache file [/run/user/1983201109/primary] does not exist, it will be recreated ^^^^^^^^^^^^^^^^^ This path cannot exist, I think SSSD should be checking for /run/user/1983201109/krb5cc/primary instead
[sssd[be[AD.EXAMPLE.COM]]] [check_for_valid_tgt] (0x0020): krb5_cc_retrieve_cred failed. [sssd[be[AD.EXAMPLE.COM]]] [fo_resolve_service_send] (0x0100): Trying to resolve service 'AD'
- When using enterprise credentials, multiple logins always get a new
ccache. I don't see this happening with origin/master, sorry.
I thought, that it is aim of cache collection to have separate ccache for different logins.
Yes, but these were two separate logins of the same principal. I've demonstrated the issue to Lukas in a VM to make sure we both see the problem.
I would like to describe situation, because I am not really sure, where is the problem.
String "ptest\@FOO.BAR@SSSD-AD.TEST" is sent to krb5_child with enterprise
An enterprise principal should never be send to the krb5_child. (There was an issue with AD, because AD expected the canonicalization flag explicitly set even for enterprise principals, which lead to enterprise principals stored in SSSD's cache, but this is already fixed) Either the principal from the SSSD cache or a constructed one (username@REALM) should be send to krb5_child and both should not be enterprise principals.
You are right, only "ptest@FOO.BAR" is sent to krb5_child and stored in kr->upn. Enterprise principal is generated in k5c_setup and stored in kr->name and this is the string, which I meant. Not, it is clear to me.
LS
The principal is then expanded to an enterprise principal in krb5_child if configured.
principal. We have empty cache collection. New ccache is created (after login). This new ccache has principal name "ptest@SSSD-AD.TEST"
Output from klist -l #> Principal name Cache name #> -------------- ---------- #> ptest@SSSD-AD.TEST DIR::/run/user/947005188/krb5cc/tktceNIlD
The principal form the cache (the canonicalized principal) is returned to the krb5 provider together with the ccache path and should be saved to the cache.
User decides to log on machine second time. Attribute ccacheFile has value "DIR:/run/user/947005188/krb5cc/", therefore we need find ccache in collection. With my patches, sssd uses function krb5_cc_cache_match to find ccache by principal. krb5_cc_cache_match internally uses krb5_principal_compare_flags to compare two principals and here is the problem
The second login should pick the principal from SSSD's cache which should be the one from above and hence the one from the ccache file. If configured the principal is expanded to an enterprise principal quite early in krb5_child. So you either make the comparison before the expansion, save the original principal in the context of the krb5_child and use it for the comparison or if possible do the comparison already in the krb5 provider.
HTH
bye, Sumit
With this iteration of the patches, all my testcases passed. I only see a typo in one of the comments that will be OK to fix before pushing.
ACK.
Great work, thank you for not giving up on the issue.
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Jakub Hrozek -
Lukas Slebodnik -
Simo Sorce -
Sumit Bose