[PATCH BUG 13478] krb5_wrap: fix keep_old_entries logic for older kerberos libraries

[Christof Schmitt]

On Thu, Jun 21, 2018 at 08:06:46PM +0300, Alexander Bokovoy wrote:
> On ti, 19 kesä 2018, Christof Schmitt via samba-technical wrote:
> > From 0814f429c311b315362d27fd884d3ea58f484210 Mon Sep 17 00:00:00 2001
> > From: Christof Schmitt <cs at samba.org>
> > Date: Tue, 19 Jun 2018 15:09:41 -0700
> > Subject: [PATCH] krb5_wrap: fix keep_old_entries logic for older kerberos
> >  libraries
> > 
> > MIT kerberos 1.13 and older only stores the 8 bits of the KVNO. The
> > change from commit 35b2fb4ff32 resulted in breakage for these kerberos
> > versions: 'net ads keytab create' reads a large KVNO from AD, and only
> > the lower 8 bits are stored. The next check then removed the entry again
> > as the 8bit value did not match the currently valid KVNO.
> > 
> > Fix this by limiting the check to only 8 bits.
> > 
> > BUG: https://bugzilla.samba.org/show_bug.cgi?id=13478
> > 
> > Signed-off-by: Christof Schmitt <cs at samba.org>
> > ---
> >  lib/krb5_wrap/krb5_samba.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/krb5_wrap/krb5_samba.c b/lib/krb5_wrap/krb5_samba.c
> > index f49838c..7e90913 100644
> > --- a/lib/krb5_wrap/krb5_samba.c
> > +++ b/lib/krb5_wrap/krb5_samba.c
> > @@ -1549,7 +1549,7 @@ krb5_error_code smb_krb5_kt_seek_and_delete_old_entries(krb5_context context,
> >  		}
> >  
> >  		if (!flush &&
> > -		    (kt_entry.vno == kvno) &&
> > +		    ((kt_entry.vno & 0xff) == (kvno & 0xff)) &&
> >  		    (kt_entry_enctype != enctype))
> >  		{
> >  			DEBUG(5, (__location__ ": Saving entry with kvno [%d] "
> > -- 
> > 1.8.3.1
> 
> A few lines above in the same function we already cut off to 8-bit since
> 2015.
> 
> RB+. Please push.

Thank you. I will push the patch.

> We need eventually to extend the code to handle 32-bit versions but this
> comment from 'more_recent()' helper in MIT krb5 does not make it optimistic:
> 
>     /*
>      * If a small kvno was written at the same time or later than a large kvno,
>      * the kvno probably wrapped at some boundary, so consider the small kvno
>      * more recent.  Wraparound can happen due to pre-1.14 keytab file format
>      * limitations (8-bit kvno storage), pre-1.14 kadmin protocol limitations
>      * (8-bit kvno marshalling), or KDB limitations (16-bit kvno storage).
>      */

It sounds like we might wait until kerberos 1.14 or later is in common
use, then the problem will be no longer relevant?

Christof