[PATCHSET] Samba AD with MIT Kerberos

Mon Apr 24 20:47:34 UTC 2017

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                             asn at samba.org
www.samba.org