cli_credentials ccache semantics and test changes

[Andrew Bartlett]

On Wed, 2012-05-23 at 21:15 -0400, simo wrote:
> On Thu, 2012-05-24 at 10:55 +1000, Andrew Bartlett wrote: 
> > On Wed, 2012-05-23 at 18:47 +0200, Alexander Bokovoy wrote:
> > 
> > > - Log -----------------------------------------------------------------
> > > commit dcfb34fbb4b7484bdaa70fbe9ae9fd84738ab469
> > > Author: Alexander Bokovoy <ab at samba.org>
> > > Date:   Wed May 23 17:34:24 2012 +0300
> > > 
> > >     blackbox: fix samba4.blackbox.kinit test
> > >     
> > >     This deserves some explanation.
> > >     
> > >     With commit 518232d4578d700f5f5ea1609275a6cd1de3a1e7 samba4.blackbox.kinit test set
> > >     was wrapped with password settings reset before and after the tests with an idea to
> > >     maintain reliable state for the tests. As result, the resetting of the password
> > >     settings was done after the test that tried to use smbclient with a Kerberos ticket
> > >     obtained with machine account credentials.
> > >     
> > >     However, the code in credentials_krb5.c, function cli_credentials_get_client_gss_creds(),
> > >     never worked correctly when credentials were already in ccache. Instead, gensec_gssapi module
> > >     always re-kinited even if existing credentials were available in the ccache. This had an effect
> > >     on 'samba4.blackbox.kinit(dc:local).reset password policies(dc:local)' test equal to
> > >     never having initialized ccache at all, as if 'rm -f $KRB5CCNAME' was run before the test.
> > >     
> > >     When the issue of not using already initialized credentials from ccache was fixed with
> > >     d0aae88f1290e6a7a6d4bfc24aa62795e4892a31 'auth-credentials: Support using pre-fetched ccache
> > >     when obtaining kerberos credentials' commit, Samba 4 credentials library started to correctly
> > >     re-used already obtained credentials from ccaches. This caused failure of the test
> > >     'samba4.blackbox.kinit(dc:local).reset password policies(dc:local)' because machine account
> > >     has no permissions to modify password settings.
> > >     
> > >     Thus, the correct fix is to reset ccache state before performing the test.
> > >     
> > >     Autobuild-User: Alexander Bokovoy <ab at samba.org>
> > >     Autobuild-Date: Wed May 23 18:46:12 CEST 2012 on sn-devel-104
> > 
> > Alexander,
> > 
> > I'm really sorry, but this is not the right way to handle it.  Indeed,
> > the need to change this test (which was perfectly correct beforehand)
> > shows that the code change was incorrect.  We will need to revert both
> > of these, and I need to work with you more closely to sort out a way to
> > support your legitimate needs.
> 
> Andrew,
> please identify what is wrong with the change.
> 
> To the best of our knowledge the previous code was simply wrong and the
> test depended on wrong behavior.

Simo,

When making kerberos-related changes were you feel tests are wrong, or
which require long explanation, I would really appreciate having these
past me explicitly.  My expertise is available and have a long history
in this area.

In this specific case, the command run is:

testit "reset password policies" $VALGRIND $samba_tool domain
passwordsettings $PWSETCONFIG set --complexity=default
--history-length=default --min-pwd-length=default --min-pwd-age=default
--max-pwd-age=default || failed=`expr $failed + 1`

at this time: PWSETCONFIG=-H ldap://$SERVER -U$USERNAME%$PASSWORD

That is, a username is explicitly specified.  Therefore, this code must
ignore any credentials cache in the environment.  

To make this clearer, I've made a patch for the test_passwords.sh test
to demonstrate correct behaviour in a much clearer way.  I hope you can
understand why this shows the change made was not correct. 

> We did need the change to make things work with credentials obtained
> before the samba libraries were called. Without that change we were not
> able to reuse a perfectly valid ccache.

As I said, I'm very willing to work with you to ensure you retain this
outcome, however the change made not the correct way to achieve that.

As I recall, it was related to the way that the realm was passed in to
the python layer, and it being seen as being specified compared with the
credentials cache only being guessed.  I thought I had suggested making
it possible to specify the exact credentials cache you wanted to use at
the python layer with a new API like creds.set_named_ccache().  We can
also look at the way the realm specification is handled, particularly
when it matches the realm in the credentials cache.

I'm working with Alexander on this now, and hope we can sort it shortly.

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s4-selftest-Demonstrate-the-correct-behaviour-betwee.patch
Type: text/x-patch
Size: 1544 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120524/a8c7d45f/attachment.bin>