[PATCH] Detect re-created trusts on an IPA server

[Jakub Hrozek]

Hi,

the attached patches fix https://fedorahosted.org/sssd/ticket/2639

The first patches always retry fetching the keytab on lookup failure.

For the last two patches, I used Sumit's idea to check the last connection
time of the AD LDAP connection and compare it with the TDO modifyTimestamp
to only fetch the keytab and retry if there is a newer trust.

Testing is a bit tricky, but what I did is pretty much:
    $ getent passwd [e-mail]
    [e-mail]:*:962400500:962400500:Administrator:/home/win.trust.test/administrator:
    # Make sure sssd doesn't reuse already established and working
    # connection. Somewhat simulates GSSAPI connection expiring
    $ sudo pkill -USR1 sssd
    $ sudo pkill -USR2 sssd
    $ ipa trust-add --type=ad win.trust.test --admin Administrator --password
    # Make sure a wrong keytab is in place for the next connection
    # attempt
    $ sudo cp old.keytab /var/lib/sss/keytabs/win.trust.test.keytab
    $ sudo sss_cache -E
    $ getent passwd [e-mail]
    [e-mail]:*:962400500:962400500:Administrator:/home/win.trust.test/administrator:

[Jakub Hrozek]

On Wed, Sep 23, 2015 at 10:09:40AM +0200, Jakub Hrozek wrote:
> Hi,
>
> the attached patches fix https://fedorahosted.org/sssd/ticket/2639
>
> The first patches always retry fetching the keytab on lookup failure.
>
> For the last two patches, I used Sumit's idea to check the last connection
> time of the AD LDAP connection and compare it with the TDO modifyTimestamp
> to only fetch the keytab and retry if there is a newer trust.
>
> Testing is a bit tricky, but what I did is pretty much:
>     $ getent passwd [e-mail]
>     [e-mail]:*:962400500:962400500:Administrator:/home/win.trust.test/administrator:
>     # Make sure sssd doesn't reuse already established and working
>     # connection. Somewhat simulates GSSAPI connection expiring
>     $ sudo pkill -USR1 sssd
>     $ sudo pkill -USR2 sssd
>     $ ipa trust-add --type=ad win.trust.test --admin Administrator --password
>     # Make sure a wrong keytab is in place for the next connection
>     # attempt
>     $ sudo cp old.keytab /var/lib/sss/keytabs/win.trust.test.keytab
>     $ sudo sss_cache -E
>     $ getent passwd [e-mail]
>     [e-mail]:*:962400500:962400500:Administrator:/home/win.trust.test/administrator:

Hi,

after IRC discussion with Sumit, I'm withdrawing the last two patches.
At least for now, maybe even until we get nicer error reporting from
sdap. It seems the time-based keytab check would be fragile and it would
be better to rely on error code that would indicate the keytab can't be
used.

Instead, the patch "FO: Also reset the server common data in addition to
SRV" has more tests (actually it's more of refactoring of the current
test to make adding new ones easier, the new test is then trivial).

No other changes were done to the patchset.

In the FO patch, it's easy to verify the fix with:
    - git rebase, edit the fix
    - git reset HEAD~1 src/providers/fail_over.c
    - make check # Will fail
    - git checkout src/providers/fail_over.c
    - make check # Will pass

Since the test mostly cycles through servers, resets them and cycles again,
which is something we can expect from failover users, I think this proves
the fix well.

[Sumit Bose]

On Wed, Sep 23, 2015 at 03:01:42PM +0200, Jakub Hrozek wrote:
> On Wed, Sep 23, 2015 at 10:09:40AM +0200, Jakub Hrozek wrote:
> > Hi,
> >
> > the attached patches fix https://fedorahosted.org/sssd/ticket/2639
> >
> > The first patches always retry fetching the keytab on lookup failure.
> >
> > For the last two patches, I used Sumit's idea to check the last connection
> > time of the AD LDAP connection and compare it with the TDO modifyTimestamp
> > to only fetch the keytab and retry if there is a newer trust.
> >
> > Testing is a bit tricky, but what I did is pretty much:
> >     $ getent passwd [e-mail]
> >     [e-mail]:*:962400500:962400500:Administrator:/home/win.trust.test/administrator:
> >     # Make sure sssd doesn't reuse already established and working
> >     # connection. Somewhat simulates GSSAPI connection expiring
> >     $ sudo pkill -USR1 sssd
> >     $ sudo pkill -USR2 sssd
> >     $ ipa trust-add --type=ad win.trust.test --admin Administrator --password
> >     # Make sure a wrong keytab is in place for the next connection
> >     # attempt
> >     $ sudo cp old.keytab /var/lib/sss/keytabs/win.trust.test.keytab
> >     $ sudo sss_cache -E
> >     $ getent passwd [e-mail]
> >     [e-mail]:*:962400500:962400500:Administrator:/home/win.trust.test/administrator:
>
> Hi,
>
> after IRC discussion with Sumit, I'm withdrawing the last two patches.
> At least for now, maybe even until we get nicer error reporting from
> sdap. It seems the time-based keytab check would be fragile and it would
> be better to rely on error code that would indicate the keytab can't be
> used.
>
> Instead, the patch "FO: Also reset the server common data in addition to
> SRV" has more tests (actually it's more of refactoring of the current
> test to make adding new ones easier, the new test is then trivial).
>
> No other changes were done to the patchset.
>
> In the FO patch, it's easy to verify the fix with:
>     - git rebase, edit the fix
>     - git reset HEAD~1 src/providers/fail_over.c
>     - make check # Will fail
>     - git checkout src/providers/fail_over.c
>     - make check # Will pass
>
> Since the test mostly cycles through servers, resets them and cycles again,
> which is something we can expect from failover users, I think this proves
> the fix well.

Hi,

the 4 patches look good, work as expected and pass CI
http://sssd-ci.duckdns.org/logs/job/27/65/summary.html, ACK.

bye,
Sumit

[Jakub Hrozek]

On Wed, Sep 23, 2015 at 06:16:13PM +0200, Sumit Bose wrote:
> On Wed, Sep 23, 2015 at 03:01:42PM +0200, Jakub Hrozek wrote:
> > On Wed, Sep 23, 2015 at 10:09:40AM +0200, Jakub Hrozek wrote:
> > > Hi,
> > >
> > > the attached patches fix https://fedorahosted.org/sssd/ticket/2639
> > >
> > > The first patches always retry fetching the keytab on lookup failure.
> > >
> > > For the last two patches, I used Sumit's idea to check the last connection
> > > time of the AD LDAP connection and compare it with the TDO modifyTimestamp
> > > to only fetch the keytab and retry if there is a newer trust.
> > >
> > > Testing is a bit tricky, but what I did is pretty much:
> > >     $ getent passwd [e-mail]
> > >     [e-mail]:*:962400500:962400500:Administrator:/home/win.trust.test/administrator:
> > >     # Make sure sssd doesn't reuse already established and working
> > >     # connection. Somewhat simulates GSSAPI connection expiring
> > >     $ sudo pkill -USR1 sssd
> > >     $ sudo pkill -USR2 sssd
> > >     $ ipa trust-add --type=ad win.trust.test --admin Administrator --password
> > >     # Make sure a wrong keytab is in place for the next connection
> > >     # attempt
> > >     $ sudo cp old.keytab /var/lib/sss/keytabs/win.trust.test.keytab
> > >     $ sudo sss_cache -E
> > >     $ getent passwd [e-mail]
> > >     [e-mail]:*:962400500:962400500:Administrator:/home/win.trust.test/administrator:
> >
> > Hi,
> >
> > after IRC discussion with Sumit, I'm withdrawing the last two patches.
> > At least for now, maybe even until we get nicer error reporting from
> > sdap. It seems the time-based keytab check would be fragile and it would
> > be better to rely on error code that would indicate the keytab can't be
> > used.
> >
> > Instead, the patch "FO: Also reset the server common data in addition to
> > SRV" has more tests (actually it's more of refactoring of the current
> > test to make adding new ones easier, the new test is then trivial).
> >
> > No other changes were done to the patchset.
> >
> > In the FO patch, it's easy to verify the fix with:
> >     - git rebase, edit the fix
> >     - git reset HEAD~1 src/providers/fail_over.c
> >     - make check # Will fail
> >     - git checkout src/providers/fail_over.c
> >     - make check # Will pass
> >
> > Since the test mostly cycles through servers, resets them and cycles again,
> > which is something we can expect from failover users, I think this proves
> > the fix well.
>
> Hi,
>
> the 4 patches look good, work as expected and pass CI
> http://sssd-ci.duckdns.org/logs/job/27/65/summary.html, ACK.

Thank you for the quick review:
    * 4c53f8b7400630ae06459aa8b5079427edcaa348
    * 669ce24f8157b7d79914b3eb5a18214ef42aacc8
    * bc58e1cfee742178f95922d964349d6c262f6df7
    * 42bd89dbe77846b6ee60365bba50da521745bca1