Quantcast

[PATCHSET] Samba AD with MIT Kerberos

classic Classic list List threaded Threaded
48 messages Options
123
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
On Friday, 7 April 2017 11:50:21 CEST Andreas Schneider via samba-technical
wrote:

> > Presumably sn-devel would show likewise, but I try to avoid burning
> > cycles on that box.
>
> Hi Andrew,
>
> I run the test.
>
> Ubuntu 14.04 (same as autobuild)
> master-mit-kdc branch built with Heimdal
>
> asn@u1404:~/workspace/projects/samba> make -j test
> TESTS="samba4.blackbox.upgradeprovision.alpha13"
>
> [1(0)/1 at 0s] samba4.blackbox.upgradeprovision.alpha13
>
> ALL OK (15 tests in 1 testsuites)
>
>
> This still looks like a configure/gcc issue on your box ... :-)
>
>
> Cheers,
>
> Andreas

Rebased patchset on latest auth subsystem changes attached.


Cheers,


        Andreas

--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

master-mit-kdc.patch.txt (216K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
On Mon, 2017-04-10 at 07:59 +0200, Andreas Schneider wrote:

> On Friday, 7 April 2017 11:50:21 CEST Andreas Schneider via samba-
> technical 
> wrote:
> > > Presumably sn-devel would show likewise, but I try to avoid
> > > burning
> > > cycles on that box.
> >
> > Hi Andrew,
> >
> > I run the test.
> >
> > Ubuntu 14.04 (same as autobuild)
> > master-mit-kdc branch built with Heimdal
> >
> > asn@u1404:~/workspace/projects/samba> make -j test
> > TESTS="samba4.blackbox.upgradeprovision.alpha13"
> >
> > [1(0)/1 at 0s] samba4.blackbox.upgradeprovision.alpha13
> >
> > ALL OK (15 tests in 1 testsuites)
> >
> >
> > This still looks like a configure/gcc issue on your box ... :-)
> >
> >
> > Cheers,
> >
> > Andreas
>
>
> Rebased patchset on latest auth subsystem changes attached.

Thanks.  I'll kick off a build on the same system that caused the
trouble last time.

Andrew Bartlett

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
On Mon, 2017-04-10 at 18:17 +1200, Andrew Bartlett via samba-technical
wrote:

> On Mon, 2017-04-10 at 07:59 +0200, Andreas Schneider wrote:
> > On Friday, 7 April 2017 11:50:21 CEST Andreas Schneider via samba-
> > technical 
> > wrote:
> > > > Presumably sn-devel would show likewise, but I try to avoid
> > > > burning
> > > > cycles on that box.
> > >
> > > Hi Andrew,
> > >
> > > I run the test.
> > >
> > > Ubuntu 14.04 (same as autobuild)
> > > master-mit-kdc branch built with Heimdal
> > >
> > > asn@u1404:~/workspace/projects/samba> make -j test
> > > TESTS="samba4.blackbox.upgradeprovision.alpha13"
> > >
> > > [1(0)/1 at 0s] samba4.blackbox.upgradeprovision.alpha13
> > >
> > > ALL OK (15 tests in 1 testsuites)
> > >
> > >
> > > This still looks like a configure/gcc issue on your box ... :-)
> > >
> > >
> > > Cheers,
> > >
> > > Andreas
> >
> >
> > Rebased patchset on latest auth subsystem changes attached.
>
> Thanks.  I'll kick off a build on the same system that caused the
> trouble last time.

This time it filled the disk, but at least it got 3 hours into the
test, which is a pretty good start.

When I next come up for air I'll investigate what went on, and perhaps
get you a login if it isn't obvious...

Andrew Bartlett

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
On Monday, 10 April 2017 13:42:36 CEST Andrew Bartlett via samba-technical
wrote:

> On Mon, 2017-04-10 at 18:17 +1200, Andrew Bartlett via samba-technical
>
> wrote:
> > On Mon, 2017-04-10 at 07:59 +0200, Andreas Schneider wrote:
> > > On Friday, 7 April 2017 11:50:21 CEST Andreas Schneider via samba-
> > > technical
> > >
> > > wrote:
> > > > > Presumably sn-devel would show likewise, but I try to avoid
> > > > > burning
> > > > > cycles on that box.
> > > >
> > > > Hi Andrew,
> > > >
> > > > I run the test.
> > > >
> > > > Ubuntu 14.04 (same as autobuild)
> > > > master-mit-kdc branch built with Heimdal
> > > >
> > > > asn@u1404:~/workspace/projects/samba> make -j test
> > > > TESTS="samba4.blackbox.upgradeprovision.alpha13"
> > > >
> > > > [1(0)/1 at 0s] samba4.blackbox.upgradeprovision.alpha13
> > > >
> > > > ALL OK (15 tests in 1 testsuites)
> > > >
> > > >
> > > > This still looks like a configure/gcc issue on your box ... :-)
> > > >
> > > >
> > > > Cheers,
> > > >
> > > > Andreas
> > >
> > > Rebased patchset on latest auth subsystem changes attached.
> >
> > Thanks.  I'll kick off a build on the same system that caused the
> > trouble last time.
>
> This time it filled the disk, but at least it got 3 hours into the
> test, which is a pretty good start.
>
> When I next come up for air I'll investigate what went on, and perhaps
> get you a login if it isn't obvious...
Rebased version on latest master ...


        Andreas

--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

master-mit-kdc.patch.txt (215K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
Should something be added to the WHATSNEW.txt about this?

On Tue, Apr 18, 2017 at 8:10 AM, Andreas Schneider via samba-technical <
[hidden email]> wrote:

> On Monday, 10 April 2017 13:42:36 CEST Andrew Bartlett via samba-technical
> wrote:
> > On Mon, 2017-04-10 at 18:17 +1200, Andrew Bartlett via samba-technical
> >
> > wrote:
> > > On Mon, 2017-04-10 at 07:59 +0200, Andreas Schneider wrote:
> > > > On Friday, 7 April 2017 11:50:21 CEST Andreas Schneider via samba-
> > > > technical
> > > >
> > > > wrote:
> > > > > > Presumably sn-devel would show likewise, but I try to avoid
> > > > > > burning
> > > > > > cycles on that box.
> > > > >
> > > > > Hi Andrew,
> > > > >
> > > > > I run the test.
> > > > >
> > > > > Ubuntu 14.04 (same as autobuild)
> > > > > master-mit-kdc branch built with Heimdal
> > > > >
> > > > > asn@u1404:~/workspace/projects/samba> make -j test
> > > > > TESTS="samba4.blackbox.upgradeprovision.alpha13"
> > > > >
> > > > > [1(0)/1 at 0s] samba4.blackbox.upgradeprovision.alpha13
> > > > >
> > > > > ALL OK (15 tests in 1 testsuites)
> > > > >
> > > > >
> > > > > This still looks like a configure/gcc issue on your box ... :-)
> > > > >
> > > > >
> > > > > Cheers,
> > > > >
> > > > >         Andreas
> > > >
> > > > Rebased patchset on latest auth subsystem changes attached.
> > >
> > > Thanks.  I'll kick off a build on the same system that caused the
> > > trouble last time.
> >
> > This time it filled the disk, but at least it got 3 hours into the
> > test, which is a pretty good start.
> >
> > When I next come up for air I'll investigate what went on, and perhaps
> > get you a login if it isn't obvious...
>
> Rebased version on latest master ...
>
>
>         Andreas
>
> --
> Andreas Schneider                   GPG-ID: CC014E3D
> Samba Team                             [hidden email]
> www.samba.org
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Tue, Apr 18, 2017 at 04:10:59PM +0200, Andreas Schneider via samba-technical wrote:
> Rebased version on latest master ...

Andreas, is this a WIP, or would you like me to spend
some cycles doing a review with an eye towards merge ?

Jeremy.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
On Wednesday, 19 April 2017 02:23:41 CEST Jeremy Allison wrote:
> On Tue, Apr 18, 2017 at 04:10:59PM +0200, Andreas Schneider via samba-
technical wrote:
> > Rebased version on latest master ...
>
> Andreas, is this a WIP, or would you like me to spend
> some cycles doing a review with an eye towards merge ?

This is ready to go upstream.

Review is much appreciated!

I will write the WHATSNEW.txt entry once this is upstream ...


        Andreas

--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
The configure --help output does not explain the changed behaviour, it
still says:

  --with-system-mitkrb5
            enable system MIT krb5 build (includes Samba 4 client and
Samba 3 code base).You may specify list of paths where Kerberos is
installed (e.g. /usr/local
            /usr/kerberos) to search krb5-config


In this patch, the comment in the code still says 1 second:

commit 0711cd66419989fb6a22f4a6e7b67855981892c6
Author: Andreas Schneider <[hidden email]>
Date:   Mon Sep 26 18:51:33 2016 +0200

    selftest: Set clockskew grace time to 5 seconds
    
    Signed-off-by: Andreas Schneider <[hidden email]>

diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
index 3e5a7c3..e6f5ef8 100644
--- a/selftest/target/Samba.pm
+++ b/selftest/target/Samba.pm
@@ -201,6 +201,10 @@ sub mk_krb5_conf($$)
  ticket_lifetime = 24h
  forwardable = yes
  allow_weak_crypto = yes
+ # Set the grace clocskew to 1 second
+ # This is especially required by samba3.raw.session krb5 and
+ # reauth tests
+ clockskew = 5


When building on MIT 1.14.4 (on my Fedora laptop), I get:

ERROR: MIT krb5 build requires at least 1.14.4. 1.15.1 is found and
cannot be used
ERROR: You may try to build with embedded Heimdal Kerebros by not
specifying --with-system-mitkrb5
 
This is different to master, and is what caused the earlier autobuild
failure I mentioned on ubuntu 14.04 (in the Catalyst Cloud).  What I
can't find is which commit changed the AD DC to be on with --with-
system-mitkrb5.  

Additionally specifying --without-ad-dc doesn't help, and isn't
suggested in any case.  I think the default for --with-system-mitkrb5
should be --without-ad-dc for now.


commit 6e48c4ad9718f3ee6fbf78f7236105f2dfd9bdab
Author: Andreas Schneider <[hidden email]>
Date:   Fri Oct 9 15:06:52 2015 +0200

    python: Add provisioning support for MIT KDC in samba-tool
    
    Signed-off-by: Andreas Schneider <[hidden email]>
In these provision changes, you directly import _glue into domain.py
and python/samba/provision/kerberos.py.  Instead you should do like
with_ntvfs_fileserver, and go via samba/__init__.py.  

eg
python/samba/__init__.py:is_ntvfs_fileserver_built =
_glue.is_ntvfs_fileserver_built

It also seems to partially revert:

commit 04d8e0605f27d1fe57de05a9dba749ce36f7e004
Author: Andreas Schneider <[hidden email]>
Date:   Mon Nov 23 11:44:26 2015 +0100

    waf: Create kerberos_implementation.py for provisioning
    
    Signed-off-by: Andreas Schneider <[hidden email]>

We are getting closer, but some details still remain to get this right.
 I wish you the very best with the last few details.  Otherwise, I hope
we can find some time to knock this off at SambaXP.

Thanks,

Andrew Bartlett

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
On Wed, Apr 19, 2017 at 09:26:29PM +1200, Andrew Bartlett via samba-technical wrote:

> The configure --help output does not explain the changed behaviour, it
> still says:
>
>   --with-system-mitkrb5
>             enable system MIT krb5 build (includes Samba 4 client and
> Samba 3 code base).You may specify list of paths where Kerberos is
> installed (e.g. /usr/local
>             /usr/kerberos) to search krb5-config
>
>
> In this patch, the comment in the code still says 1 second:
>
> commit 0711cd66419989fb6a22f4a6e7b67855981892c6
> Author: Andreas Schneider <[hidden email]>
> Date:   Mon Sep 26 18:51:33 2016 +0200
>
>     selftest: Set clockskew grace time to 5 seconds
>     
>     Signed-off-by: Andreas Schneider <[hidden email]>
>
> diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
> index 3e5a7c3..e6f5ef8 100644
> --- a/selftest/target/Samba.pm
> +++ b/selftest/target/Samba.pm
> @@ -201,6 +201,10 @@ sub mk_krb5_conf($$)
>   ticket_lifetime = 24h
>   forwardable = yes
>   allow_weak_crypto = yes
> + # Set the grace clocskew to 1 second
> + # This is especially required by samba3.raw.session krb5 and
> + # reauth tests
> + clockskew = 5

FYI Andreas I'd already spotted the above :-).

> When building on MIT 1.14.4 (on my Fedora laptop), I get:
>
> ERROR: MIT krb5 build requires at least 1.14.4. 1.15.1 is found and
> cannot be used
> ERROR: You may try to build with embedded Heimdal Kerebros by not
> specifying --with-system-mitkrb5
>  
> This is different to master, and is what caused the earlier autobuild
> failure I mentioned on ubuntu 14.04 (in the Catalyst Cloud).  What I
> can't find is which commit changed the AD DC to be on with --with-
> system-mitkrb5.  
>
> Additionally specifying --without-ad-dc doesn't help, and isn't
> suggested in any case.  I think the default for --with-system-mitkrb5
> should be --without-ad-dc for now.
>
>
> commit 6e48c4ad9718f3ee6fbf78f7236105f2dfd9bdab
> Author: Andreas Schneider <[hidden email]>
> Date:   Fri Oct 9 15:06:52 2015 +0200
>
>     python: Add provisioning support for MIT KDC in samba-tool
>     
>     Signed-off-by: Andreas Schneider <[hidden email]>
> In these provision changes, you directly import _glue into domain.py
> and python/samba/provision/kerberos.py.  Instead you should do like
> with_ntvfs_fileserver, and go via samba/__init__.py.  
>
> eg
> python/samba/__init__.py:is_ntvfs_fileserver_built =
> _glue.is_ntvfs_fileserver_built
>
> It also seems to partially revert:
>
> commit 04d8e0605f27d1fe57de05a9dba749ce36f7e004
> Author: Andreas Schneider <[hidden email]>
> Date:   Mon Nov 23 11:44:26 2015 +0100
>
>     waf: Create kerberos_implementation.py for provisioning
>     
>     Signed-off-by: Andreas Schneider <[hidden email]>
>
> We are getting closer, but some details still remain to get this right.
>  I wish you the very best with the last few details.  Otherwise, I hope
> we can find some time to knock this off at SambaXP.

Can you fix up Andrew's issues and re-post a fixed
version for re-review !

Thanks,

Jeremy.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Wednesday, 19 April 2017 11:26:29 CEST Andrew Bartlett wrote:
> The configure --help output does not explain the changed behaviour, it
> still says:
>
>   --with-system-mitkrb5
>             enable system MIT krb5 build (includes Samba 4 client and
> Samba 3 code base).You may specify list of paths where Kerberos is
> installed (e.g. /usr/local
>             /usr/kerberos) to search krb5-config
>

Fixed, and also changed the test of the --without-ad-dc option. We need to
stop referring to Samba4 and Samba3 and use Samba AD and Samba FS!

> In this patch, the comment in the code still says 1 second:
>
> commit 0711cd66419989fb6a22f4a6e7b67855981892c6
> Author: Andreas Schneider <[hidden email]>
> Date:   Mon Sep 26 18:51:33 2016 +0200
>
>     selftest: Set clockskew grace time to 5 seconds
>    
>     Signed-off-by: Andreas Schneider <[hidden email]>
>
> diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
> index 3e5a7c3..e6f5ef8 100644
> --- a/selftest/target/Samba.pm
> +++ b/selftest/target/Samba.pm
> @@ -201,6 +201,10 @@ sub mk_krb5_conf($$)
>   ticket_lifetime = 24h
>   forwardable = yes
>   allow_weak_crypto = yes
> + # Set the grace clocskew to 1 second
> + # This is especially required by samba3.raw.session krb5 and
> + # reauth tests
> + clockskew = 5
>
Fixed.

>
> When building on MIT 1.14.4 (on my Fedora laptop), I get:
>
> ERROR: MIT krb5 build requires at least 1.14.4. 1.15.1 is found and
> cannot be used
> ERROR: You may try to build with embedded Heimdal Kerebros by not
> specifying --with-system-mitkrb5

OK, the printed versions are in the wrong order, fixed.

> This is different to master, and is what caused the earlier autobuild
> failure I mentioned on ubuntu 14.04 (in the Catalyst Cloud).  What I
> can't find is which commit changed the AD DC to be on with --with-
> system-mitkrb5.
>
> Additionally specifying --without-ad-dc doesn't help, and isn't
> suggested in any case.  I think the default for --with-system-mitkrb5
> should be --without-ad-dc for now.

Fixed, we need integer comparsion for versions and not strings.

 def parse_version(v):
-    return tuple(map(str, (v.split("."))))
+    return tuple(map(int, (v.split("."))))

This explains a lot :)

> commit 6e48c4ad9718f3ee6fbf78f7236105f2dfd9bdab
> Author: Andreas Schneider <[hidden email]>
> Date:   Fri Oct 9 15:06:52 2015 +0200
>
>     python: Add provisioning support for MIT KDC in samba-tool
>    
>     Signed-off-by: Andreas Schneider <[hidden email]>
> In these provision changes, you directly import _glue into domain.py
> and python/samba/provision/kerberos.py.  Instead you should do like
> with_ntvfs_fileserver, and go via samba/__init__.py.
>
> eg
> python/samba/__init__.py:is_ntvfs_fileserver_built =
> _glue.is_ntvfs_fileserver_built
Fixed.

> It also seems to partially revert:
>
> commit 04d8e0605f27d1fe57de05a9dba749ce36f7e004
> Author: Andreas Schneider <[hidden email]>
> Date:   Mon Nov 23 11:44:26 2015 +0100
>
>     waf: Create kerberos_implementation.py for provisioning
>    
>     Signed-off-by: Andreas Schneider <[hidden email]>
>
> We are getting closer, but some details still remain to get this right.
>  I wish you the very best with the last few details.  Otherwise, I hope
> we can find some time to knock this off at SambaXP.
Fixed.

Thanks for the review!


Updated patchset attached.


        Andreas


--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

master-mit-kdc_2017-04-19.patch.txt (216K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
On Thursday, 20 April 2017 16:13:25 CEST Andreas Schneider via samba-technical
wrote:
> Thanks for the review!
>
>
> Updated patchset attached.
>

Next week is SambaXP. Can we try to get this upstream *before* SambaXP?


Thanks,


        Andreas

--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Thu, 2017-04-20 at 16:13 +0200, Andreas Schneider via samba-
technical wrote:

> On Wednesday, 19 April 2017 11:26:29 CEST Andrew Bartlett wrote:
> > The configure --help output does not explain the changed behaviour,
> > it
> > still says:
> >
> >   --with-system-mitkrb5
> >             enable system MIT krb5 build (includes Samba 4 client
> > and
> > Samba 3 code base).You may specify list of paths where Kerberos is
> > installed (e.g. /usr/local
> >             /usr/kerberos) to search krb5-config
> >
>
> Fixed, and also changed the test of the --without-ad-dc option. We
> need to 
> stop referring to Samba4 and Samba3 and use Samba AD and Samba FS!

It still fails for me with:

Checking for gssapi                                                               : yes 
ERROR: MIT krb5 build requires at least 1.15.1. 1.14.4 is found and cannot be used
ERROR: You may try to build with embedded Heimdal Kerebros by not specifying --with-system-mitkrb5

This implies to me that you have not restored the behaviour to have --
without-ad-dc be the default when --with-system-mitkrb5 is set.  

There are these problems:
 - It means that builds that worked will now fail, until an extra
option is specified
 - The AD DC will be built for installations that previously didn't get
it
 - Nothing in the error message suggests that --without-ad-dc is a fix
for this error.

Please do as I requested, and again make --with-system-mitkrb5 imply --
without-ad-dc unless specified otherwise.  Then test that you can
./configure --with-system-mitkrb5 on Ubuntu 14.04 and Fedora 25.

There will be a time when this will also build the AD DC by default,
but that needs to be later, preferably once we sort out the remaining
features.

> > In this patch, the comment in the code still says 1 second:
> >
> > commit 0711cd66419989fb6a22f4a6e7b67855981892c6
> > Author: Andreas Schneider <[hidden email]>
> > Date:   Mon Sep 26 18:51:33 2016 +0200
> >
> >     selftest: Set clockskew grace time to 5 seconds
> >     
> >     Signed-off-by: Andreas Schneider <[hidden email]>
> >
> > diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
> > index 3e5a7c3..e6f5ef8 100644
> > --- a/selftest/target/Samba.pm
> > +++ b/selftest/target/Samba.pm
> > @@ -201,6 +201,10 @@ sub mk_krb5_conf($$)
> >   ticket_lifetime = 24h
> >   forwardable = yes
> >   allow_weak_crypto = yes
> > + # Set the grace clocskew to 1 second
> > + # This is especially required by samba3.raw.session krb5 and
> > + # reauth tests
> > + clockskew = 5
> >
>
> Fixed.
>
> >
> > When building on MIT 1.14.4 (on my Fedora laptop), I get:
> >
> > ERROR: MIT krb5 build requires at least 1.14.4. 1.15.1 is found and
> > cannot be used
> > ERROR: You may try to build with embedded Heimdal Kerebros by not
> > specifying --with-system-mitkrb5
>
> OK, the printed versions are in the wrong order, fixed.

Per the above, it wasn't just the order I was worried about.

> > This is different to master, and is what caused the earlier
> > autobuild
> > failure I mentioned on ubuntu 14.04 (in the Catalyst Cloud).  What
> > I
> > can't find is which commit changed the AD DC to be on with --with-
> > system-mitkrb5.
> >
> > Additionally specifying --without-ad-dc doesn't help, and isn't
> > suggested in any case.  I think the default for --with-system-
> > mitkrb5
> > should be --without-ad-dc for now.
>
> Fixed, we need integer comparsion for versions and not strings.
>
>  def parse_version(v):
> -    return tuple(map(str, (v.split("."))))
> +    return tuple(map(int, (v.split("."))))
>
> This explains a lot :)
>
> > commit 6e48c4ad9718f3ee6fbf78f7236105f2dfd9bdab
> > Author: Andreas Schneider <[hidden email]>
> > Date:   Fri Oct 9 15:06:52 2015 +0200
> >
> >     python: Add provisioning support for MIT KDC in samba-tool
> >     
> >     Signed-off-by: Andreas Schneider <[hidden email]>
> > In these provision changes, you directly import _glue into
> > domain.py
> > and python/samba/provision/kerberos.py.  Instead you should do like
> > with_ntvfs_fileserver, and go via samba/__init__.py.
> >
> > eg
> > python/samba/__init__.py:is_ntvfs_fileserver_built =
> > _glue.is_ntvfs_fileserver_built
>
> Fixed.
>
> > It also seems to partially revert:
> >
> > commit 04d8e0605f27d1fe57de05a9dba749ce36f7e004
> > Author: Andreas Schneider <[hidden email]>
> > Date:   Mon Nov 23 11:44:26 2015 +0100
> >
> >     waf: Create kerberos_implementation.py for provisioning
> >     
> >     Signed-off-by: Andreas Schneider <[hidden email]>
> >
> > We are getting closer, but some details still remain to get this
> > right.
> >  I wish you the very best with the last few details.  Otherwise, I
> > hope
> > we can find some time to knock this off at SambaXP.
>
> Fixed.
>
> Thanks for the review!
>
>
> Updated patchset attached.

I'm sorry that I can't give you a review quite yet.  Additionally,
Jeremy's war on talloc_autofree() has broken the patch set against
master.  It isn't a hard fix, but I know it will be frustrating.

I'm sure we can move forward on some of the patches, however this patch
is at the front of the set.  If you can get me a set of un-dependent
patches it may be easier to filter those in, and reduce the queue size.

Sorry,

Andrew Bartlett

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
On Monday, 24 April 2017 13:44:27 CEST Andrew Bartlett via samba-technical
wrote:

> On Thu, 2017-04-20 at 16:13 +0200, Andreas Schneider via samba-
>
> technical wrote:
> > On Wednesday, 19 April 2017 11:26:29 CEST Andrew Bartlett wrote:
> > > The configure --help output does not explain the changed behaviour,
> > > it
> > > still says:
> > >
> > >   --with-system-mitkrb5
> > >             enable system MIT krb5 build (includes Samba 4 client
> > > and
> > > Samba 3 code base).You may specify list of paths where Kerberos is
> > > installed (e.g. /usr/local
> > >             /usr/kerberos) to search krb5-config
> >
> > Fixed, and also changed the test of the --without-ad-dc option. We
> > need to
> > stop referring to Samba4 and Samba3 and use Samba AD and Samba FS!
>
> It still fails for me with:
>
> Checking for
> gssapi                                                               : yes
> ERROR: MIT krb5 build requires at least 1.15.1. 1.14.4 is found and cannot
> be used ERROR: You may try to build with embedded Heimdal Kerebros by not
> specifying --with-system-mitkrb5
Yes, this looks correct to me.

> This implies to me that you have not restored the behaviour to have --
> without-ad-dc be the default when --with-system-mitkrb5 is set.

Yes, by default the DC should be built, the same way as it is with Heimdal
too. If you do not want to build the DC you should run configure with:

        --without-ad-dc

That's why we have that option. It should not be the other way around. Else we
would have to add another option with does the opposite and if we remove it it
would be confusing twice.

> There are these problems:
>  - It means that builds that worked will now fail, until an extra
> option is specified

It means that we just require a newer version to build Samba with MIT Kerberos
by default. The option --without-ad-dc is already used by packages which build
Samba with MIT Kerberos, e.g. SUSE and Fedora.

I've added the following warning to the configure error:

ERROR: If you want to just build Samba FS use the option --without-ad-dc

>  - The AD DC will be built for installations that previously didn't get
> it
>  - Nothing in the error message suggests that --without-ad-dc is a fix
> for this error.

See rebased attached patchset.
 
> Please do as I requested, and again make --with-system-mitkrb5 imply --
> without-ad-dc unless specified otherwise.  Then test that you can
> ./configure --with-system-mitkrb5 on Ubuntu 14.04 and Fedora 25.
>
> There will be a time when this will also build the AD DC by default,
> but that needs to be later, preferably once we sort out the remaining
> features.

This will be much more confusing when the option set changes later and not now
when we add it. Distributions building Samba with MIT Kerberos already use --
without-ad-dc.

Also this is not half baked unstable code. This is very well tested code which
is mature and we want to make progress. This code revealed so many issues in
Samba AD. Most of the time I fixed bugs in the AD DC which are not MIT
Kerberos related at all. Just look what Metze and I worked on over the last
year.

Rewrite of libsmb/cliconnect.c to pass down cli credentials to gensec
A lot of gensec fixes
Trusted domain fixes

> I'm sorry that I can't give you a review quite yet.  Additionally,
> Jeremy's war on talloc_autofree() has broken the patch set against
> master.  It isn't a hard fix, but I know it will be frustrating.

See attached patchset.

I rebase this patchset since several years every few days now! Most of the
time this patchset had between 70 and 120 patches. We already sent smaller
junks to get them upstream the last years. This is the final one.


Attached is a rebased patchset on master.


        Andreas

--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

master-mit-kdc_2017-04-24.patch.txt (219K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
On Mon, 2017-04-24 at 14:36 +0200, Andreas Schneider wrote:

> On Monday, 24 April 2017 13:44:27 CEST Andrew Bartlett via samba-
> technical 
> wrote:
> > On Thu, 2017-04-20 at 16:13 +0200, Andreas Schneider via samba-
> >
> > technical wrote:
> > > On Wednesday, 19 April 2017 11:26:29 CEST Andrew Bartlett wrote:
> > > > The configure --help output does not explain the changed
> > > > behaviour,
> > > > it
> > > > still says:
> > > >
> > > >   --with-system-mitkrb5
> > > >             enable system MIT krb5 build (includes Samba 4
> > > > client
> > > > and
> > > > Samba 3 code base).You may specify list of paths where Kerberos
> > > > is
> > > > installed (e.g. /usr/local
> > > >             /usr/kerberos) to search krb5-config
> > >
> > > Fixed, and also changed the test of the --without-ad-dc option.
> > > We
> > > need to 
> > > stop referring to Samba4 and Samba3 and use Samba AD and Samba
> > > FS!
> >
> > It still fails for me with:
> >
> > Checking for
> > gssapi                                                             
> >   : yes 
> > ERROR: MIT krb5 build requires at least 1.15.1. 1.14.4 is found and
> > cannot
> > be used ERROR: You may try to build with embedded Heimdal Kerebros
> > by not
> > specifying --with-system-mitkrb5
>
> Yes, this looks correct to me.
>
> > This implies to me that you have not restored the behaviour to have
> > --
> > without-ad-dc be the default when --with-system-mitkrb5 is set.
>
> Yes, by default the DC should be built, the same way as it is with
> Heimdal 
> too. If you do not want to build the DC you should run configure
> with:
>
> --without-ad-dc
>
> That's why we have that option. It should not be the other way
> around. Else we 
> would have to add another option with does the opposite and if we
> remove it it 
> would be confusing twice.

Then you need to either:
 - patch our autobuild scripts to pass that option for the samba-
systemkrb5 target or
 - Include the correct version of MIT Krb5 in third_party

I'm sorry, but you will NOT get my positive review until I can
reproduce an autobuild on a standard apt-get installed Ubuntu 14.04.  I
ran a build on Ubuntu 14.04 last night on your previous patch set and
got:

[1569/3076] Compiling source4/torture/nbench/nbench.c
../source4/torture/krb5/kdc-mit.c: In function
torture_krb5_init_context:
../source4/torture/krb5/kdc-mit.c:532:2: error: implicit declaration of
function krb5_set_kdc_send_hook [-Werror=implicit-function-declaration]
  krb5_set_kdc_send_hook((*smb_krb5_context)->krb5_context,
  ^
../source4/torture/krb5/kdc-mit.c:536:2: error: implicit declaration of
function krb5_set_kdc_recv_hook [-Werror=implicit-function-declaration]
  krb5_set_kdc_recv_hook((*smb_krb5_context)->krb5_context,
  ^
cc1: all warnings being treated as errors

This seems harsh, but otherwise we remove the support for travis-ci
builds, which are the only way that folks outside the Samba Team can
easily ensure they don't waste our time by submitting patches that
don't even compile.   Some really important work (like the python3 and
no-python patches) critically relied on this.

Likewise, my team at Catalyst does all our internal builds on fresh
Ubuntu 14.04 machines using the Catalyst cloud and our cloud-autobuild
scripts.  Now, these are easier to move to 16.04 but it will only take
one bump to the required MIT krb5 version and we will be back in this
pickle.

To be clear, it is NOT enough that autobuild.py only runs on sn-devel!
Indeed, a good deal of our back-and-forth seems to be due to this
issue.

In the meantime, please do as I request so we can get the rest of this
in!  Patches to change the required versions / what is built are much
easier to rebase!

> Also this is not half baked unstable code. This is very well tested
> code which 
> is mature and we want to make progress. This code revealed so many
> issues in 
> Samba AD. Most of the time I fixed bugs in the AD DC which are not
> MIT 
> Kerberos related at all. Just look what Metze and I worked on over
> the last 
> year.

See the above.  You asked for review before SambaXP.  If you can give a
little on this, we can get the rest of this in, and discuss the rest
there.  That will be much easier and more productive than you holding
out on this.

> Rewrite of libsmb/cliconnect.c to pass down cli credentials to gensec
> A lot of gensec fixes
> Trusted domain fixes

I don't doubt the work you have done, and for that reason I'm doing
what I can to get this merged.  I know it is highly frustrating!

> > I'm sorry that I can't give you a review quite yet.  Additionally,
> > Jeremy's war on talloc_autofree() has broken the patch set against
> > master.  It isn't a hard fix, but I know it will be frustrating.
>
> See attached patchset.
>
> I rebase this patchset since several years every few days now! Most
> of the 
> time this patchset had between 70 and 120 patches. We already sent
> smaller 
> junks to get them upstream the last years. This is the final one.

If you don't mind, then please continue to accept my push-back.  I was
just trying to find a way forward for you.

Finally, what I do think this shows is that we need a, public build
platform where trusted and (even more importantly) semi-trusted users
can proposed Samba changes that are automatically tested.  We need to
maintain that so it provides what we need to build Samba, and it needs
to be attractive enough for team members to use.  

Just sn-devel is not enough, but likewise github and travis-ci isn't
used by team members, so nobody notices if it is broken!

In the meantime, if you could push your changes to a github branch, and
tweak it until it passes the build there, it would be most appreciated.

I'm running a new build of your latest patch on Ubuntu 14.04 and 16.04
for you now, but I don't like being a manual test agent, so it would be
helpful if you could take that part over.

Thanks,

Andrew Bartlett
--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
On Monday, 24 April 2017 22:11:53 CEST Andrew Bartlett wrote:

> On Mon, 2017-04-24 at 14:36 +0200, Andreas Schneider wrote:
> > On Monday, 24 April 2017 13:44:27 CEST Andrew Bartlett via samba-
> > technical
> >
> > wrote:
> > > On Thu, 2017-04-20 at 16:13 +0200, Andreas Schneider via samba-
> > >
> > > technical wrote:
> > > > On Wednesday, 19 April 2017 11:26:29 CEST Andrew Bartlett wrote:
> > > > > The configure --help output does not explain the changed
> > > > > behaviour,
> > > > > it
> > > > > still says:
> > > > >
> > > > >   --with-system-mitkrb5
> > > > >             enable system MIT krb5 build (includes Samba 4
> > > > > client
> > > > > and
> > > > > Samba 3 code base).You may specify list of paths where Kerberos
> > > > > is
> > > > > installed (e.g. /usr/local
> > > > >             /usr/kerberos) to search krb5-config
> > > >
> > > > Fixed, and also changed the test of the --without-ad-dc option.
> > > > We
> > > > need to
> > > > stop referring to Samba4 and Samba3 and use Samba AD and Samba
> > > > FS!
> > >
> > > It still fails for me with:
> > >
> > > Checking for
> > > gssapi                                                            
> > >   : yes
> > > ERROR: MIT krb5 build requires at least 1.15.1. 1.14.4 is found and
> > > cannot
> > > be used ERROR: You may try to build with embedded Heimdal Kerebros
> > > by not
> > > specifying --with-system-mitkrb5
> >
> > Yes, this looks correct to me.
> >
> > > This implies to me that you have not restored the behaviour to have
> > > --
> > > without-ad-dc be the default when --with-system-mitkrb5 is set.
> >
> > Yes, by default the DC should be built, the same way as it is with
> > Heimdal
> > too. If you do not want to build the DC you should run configure
> >
> > with:
> > --without-ad-dc
> >
> > That's why we have that option. It should not be the other way
> > around. Else we
> > would have to add another option with does the opposite and if we
> > remove it it
> > would be confusing twice.
>
> Then you need to either:
>  - patch our autobuild scripts to pass that option for the samba-
> systemkrb5 target or
>  - Include the correct version of MIT Krb5 in third_party

172                       ("configure", "./configure.developer " +
samba_configure_params + " --with-system-mitkrb5 --without-ad-dc", "text/
plain"),

script/autobuild.py already sets --without-ad-dc.

See 9e98ac05c24bac7e49821160b0b3330fb95b68c2

> I'm sorry, but you will NOT get my positive review until I can
> reproduce an autobuild on a standard apt-get installed Ubuntu 14.04.  I
> ran a build on Ubuntu 14.04 last night on your previous patch set and
> got:
>
> [1569/3076] Compiling source4/torture/nbench/nbench.c
> ../source4/torture/krb5/kdc-mit.c: In function
> torture_krb5_init_context:
> ../source4/torture/krb5/kdc-mit.c:532:2: error: implicit declaration of
> function krb5_set_kdc_send_hook [-Werror=implicit-function-declaration]
>   krb5_set_kdc_send_hook((*smb_krb5_context)->krb5_context,
>   ^
> ../source4/torture/krb5/kdc-mit.c:536:2: error: implicit declaration of
> function krb5_set_kdc_recv_hook [-Werror=implicit-function-declaration]
>   krb5_set_kdc_recv_hook((*smb_krb5_context)->krb5_context,
>   ^
> cc1: all warnings being treated as errors

This looks like a different problem. I will reproduce it on Ubuntu and fix it.
Thanks for pointing that out.

> This seems harsh, but otherwise we remove the support for travis-ci
> builds, which are the only way that folks outside the Samba Team can
> easily ensure they don't waste our time by submitting patches that
> don't even compile.   Some really important work (like the python3 and
> no-python patches) critically relied on this.
>
> Likewise, my team at Catalyst does all our internal builds on fresh
> Ubuntu 14.04 machines using the Catalyst cloud and our cloud-autobuild
> scripts.  Now, these are easier to move to 16.04 but it will only take
> one bump to the required MIT krb5 version and we will be back in this
> pickle.
>
> To be clear, it is NOT enough that autobuild.py only runs on sn-devel!
> Indeed, a good deal of our back-and-forth seems to be due to this
> issue.
>
> In the meantime, please do as I request so we can get the rest of this
> in!  Patches to change the required versions / what is built are much
> easier to rebase!
>
> > Also this is not half baked unstable code. This is very well tested
> > code which
> > is mature and we want to make progress. This code revealed so many
> > issues in
> > Samba AD. Most of the time I fixed bugs in the AD DC which are not
> > MIT
> > Kerberos related at all. Just look what Metze and I worked on over
> > the last
> > year.
>
> See the above.  You asked for review before SambaXP.  If you can give a
> little on this, we can get the rest of this in, and discuss the rest
> there.  That will be much easier and more productive than you holding
> out on this.
>
> > Rewrite of libsmb/cliconnect.c to pass down cli credentials to gensec
> > A lot of gensec fixes
> > Trusted domain fixes
>
> I don't doubt the work you have done, and for that reason I'm doing
> what I can to get this merged.  I know it is highly frustrating!
>
> > > I'm sorry that I can't give you a review quite yet.  Additionally,
> > > Jeremy's war on talloc_autofree() has broken the patch set against
> > > master.  It isn't a hard fix, but I know it will be frustrating.
> >
> > See attached patchset.
> >
> > I rebase this patchset since several years every few days now! Most
> > of the
> > time this patchset had between 70 and 120 patches. We already sent
> > smaller
> > junks to get them upstream the last years. This is the final one.
>
> If you don't mind, then please continue to accept my push-back.  I was
> just trying to find a way forward for you.

If you point me to issues I will happily address them. I will reproduce that
issue and fix it tomorrow.

> Finally, what I do think this shows is that we need a, public build
> platform where trusted and (even more importantly) semi-trusted users
> can proposed Samba changes that are automatically tested.  We need to
> maintain that so it provides what we need to build Samba, and it needs
> to be attractive enough for team members to use.
>
> Just sn-devel is not enough, but likewise github and travis-ci isn't
> used by team members, so nobody notices if it is broken!
>
> In the meantime, if you could push your changes to a github branch, and
> tweak it until it passes the build there, it would be most appreciated.
>
> I'm running a new build of your latest patch on Ubuntu 14.04 and 16.04
> for you now, but I don't like being a manual test agent, so it would be
> helpful if you could take that part over.

Well, often I'm the one who fixes the build on openSUSE or Fedora because most
people only use Ubuntu and never run their code on other Linux distributions.
So I'm the one producing patches to fix those issues and keep it working.
However, finger pointing doesn't help us, working together does :-)

New patchset with the issues addressed will follow tomorrow.


Thanks for the review, much appreciated.


        Andreas


--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Automated CI systems

Samba - samba-technical mailing list
On Mon, 2017-04-24 at 22:47 +0200, Andreas Schneider wrote:
>
> Well, often I'm the one who fixes the build on openSUSE or Fedora
> because most 
> people only use Ubuntu and never run their code on other Linux
> distributions. 
> So I'm the one producing patches to fix those issues and keep it
> working. 
> However, finger pointing doesn't help us, working together does :-)

Indeed.  I would love to have a gitlab CE driven (so free software, so
we actually use it) implementation of gitlab CI, running on our
allocation of cloud credit with Rackspace.

I've wanted to do this for ages, but have made no progress, but it
someone could find the time to build and maintain this, we could help
get past this silly situation where we all pass around patches that
fail autobuilds.  Ideally by the time another developer looks at the
patch the failure would be indicated, and we can all avoid wasting
time!

We could even include multiple target platforms.

> New patchset with the issues addressed will follow tomorrow.

Thanks!

> Thanks for the review, much appreciated.

I'm really glad to get this moving.  Not much further now!

Andrew Bartlett

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Monday, 13 March 2017 18:40:21 CEST Andrew Bartlett wrote:
> On Mon, 2017-03-13 at 08:29 +0100, Andreas Schneider via samba-
>
> technical wrote:
> > Hello,
> >
> > after more than 3 years of work I finally got this:
> > ALL OK (14658 tests in 2030 testsuites)
> >
> > The testsuite completed for the first time!

Here is a new rebased version which fixes the issues with the KDC tests if we
do not build the AD DC.

On Ubuntu 14.04 I got:

./script/autobuild.py --testbase=/tmp samba-systemkrb5
All OK

So it successfully passed. I have a private autobuild running too now.


Thanks for your help to address these issues.


Cheers,


        Andreas


--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

master-mit-kdc_2017-04-25.patch.txt (219K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
On Tuesday, 25 April 2017 10:26:06 CEST Andreas Schneider via samba-technical
wrote:

> On Monday, 13 March 2017 18:40:21 CEST Andrew Bartlett wrote:
> > On Mon, 2017-03-13 at 08:29 +0100, Andreas Schneider via samba-
> >
> > technical wrote:
> > > Hello,
> > >
> > > after more than 3 years of work I finally got this:
> > > ALL OK (14658 tests in 2030 testsuites)
> > >
> > > The testsuite completed for the first time!
>
> Here is a new rebased version which fixes the issues with the KDC tests if
> we do not build the AD DC.
>
> On Ubuntu 14.04 I got:
>
> ./script/autobuild.py --testbase=/tmp samba-systemkrb5
> All OK
>
> So it successfully passed. I have a private autobuild running too now.
>
>
> Thanks for your help to address these issues.

I've found another issue in a common test function. The attached patchset
succeded in a private autobuild:


    Your autobuild on sn-devel-144 has succeeded after 244.0 minutes.


Please review.


Thanks,


        Andreas


--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

master-mit-kdc_2017-04-25_16-23.patch.txt (219K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
On Tue, Apr 25, 2017 at 04:25:46PM +0200, Andreas Schneider via samba-technical wrote:

> On Tuesday, 25 April 2017 10:26:06 CEST Andreas Schneider via samba-technical
> wrote:
> > On Monday, 13 March 2017 18:40:21 CEST Andrew Bartlett wrote:
> > > On Mon, 2017-03-13 at 08:29 +0100, Andreas Schneider via samba-
> > >
> > > technical wrote:
> > > > Hello,
> > > >
> > > > after more than 3 years of work I finally got this:
> > > > ALL OK (14658 tests in 2030 testsuites)
> > > >
> > > > The testsuite completed for the first time!
> >
> > Here is a new rebased version which fixes the issues with the KDC tests if
> > we do not build the AD DC.
> >
> > On Ubuntu 14.04 I got:
> >
> > ./script/autobuild.py --testbase=/tmp samba-systemkrb5
> > All OK
> >
> > So it successfully passed. I have a private autobuild running too now.
> >
> >
> > Thanks for your help to address these issues.
>
>
> I've found another issue in a common test function. The attached patchset
> succeded in a private autobuild:
>
>
>     Your autobuild on sn-devel-144 has succeeded after 244.0 minutes.
>
>
> Please review.

Just a few minor nits I've found so far.



In [PATCH 10/51] s4-kdc: Add a MIT Kerberos KDC service:

+/*                
+   Unix SMB/CIFS implementation.
+
+   run s3 file server within Samba4

Should be:

+++ b/source4/kdc/kdc-service-mit.c
@@ -0,0 +1,120 @@
+/*
+   Unix SMB/CIFS implementation.
+
+   run MIT krb5 server within Samba4
+

Also:

+NTSTATUS server_service_mitkdc_init(void);
+
+NTSTATUS server_service_mitkdc_init(void)
+{
+       return register_server_service("kdc", mitkdc_task_init);
+}

needs to be:

+NTSTATUS server_service_mitkdc_init(TALLOC_CTX *);
+
+NTSTATUS server_service_mitkdc_init(TALLOC_CTX *ctx)
+{
+       return register_server_service("kdc", mitkdc_task_init);
+}

Sorry :-(.
--------------------------------------------------

In [PATCH 11/51] s4-kdc: Add MIT KRB5 based irpc service for PAC validation

+NTSTATUS samba_setup_mit_kdc_irpc(struct task_server *task)
+{
+       struct samba_kdc_base_context base_ctx;
+       struct mit_kdc_irpc_context *mki_ctx;
+       NTSTATUS status;
+       int code;
+
+       mki_ctx = talloc_zero(task, struct mit_kdc_irpc_context);

Missing check for NULL return from talloc_zero

--------------------------------------------------

In [PATCH 34/51] s4-kdc: Start the kpasswd service with MIT KDC

+       tmp_ctx = talloc_new(mem_ctx);
+       if (tmp_ctx == NULL) {
+               return NT_STATUS_NO_MEMORY;
+       }


Use talloc_named_const() instead so we have a good name
for this context.

Cheers,

        Jeremy.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCHSET] Samba AD with MIT Kerberos

Samba - samba-technical mailing list
On Tuesday, 25 April 2017 22:39:39 CEST Jeremy Allison wrote:
> >     Your autobuild on sn-devel-144 has succeeded after 244.0 minutes.
> >
> > Please review.
>
> Just a few minor nits I've found so far.
>

Thank you very much, updated patchset which addresses these things attached.



        Andreas

--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

master-mit-kdc_2017-04-26.patch.txt (219K) Download Attachment
123
Loading...