[PATCH] Implement a more abstracted kpasswd service

Andreas Schneider asn at samba.org
Tue Sep 13 08:38:36 UTC 2016


On Friday, 9 September 2016 14:27:08 CEST Jeremy Allison wrote:
> On Fri, Sep 09, 2016 at 10:10:52AM +0200, Andreas Schneider wrote:
> > On Thursday, 8 September 2016 15:26:20 CEST Jeremy Allison wrote:
> > > On Wed, Sep 07, 2016 at 06:02:45PM +0200, Andreas Schneider wrote:
> > > > Hi Andrew,
> > > > 
> > > > I've implemented a working kpasswd service with MIT Kerberos in the
> > > > meantime. This patchset is the work to cleanup Heimdal code and
> > > > prepare
> > > > for later MIT Kerberos code.
> > > > 
> > > > I've started with splitting up the test_passwords.sh test. Now we have
> > > > a
> > > > test_password_settings.sh and test_kpasswd_heimdal.sh test.
> > > > 
> > > > The test_kpasswd_heimdal.sh only tests the kpasswd service
> > > > implementation
> > > > in different ways. It has some additional tests, like doing a password
> > > > change with kinit.
> > > > 
> > > > Next I reworked the kpasswd service implementation to be able to share
> > > > code
> > > > which is not Kerberos flavor specific.
> > > > 
> > > > The patchset is attached but you can also find it here:
> > > > 
> > > > https://git.samba.org/?p=asn/samba.git;a=shortlog;h=refs/heads/master-> > > > kpas
> > > > swd
> > > > 
> > > > 
> > > > Please review and push if appropriate!
> > > 
> > > Pushed the first 4 patches as they're obviously correct.
> > > 
> > > Still looking over the rest !
> > 
> > Thanks you very much Jeremy!
> 
> Pushed the next 8 with one change.
> 
> In:
> 
> [PATCH 11/18] s4-kdc: Move kpasswd_make_error_reply() to a helper file.
> 
> You have:
> 
> +       /*
> +        * The string 's' has two terminating nul-bytes which are also
> +        * reflected by 'slen'. Normally Kerberos doesn't expect that
> strings +        * are nul-terminated, but Heimdal does!
> +        */
> +#ifndef SAMBA4_USES_HEIMDAL
> +       slen -= 2;
> +#endif
> +       error_data->length = 2 + slen;
> 
> I changed this to:
> 
> +       /*
> +        * The string 's' has two terminating nul-bytes which are also
> +        * reflected by 'slen'. Normally Kerberos doesn't expect that
> strings +        * are nul-terminated, but Heimdal does!
> +        */
> +#ifndef SAMBA4_USES_HEIMDAL
> +       if (slen < 2) {
> +               return false;
> +       }
> +       slen -= 2;
> +#endif
> +       if (2 + slen < slen) {
> +               return false;
> +       }
> +       error_data->length = 2 + slen;
> 
> - note the extra checks on slen to prevent integer
> overflow/underflow.

This code leaks memory on error now :(

The attached patch addresses the issue.


Cheers,


	-- andreas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s4-kdc-Do-not-leak-memory-on-error-in-kpasswd_make_e.patch
Type: text/x-patch
Size: 839 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160913/96bb95dc/0001-s4-kdc-Do-not-leak-memory-on-error-in-kpasswd_make_e.bin>


More information about the samba-technical mailing list