[PATCHSET] Samba AD with MIT Kerberos

Andrew Bartlett abartlet at samba.org
Mon Apr 24 20:11:53 UTC 2017


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




More information about the samba-technical mailing list