[PATCH] Create a 'binddns dir' for files used by the bind_dlz module and named

Andreas Schneider asn at samba.org
Fri Aug 25 11:24:29 UTC 2017


On Friday, 25 August 2017 07:50:43 CEST Andreas Schneider via samba-technical 
wrote:
> On Thursday, 24 August 2017 23:32:49 CEST Jeremy Allison wrote:
> > On Thu, Aug 24, 2017 at 07:57:07AM +0200, Andreas Schneider wrote:
> > > Yes, this fixes the test. Updated patchset attached.
> > > 
> > > 
> > > Thanks for the review!
> > 
> > Andreas, Andrew already pushed the first 3 patches, and
> > when I add the following 6 I get:
> > 
> > [1(0)/2192 at 0s] samba.tests.source
> > [2(6)/2192 at 0s] samba.tests.docs
> > [3(8)/2192 at 2m13s] samba.tests.blackbox.ndrdump
> > [4(13)/2192 at 2m13s] ldb.python
> > [5(171)/2192 at 2m14s] samba.tests.credentials
> > [6(207)/2192 at 2m14s] samba.tests.credentials.python3
> > [7(243)/2192 at 2m15s] samba.tests.registry
> > [8(247)/2192 at 2m15s] samba.tests.auth
> > [9(252)/2192 at 2m15s] samba.tests.auth.python3
> > [10(257)/2192 at 2m15s] samba.tests.get_opt
> > [11(261)/2192 at 2m15s] samba.tests.get_opt.python3
> > [12(265)/2192 at 2m15s] samba.tests.security
> > [13(286)/2192 at 2m16s] samba.tests.dcerpc.misc
> > [14(293)/2192 at 2m16s] samba.tests.dcerpc.misc.python3
> > [15(300)/2192 at 2m16s] samba.tests.dcerpc.integer
> > [16(335)/2192 at 2m18s] samba.tests.param
> > [17(348)/2192 at 2m19s] samba.tests.param.python3
> > [18(361)/2192 at 2m20s] samba.tests.upgrade
> > [19(363)/2192 at 2m20s] samba.tests.core
> > [20(372)/2192 at 2m21s] samba.tests.core.python3
> > [21(381)/2192 at 2m21s] samba.tests.provision
> > UNEXPECTED(error):
> > samba.tests.provision.samba.tests.provision.ProvisionTestCase.test_setup_s
> > e
> > cretsdb(none) REASON: Exception: Exception: Traceback (most recent call
> > 
> > last):
> >   File "/memdisk/jra/a/b112403/samba/bin/python/samba/tests/provision.py",
> > 
> > line 64, in test_setup_secretsdb ldb = setup_secretsdb(paths, None, None,
> > lp=env_loadparm())
> > 
> >   File
> > 
> > "/memdisk/jra/a/b112403/samba/bin/python/samba/provision/__init__.py",
> > line
> > 952, in setup_secretsdb dns_keytab_path = os.path.join(paths.binddns_dir,
> > paths.dns_keytab) File "/usr/lib/python2.7/posixpath.py", line 77, in join
> > 
> >     elif path == '' or path.endswith('/'):
> > AttributeError: 'NoneType' object has no attribute 'endswith'
> > 
> > FAILED (0 failures, 1 errors and 0 unexpected successes in 0 testsuites)
> > 
> > A summary with detailed information can be found in:
> >   ./bin/ab/summary
> > 
> > Can you take a look ?
> > 
> > > Andreas Schneider                   GPG-ID: CC014E3D
> > > Samba Team                             asn at samba.org
> > > www.samba.org
> > > 
> > > From 9c8a7c0c8b81b0390d9d3756e94909998f98ef1b Mon Sep 17 00:00:00 2001
> > > From: Andreas Schneider <asn at samba.org>
> > > Date: Thu, 10 Aug 2017 11:36:52 +0200
> > > Subject: [PATCH 1/9] wafsamba: Add INSTALL_DIR function
> > > 
> > > The install_dir function in waf has been deprecated and it doesn't
> > > support setting directory permissions. So we need to implement our own
> > > function anyway.
> > > 
> > > BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957
> > > 
> > > Signed-off-by: Andreas Schneider <asn at samba.org>
> > > Reviewed-by: Andrew Bartlet <abartlet at samba.org>
> > > ---
> > > 
> > >  buildtools/wafsamba/wafsamba.py | 18 ++++++++++++++++++
> > >  1 file changed, 18 insertions(+)
> > > 
> > > diff --git a/buildtools/wafsamba/wafsamba.py
> > > b/buildtools/wafsamba/wafsamba.py index 1bdabf60640..b1e617916e0 100644
> > > --- a/buildtools/wafsamba/wafsamba.py
> > > +++ b/buildtools/wafsamba/wafsamba.py
> > > @@ -885,6 +885,24 @@ def INSTALL_WILDCARD(bld, destdir, pattern,
> > > chmod=MODE_644, flat=False,>
> > > 
> > >                    python_fixup=python_fixup, base_name=trim_path)
> > >  
> > >  Build.BuildContext.INSTALL_WILDCARD = INSTALL_WILDCARD
> > > 
> > > +def INSTALL_DIR(bld, path, chmod=0o755):
> > > +    """Install a directory if it doesn't exist, always set
> > > permissions."""
> > > +
> > > +    if not path:
> > > +        return []
> > > +
> > > +    if bld.is_install > 0:
> > > +        path = bld.EXPAND_VARIABLES(path)
> > > +        if not os.path.isdir(path):
> > > +            try:
> > > +                os.makedirs(path)
> > > +                os.chmod(path, chmod)
> > > +            except OSError, e:
> > > +                if not os.path.isdir(path):
> > > +                    raise Utils.WafError("Cannot create the folder '%s'
> > > (error: %s)" % (path, e)) +        else:
> > > +            os.chmod(path, chmod)
> > > +Build.BuildContext.INSTALL_DIR = INSTALL_DIR
> > > 
> > >  def INSTALL_DIRS(bld, destdir, dirs):
> > >      '''install a set of directories'''

Ok, this one should be working :-)


Passed 'make test' more or less locally. Unrelated tests failed like the gpgme 
tests.


	Andreas


-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org
-------------- next part --------------
>From 8c31c821e8736227ff3cd6b3620196876dd23f77 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 10 Aug 2017 11:43:11 +0200
Subject: [PATCH 1/6] dynconfig: Change permission of the private dir to 0700

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957

Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Andrew Bartlet <abartlet at samba.org>
---
 dynconfig/wscript                  | 2 +-
 python/samba/provision/__init__.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/dynconfig/wscript b/dynconfig/wscript
index 7e9bde929d0..ba0c896b90e 100644
--- a/dynconfig/wscript
+++ b/dynconfig/wscript
@@ -418,7 +418,7 @@ def build(bld):
     bld.INSTALL_DIR("${CONFIGDIR}")
     bld.INSTALL_DIR("${LOGFILEBASE}")
     bld.INSTALL_DIR("${PRIVILEGED_SOCKET_DIR}")
-    bld.INSTALL_DIR("${PRIVATE_DIR}")
+    bld.INSTALL_DIR("${PRIVATE_DIR}", 0o700)
     bld.INSTALL_DIR("${STATEDIR}")
     bld.INSTALL_DIR("${CACHEDIR}")
 
diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
index 2387931987e..91d2105929c 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -2065,7 +2065,7 @@ def provision(logger, session_info, smbconf=None,
         serverrole = lp.get("server role")
 
     if not os.path.exists(paths.private_dir):
-        os.mkdir(paths.private_dir)
+        os.mkdir(paths.private_dir, 0o700)
     if not os.path.exists(os.path.join(paths.private_dir, "tls")):
         os.makedirs(os.path.join(paths.private_dir, "tls"), 0700)
     if not os.path.exists(paths.state_dir):
-- 
2.14.1


>From 2cfe913a1c82ef583204440c05c6fee568c4b069 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 11 Aug 2017 12:45:14 +0200
Subject: [PATCH 2/6] python:samba: Remove code to change group

This is the wrong place, it will just prepare the ldif. The file is not
created here.

The code is corrently changing the group in:
    python/samba/provision/__init__.py

Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Andrew Bartlet <abartlet at samba.org>
---
 python/samba/provision/sambadns.py | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/python/samba/provision/sambadns.py b/python/samba/provision/sambadns.py
index 961f37e16a6..dcb19c7053c 100644
--- a/python/samba/provision/sambadns.py
+++ b/python/samba/provision/sambadns.py
@@ -1199,16 +1199,6 @@ def setup_bind9_dns(samdb, secretsdb, names, paths, lp, logger,
                         dns_keytab_path=paths.dns_keytab, dnspass=dnspass,
                         key_version_number=key_version_number)
 
-    dns_keytab_path = os.path.join(paths.private_dir, paths.dns_keytab)
-    if os.path.isfile(dns_keytab_path) and paths.bind_gid is not None:
-        try:
-            os.chmod(dns_keytab_path, 0640)
-            os.chown(dns_keytab_path, -1, paths.bind_gid)
-        except OSError:
-            if not os.environ.has_key('SAMBA_SELFTEST'):
-                logger.info("Failed to chown %s to bind gid %u",
-                            dns_keytab_path, paths.bind_gid)
-
     create_dns_dir(logger, paths)
 
     if dns_backend == "BIND9_FLATFILE":
-- 
2.14.1


>From 4a7101e4ff1b8e9b69b8f826d754349e4d94e5af Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 10 Aug 2017 15:04:08 +0200
Subject: [PATCH 3/6] param: Add 'binddns dir' parameter

This allows to us to have restricted acess to the directory by the group
'named' which bind is a member of.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957

Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Andrew Bartlet <abartlet at samba.org>
---
 buildtools/wafsamba/samba_patterns.py       |  1 +
 docs-xml/smbdotconf/generate-file-list.sh   |  1 +
 docs-xml/smbdotconf/security/binddnsdir.xml | 18 ++++++++++++++++++
 dynconfig/dynconfig.c                       |  1 +
 dynconfig/dynconfig.h                       |  1 +
 dynconfig/wscript                           |  7 +++++++
 lib/param/loadparm.c                        |  1 +
 lib/param/param.h                           |  1 +
 source3/param/loadparm.c                    |  2 ++
 9 files changed, 33 insertions(+)
 create mode 100644 docs-xml/smbdotconf/security/binddnsdir.xml

diff --git a/buildtools/wafsamba/samba_patterns.py b/buildtools/wafsamba/samba_patterns.py
index e809f26a095..2b939372fa4 100644
--- a/buildtools/wafsamba/samba_patterns.py
+++ b/buildtools/wafsamba/samba_patterns.py
@@ -108,6 +108,7 @@ def write_build_options_header(fp):
     fp.write("       output(screen,\"   PIDDIR: %s\\n\", get_dyn_PIDDIR());\n")
     fp.write("       output(screen,\"   SMB_PASSWD_FILE: %s\\n\",get_dyn_SMB_PASSWD_FILE());\n")
     fp.write("       output(screen,\"   PRIVATE_DIR: %s\\n\",get_dyn_PRIVATE_DIR());\n")
+    fp.write("       output(screen,\"   BINDDNS_DIR: %s\\n\",get_dyn_BINDDNS_DIR());\n")
     fp.write("\n")
 
 def write_build_options_footer(fp):
diff --git a/docs-xml/smbdotconf/generate-file-list.sh b/docs-xml/smbdotconf/generate-file-list.sh
index 4a25f1e6d49..7ab1b7caf76 100755
--- a/docs-xml/smbdotconf/generate-file-list.sh
+++ b/docs-xml/smbdotconf/generate-file-list.sh
@@ -11,6 +11,7 @@ echo "<!DOCTYPE section [
 <!ENTITY pathconfig.PIDDIR               '\${prefix}/var/run'>
 <!ENTITY pathconfig.STATEDIR             '\${prefix}/var/locks'>
 <!ENTITY pathconfig.PRIVATE_DIR          '\${prefix}/private'>
+<!ENTITY pathconfig.BINDDNS_DIR          '\${prefix}/bind-dns'>
 <!ENTITY pathconfig.SMB_PASSWD_FILE      '\${prefix}/private/smbpasswd'>
 <!ENTITY pathconfig.WINBINDD_SOCKET_DIR  '\${prefix}/var/run/winbindd'>
 <!ENTITY pathconfig.CACHEDIR             '\${prefix}/var/cache'>
diff --git a/docs-xml/smbdotconf/security/binddnsdir.xml b/docs-xml/smbdotconf/security/binddnsdir.xml
new file mode 100644
index 00000000000..c296a0ef81d
--- /dev/null
+++ b/docs-xml/smbdotconf/security/binddnsdir.xml
@@ -0,0 +1,18 @@
+<samba:parameter name="binddns dir"
+                 context="G"
+                 type="string"
+                 constant="1"
+                 xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
+<synonym>bind dns directory</synonym>
+<description>
+    <para>
+        This parameters defines the directory samba will use to store the configuration
+        files for bind, such as named.conf.
+
+        NOTE: The bind dns directory needs to be on the same mount point as the private
+        directory!
+    </para>
+</description>
+
+<value type="default">&pathconfig.BINDDNS_DIR;</value>
+</samba:parameter>
diff --git a/dynconfig/dynconfig.c b/dynconfig/dynconfig.c
index e75d7db553a..e70a10f8cfe 100644
--- a/dynconfig/dynconfig.c
+++ b/dynconfig/dynconfig.c
@@ -95,6 +95,7 @@ DEFINE_DYN_CONFIG_PARAM(PIDDIR)
 DEFINE_DYN_CONFIG_PARAM(NCALRPCDIR)
 DEFINE_DYN_CONFIG_PARAM(SMB_PASSWD_FILE)
 DEFINE_DYN_CONFIG_PARAM(PRIVATE_DIR)
+DEFINE_DYN_CONFIG_PARAM(BINDDNS_DIR)
 DEFINE_DYN_CONFIG_PARAM(LOCALEDIR)
 DEFINE_DYN_CONFIG_PARAM(NMBDSOCKETDIR)
 DEFINE_DYN_CONFIG_PARAM(DATADIR)
diff --git a/dynconfig/dynconfig.h b/dynconfig/dynconfig.h
index 4d07c103d74..bdab2e8f242 100644
--- a/dynconfig/dynconfig.h
+++ b/dynconfig/dynconfig.h
@@ -46,6 +46,7 @@ DEFINE_DYN_CONFIG_PROTO(PIDDIR)
 DEFINE_DYN_CONFIG_PROTO(NCALRPCDIR)
 DEFINE_DYN_CONFIG_PROTO(SMB_PASSWD_FILE)
 DEFINE_DYN_CONFIG_PROTO(PRIVATE_DIR)
+DEFINE_DYN_CONFIG_PROTO(BINDDNS_DIR)
 DEFINE_DYN_CONFIG_PROTO(LOCALEDIR)
 DEFINE_DYN_CONFIG_PROTO(NMBDSOCKETDIR)
 DEFINE_DYN_CONFIG_PROTO(DATADIR)
diff --git a/dynconfig/wscript b/dynconfig/wscript
index ba0c896b90e..fee37eaaf5f 100644
--- a/dynconfig/wscript
+++ b/dynconfig/wscript
@@ -192,6 +192,12 @@ dynconfig = {
          'OPTION':    '--with-statedir',
          'HELPTEXT':  'Where to put persistent state files',
     },
+    'BINDDNS_DIR' : {
+         'STD-PATH':  '${LOCALSTATEDIR}/lib',
+         'FHS-PATH':  '${LOCALSTATEDIR}/lib/samba/bind-dns',
+         'OPTION':    '--with-bind-dns-dir',
+         'HELPTEXT':  'bind-dns config directory',
+    },
     'CACHEDIR' : {
          'STD-PATH':  '${LOCALSTATEDIR}/cache',
          'FHS-PATH':  '${LOCALSTATEDIR}/cache/samba',
@@ -419,6 +425,7 @@ def build(bld):
     bld.INSTALL_DIR("${LOGFILEBASE}")
     bld.INSTALL_DIR("${PRIVILEGED_SOCKET_DIR}")
     bld.INSTALL_DIR("${PRIVATE_DIR}", 0o700)
+    bld.INSTALL_DIR("${BINDDNS_DIR}", 0o770)
     bld.INSTALL_DIR("${STATEDIR}")
     bld.INSTALL_DIR("${CACHEDIR}")
 
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index a221e879d07..b91f9657f1c 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -2655,6 +2655,7 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx)
 	/* the winbind method for domain controllers is for both RODC
 	   auth forwarding and for trusted domains */
 	lpcfg_do_global_parameter(lp_ctx, "private dir", dyn_PRIVATE_DIR);
+	lpcfg_do_global_parameter(lp_ctx, "binddns dir", dyn_BINDDNS_DIR);
 	lpcfg_do_global_parameter(lp_ctx, "registry:HKEY_LOCAL_MACHINE", "hklm.ldb");
 
 	/* This hive should be dynamically generated by Samba using
diff --git a/lib/param/param.h b/lib/param/param.h
index 589b8906db5..680c053a6cc 100644
--- a/lib/param/param.h
+++ b/lib/param/param.h
@@ -56,6 +56,7 @@ const char **lpcfg_interfaces(struct loadparm_context *);
 const char *lpcfg_realm(struct loadparm_context *);
 const char *lpcfg_netbios_name(struct loadparm_context *);
 const char *lpcfg_private_dir(struct loadparm_context *);
+const char *lpcfg_binddns_dir(struct loadparm_context *);
 int lpcfg_server_role(struct loadparm_context *);
 int lpcfg_allow_dns_updates(struct loadparm_context *);
 
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index d5b1c56e21e..42e579efcfd 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -550,6 +550,8 @@ static void init_globals(struct loadparm_context *lp_ctx, bool reinit_globals)
 			 get_dyn_SMB_PASSWD_FILE());
 	lpcfg_string_set(Globals.ctx, &Globals.private_dir,
 			 get_dyn_PRIVATE_DIR());
+	lpcfg_string_set(Globals.ctx, &Globals.binddns_dir,
+			 get_dyn_BINDDNS_DIR());
 
 	/* use the new 'hash2' method by default, with a prefix of 1 */
 	lpcfg_string_set(Globals.ctx, &Globals.mangling_method, "hash2");
-- 
2.14.1


>From e2968a52ae475a845e399e7db2f6d8a9ca6eaaf1 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Tue, 22 Aug 2017 17:10:01 +0200
Subject: [PATCH 4/6] s4:bind_dlz: Use the 'binddns dir' if possible

The code makes sure we are backwards compatible. It will first check if
we still have files in the private directory, if yes it will use those.

If the the file is not in the private directory it will try the binddns
dir.

Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Andrew Bartlet <abartlet at samba.org>
---
 selftest/target/Samba3.pm       |  1 +
 selftest/target/Samba4.pm       |  1 +
 source4/dns_server/dlz_bind9.c  | 44 ++++++++++++++++++++++++++++++++++++---
 source4/dsdb/dns/dns_update.c   | 46 ++++++++++++++++++++++++++++++++++++++---
 source4/torture/dns/dlz_bind9.c | 26 ++++++++++++++++-------
 5 files changed, 104 insertions(+), 14 deletions(-)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 54da52b7fa2..d8836d397bc 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1672,6 +1672,7 @@ sub provision($$$$$$$$$)
 	workgroup = $domain
 
 	private dir = $privatedir
+	binddns dir = $privatedir
 	pid directory = $piddir
 	lock directory = $lockdir
 	log file = $logdir/log.\%m
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 205e2813db6..d7f2e211d29 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -584,6 +584,7 @@ sub provision_raw_step1($$)
 	workgroup = $ctx->{domain}
 	realm = $ctx->{realm}
 	private dir = $ctx->{privatedir}
+	binddns dir = $ctx->{privatedir}
 	pid directory = $ctx->{piddir}
 	ncalrpc dir = $ctx->{ncalrpcdir}
 	lock dir = $ctx->{lockdir}
diff --git a/source4/dns_server/dlz_bind9.c b/source4/dns_server/dlz_bind9.c
index 7fec9423924..75a9ce0f648 100644
--- a/source4/dns_server/dlz_bind9.c
+++ b/source4/dns_server/dlz_bind9.c
@@ -682,11 +682,23 @@ _PUBLIC_ isc_result_t dlz_create(const char *dlzname,
 	}
 
 	if (state->options.url == NULL) {
-		state->options.url = lpcfg_private_path(state, state->lp, "dns/sam.ldb");
+		state->options.url = lpcfg_private_path(state,
+							state->lp,
+							"dns/sam.ldb");
 		if (state->options.url == NULL) {
 			result = ISC_R_NOMEMORY;
 			goto failed;
 		}
+
+		if (!file_exist(state->options.url)) {
+			state->options.url = talloc_asprintf(state,
+							     "%s/dns/sam.ldb",
+							     lpcfg_binddns_dir(state->lp));
+			if (state->options.url == NULL) {
+				result = ISC_R_NOMEMORY;
+				goto failed;
+			}
+		}
 	}
 
 	state->samdb = samdb_connect_url(state, state->ev_ctx, state->lp,
@@ -1266,6 +1278,7 @@ _PUBLIC_ isc_boolean_t dlz_ssumatch(const char *signer, const char *name, const
 	DATA_BLOB ap_req;
 	struct cli_credentials *server_credentials;
 	char *keytab_name;
+	char *keytab_file = NULL;
 	int ret;
 	int ldb_ret;
 	NTSTATUS nt_status;
@@ -1309,8 +1322,33 @@ _PUBLIC_ isc_boolean_t dlz_ssumatch(const char *signer, const char *name, const
 	cli_credentials_set_krb5_context(server_credentials, state->smb_krb5_ctx);
 	cli_credentials_set_conf(server_credentials, state->lp);
 
-	keytab_name = talloc_asprintf(tmp_ctx, "FILE:%s/dns.keytab",
-					lpcfg_private_dir(state->lp));
+	keytab_file = talloc_asprintf(tmp_ctx,
+				      "%s/dns.keytab",
+				      lpcfg_private_dir(state->lp));
+	if (keytab_file == NULL) {
+		state->log(ISC_LOG_ERROR, "samba_dlz: Out of memory!");
+		talloc_free(tmp_ctx);
+		return ISC_FALSE;
+	}
+
+	if (!file_exist(keytab_file)) {
+		keytab_file = talloc_asprintf(tmp_ctx,
+					      "%s/dns.keytab",
+					      lpcfg_binddns_dir(state->lp));
+		if (keytab_file == NULL) {
+			state->log(ISC_LOG_ERROR, "samba_dlz: Out of memory!");
+			talloc_free(tmp_ctx);
+			return ISC_FALSE;
+		}
+	}
+
+	keytab_name = talloc_asprintf(tmp_ctx, "FILE:%s", keytab_file);
+	if (keytab_name == NULL) {
+		state->log(ISC_LOG_ERROR, "samba_dlz: Out of memory!");
+		talloc_free(tmp_ctx);
+		return ISC_FALSE;
+	}
+
 	ret = cli_credentials_set_keytab_name(server_credentials, state->lp, keytab_name,
 						CRED_SPECIFIED);
 	if (ret != 0) {
diff --git a/source4/dsdb/dns/dns_update.c b/source4/dsdb/dns/dns_update.c
index f74256d95ea..ba8431a3d1d 100644
--- a/source4/dsdb/dns/dns_update.c
+++ b/source4/dsdb/dns/dns_update.c
@@ -170,16 +170,56 @@ static void dnsupdate_rebuild(struct dnsupdate_service *service)
 
 	path = lpcfg_parm_string(service->task->lp_ctx, NULL, "dnsupdate", "path");
 	if (path == NULL) {
-		path = lpcfg_private_path(tmp_ctx, service->task->lp_ctx, "named.conf.update");
+		path = lpcfg_private_path(tmp_ctx,
+					  service->task->lp_ctx,
+					  "named.conf.update");
+		if (path == NULL) {
+			DBG_ERR("Out of memory!");
+			talloc_free(tmp_ctx);
+			return;
+		}
+
+		/*
+		 * If the file doesn't exist, we provisioned in a the new
+		 * bind-dns directory
+		 */
+		if (!file_exist(path)) {
+			path = talloc_asprintf(tmp_ctx,
+					       "%s/named.conf.update",
+					       lpcfg_binddns_dir(service->task->lp_ctx));
+			if (path == NULL) {
+				DBG_ERR("Out of memory!");
+				talloc_free(tmp_ctx);
+				return;
+			}
+		}
 	}
 
 	path_static = lpcfg_parm_string(service->task->lp_ctx, NULL, "dnsupdate", "extra_static_grant_rules");
 	if (path_static == NULL) {
-		path_static = lpcfg_private_path(tmp_ctx, service->task->lp_ctx, "named.conf.update.static");
+		path_static = lpcfg_private_path(tmp_ctx,
+						 service->task->lp_ctx,
+						 "named.conf.update.static");
+		if (path_static == NULL) {
+			DBG_ERR("Out of memory!");
+			talloc_free(tmp_ctx);
+			return;
+		}
+
+		if (!file_exist(path_static)) {
+			path_static = talloc_asprintf(tmp_ctx,
+						      "%s/named.conf.update.static",
+						      lpcfg_binddns_dir(service->task->lp_ctx));
+			if (path_static == NULL) {
+				DBG_ERR("Out of memory!");
+				talloc_free(tmp_ctx);
+				return;
+			}
+		}
 	}
 
 	tmp_path = talloc_asprintf(tmp_ctx, "%s.tmp", path);
-	if (path == NULL || tmp_path == NULL || path_static == NULL ) {
+	if (tmp_path == NULL) {
 		DEBUG(0,(__location__ ": Unable to get paths\n"));
 		talloc_free(tmp_ctx);
 		return;
diff --git a/source4/torture/dns/dlz_bind9.c b/source4/torture/dns/dlz_bind9.c
index c29f26802f5..893158fa730 100644
--- a/source4/torture/dns/dlz_bind9.c
+++ b/source4/torture/dns/dlz_bind9.c
@@ -19,7 +19,7 @@
 
 #include "includes.h"
 #include "torture/smbtorture.h"
-#include "dlz_minimal.h"
+#include "dns_server/dlz_minimal.h"
 #include <talloc.h>
 #include <ldb.h>
 #include "lib/param/param.h"
@@ -54,13 +54,22 @@ static bool test_dlz_bind9_version(struct torture_context *tctx)
 	return true;
 }
 
+static char *test_dlz_bind9_binddns_dir(struct torture_context *tctx,
+					const char *file)
+{
+	return talloc_asprintf(tctx,
+			       "%s/%s",
+			       lpcfg_binddns_dir(tctx->lp_ctx),
+			       file);
+}
+
 static bool test_dlz_bind9_create(struct torture_context *tctx)
 {
 	void *dbdata;
 	const char *argv[] = {
 		"samba_dlz",
 		"-H",
-		lpcfg_private_path(tctx, tctx->lp_ctx, "dns/sam.ldb"),
+		test_dlz_bind9_binddns_dir(tctx, "dns/sam.ldb"),
 		NULL
 	};
 	tctx_static = tctx;
@@ -79,7 +88,8 @@ static isc_result_t dlz_bind9_writeable_zone_hook(dns_view_t *view,
 	struct torture_context *tctx = talloc_get_type((void *)view, struct torture_context);
 	struct ldb_context *samdb = samdb_connect_url(tctx, NULL, tctx->lp_ctx,
 						      system_session(tctx->lp_ctx),
-						      0, lpcfg_private_path(tctx, tctx->lp_ctx, "dns/sam.ldb"));
+						      0,
+						      test_dlz_bind9_binddns_dir(tctx, "dns/sam.ldb"));
 	struct ldb_message *msg;
 	int ret;
 	const char *attrs[] = {
@@ -108,7 +118,7 @@ static bool test_dlz_bind9_configure(struct torture_context *tctx)
 	const char *argv[] = {
 		"samba_dlz",
 		"-H",
-		lpcfg_private_path(tctx, tctx->lp_ctx, "dns/sam.ldb"),
+		test_dlz_bind9_binddns_dir(tctx, "dns/sam.ldb"),
 		NULL
 	};
 	tctx_static = tctx;
@@ -143,7 +153,7 @@ static bool test_dlz_bind9_gensec(struct torture_context *tctx, const char *mech
 	const char *argv[] = {
 		"samba_dlz",
 		"-H",
-		lpcfg_private_path(tctx, tctx->lp_ctx, "dns/sam.ldb"),
+		test_dlz_bind9_binddns_dir(tctx, "dns/sam.ldb"),
 		NULL
 	};
 	tctx_static = tctx;
@@ -323,7 +333,7 @@ static bool test_dlz_bind9_lookup(struct torture_context *tctx)
 	const char *argv[] = {
 		"samba_dlz",
 		"-H",
-		lpcfg_private_path(tctx, tctx->lp_ctx, "dns/sam.ldb"),
+		test_dlz_bind9_binddns_dir(tctx, "dns/sam.ldb"),
 		NULL
 	};
 	struct test_expected_rr *expected1 = NULL;
@@ -448,7 +458,7 @@ static bool test_dlz_bind9_zonedump(struct torture_context *tctx)
 	const char *argv[] = {
 		"samba_dlz",
 		"-H",
-		lpcfg_private_path(tctx, tctx->lp_ctx, "dns/sam.ldb"),
+		test_dlz_bind9_binddns_dir(tctx, "dns/sam.ldb"),
 		NULL
 	};
 	struct test_expected_rr *expected1 = NULL;
@@ -560,7 +570,7 @@ static bool test_dlz_bind9_update01(struct torture_context *tctx)
 	const char *argv[] = {
 		"samba_dlz",
 		"-H",
-		lpcfg_private_path(tctx, tctx->lp_ctx, "dns/sam.ldb"),
+		test_dlz_bind9_binddns_dir(tctx, "dns/sam.ldb"),
 		NULL
 	};
 	struct test_expected_rr *expected1 = NULL;
-- 
2.14.1


>From 2f9e18047f8fc6d35df917f84f252322b152889d Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 10 Aug 2017 15:37:54 +0200
Subject: [PATCH 5/6] python:samba: Use 'binddns dir' in samba-tool and
 samba_upgradedns

This provisions the bind_dlz files in the 'binddns dir'. If you want to
migrate to the new files strcuture you can run samba_upgradedns!

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957

Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Andrew Bartlet <abartlet at samba.org>
---
 python/samba/provision/__init__.py           | 19 +++++++++++++------
 python/samba/provision/sambadns.py           | 12 +++++++-----
 python/samba/tests/provision.py              |  2 ++
 source4/scripting/bin/samba_upgradedns       |  6 +++---
 source4/scripting/bin/samba_upgradeprovision | 16 +++++++++-------
 5 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
index 91d2105929c..bfff4d7d059 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -145,6 +145,7 @@ class ProvisionPaths(object):
         self.dns = None
         self.winsdb = None
         self.private_dir = None
+        self.binddns_dir = None
         self.state_dir = None
 
 
@@ -531,6 +532,7 @@ def provision_paths_from_lp(lp, dnsdomain):
     """
     paths = ProvisionPaths()
     paths.private_dir = lp.get("private dir")
+    paths.binddns_dir = lp.get("binddns dir")
     paths.state_dir = lp.get("state directory")
 
     # This is stored without path prefix for the "privateKeytab" attribute in
@@ -543,16 +545,18 @@ def provision_paths_from_lp(lp, dnsdomain):
     paths.idmapdb = os.path.join(paths.private_dir, "idmap.ldb")
     paths.secrets = os.path.join(paths.private_dir, "secrets.ldb")
     paths.privilege = os.path.join(paths.private_dir, "privilege.ldb")
-    paths.dns = os.path.join(paths.private_dir, "dns", dnsdomain + ".zone")
     paths.dns_update_list = os.path.join(paths.private_dir, "dns_update_list")
     paths.spn_update_list = os.path.join(paths.private_dir, "spn_update_list")
-    paths.namedconf = os.path.join(paths.private_dir, "named.conf")
-    paths.namedconf_update = os.path.join(paths.private_dir, "named.conf.update")
-    paths.namedtxt = os.path.join(paths.private_dir, "named.txt")
     paths.krb5conf = os.path.join(paths.private_dir, "krb5.conf")
     paths.kdcconf = os.path.join(paths.private_dir, "kdc.conf")
     paths.winsdb = os.path.join(paths.private_dir, "wins.ldb")
     paths.s4_ldapi_path = os.path.join(paths.private_dir, "ldapi")
+
+    paths.dns = os.path.join(paths.binddns_dir, "dns", dnsdomain + ".zone")
+    paths.namedconf = os.path.join(paths.binddns_dir, "named.conf")
+    paths.namedconf_update = os.path.join(paths.binddns_dir, "named.conf.update")
+    paths.namedtxt = os.path.join(paths.binddns_dir, "named.txt")
+
     paths.hklm = "hklm.ldb"
     paths.hkcr = "hkcr.ldb"
     paths.hkcu = "hkcu.ldb"
@@ -945,7 +949,7 @@ def setup_secretsdb(paths, session_info, backend_credentials, lp):
     if os.path.exists(keytab_path):
         os.unlink(keytab_path)
 
-    dns_keytab_path = os.path.join(paths.private_dir, paths.dns_keytab)
+    dns_keytab_path = os.path.join(paths.binddns_dir, paths.dns_keytab)
     if os.path.exists(dns_keytab_path):
         os.unlink(dns_keytab_path)
 
@@ -2070,6 +2074,8 @@ def provision(logger, session_info, smbconf=None,
         os.makedirs(os.path.join(paths.private_dir, "tls"), 0700)
     if not os.path.exists(paths.state_dir):
         os.mkdir(paths.state_dir)
+    if not os.path.exists(paths.binddns_dir):
+        os.mkdir(paths.binddns_dir, 0o770)
 
     if paths.sysvol and not os.path.exists(paths.sysvol):
         os.makedirs(paths.sysvol, 0775)
@@ -2199,7 +2205,8 @@ def provision(logger, session_info, smbconf=None,
     secrets_ldb.transaction_commit()
 
     # the commit creates the dns.keytab, now chown it
-    dns_keytab_path = os.path.join(paths.private_dir, paths.dns_keytab)
+    dns_keytab_path = os.path.join(paths.binddns_dir, paths.dns_keytab)
+
     if os.path.isfile(dns_keytab_path) and paths.bind_gid is not None:
         try:
             os.chmod(dns_keytab_path, 0640)
diff --git a/python/samba/provision/sambadns.py b/python/samba/provision/sambadns.py
index dcb19c7053c..268c949e34e 100644
--- a/python/samba/provision/sambadns.py
+++ b/python/samba/provision/sambadns.py
@@ -665,6 +665,8 @@ def secretsdb_setup_dns(secretsdb, names, private_dir, realm,
     if key_version_number is None:
         key_version_number = 1
 
+    # This will create the dns.keytab file in the private_dir when it is
+    # commited!
     setup_ldb(secretsdb, setup_path("secrets_dns.ldif"), {
             "REALM": realm,
             "DNSDOMAIN": dnsdomain,
@@ -954,7 +956,7 @@ def create_named_conf(paths, realm, dnsdomain, dns_backend, logger):
                     })
 
 
-def create_named_txt(path, realm, dnsdomain, dnsname, private_dir,
+def create_named_txt(path, realm, dnsdomain, dnsname, binddns_dir,
     keytab_name):
     """Write out a file containing zone statements suitable for inclusion in a
     named.conf file (including GSS-TSIG configuration).
@@ -962,7 +964,7 @@ def create_named_txt(path, realm, dnsdomain, dnsname, private_dir,
     :param path: Path of the new named.conf file.
     :param realm: Realm name
     :param dnsdomain: DNS Domain name
-    :param private_dir: Path to private directory
+    :param binddns_dir: Path to bind dns directory
     :param keytab_name: File name of DNS keytab file
     """
     setup_file(setup_path("named.txt"), path, {
@@ -970,8 +972,8 @@ def create_named_txt(path, realm, dnsdomain, dnsname, private_dir,
             "DNSNAME" : dnsname,
             "REALM": realm,
             "DNS_KEYTAB": keytab_name,
-            "DNS_KEYTAB_ABS": os.path.join(private_dir, keytab_name),
-            "PRIVATE_DIR": private_dir
+            "DNS_KEYTAB_ABS": os.path.join(binddns_dir, keytab_name),
+            "PRIVATE_DIR": binddns_dir
         })
 
 
@@ -1218,7 +1220,7 @@ def setup_bind9_dns(samdb, secretsdb, names, paths, lp, logger,
     create_named_txt(paths.namedtxt,
                      realm=names.realm, dnsdomain=names.dnsdomain,
                      dnsname = "%s.%s" % (names.hostname, names.dnsdomain),
-                     private_dir=paths.private_dir,
+                     binddns_dir=paths.binddns_dir,
                      keytab_name=paths.dns_keytab)
     logger.info("See %s for an example configuration include file for BIND",
                 paths.namedconf)
diff --git a/python/samba/tests/provision.py b/python/samba/tests/provision.py
index 11b0135f473..bada14f5936 100644
--- a/python/samba/tests/provision.py
+++ b/python/samba/tests/provision.py
@@ -42,6 +42,7 @@ def create_dummy_secretsdb(path, lp=None):
     paths = ProvisionPaths()
     paths.secrets = path
     paths.private_dir = os.path.dirname(path)
+    paths.binddns_dir = os.path.dirname(path)
     paths.keytab = "no.keytab"
     paths.dns_keytab = "no.dns.keytab"
     secrets_ldb = setup_secretsdb(paths, None, None, lp=lp)
@@ -59,6 +60,7 @@ class ProvisionTestCase(samba.tests.TestCaseInTempDir):
         secrets_tdb_path = os.path.join(self.tempdir, "secrets.tdb")
         paths.secrets = path
         paths.private_dir = os.path.dirname(path)
+        paths.binddns_dir = os.path.dirname(path)
         paths.keytab = "no.keytab"
         paths.dns_keytab = "no.dns.keytab"
         ldb = setup_secretsdb(paths, None, None, lp=env_loadparm())
diff --git a/source4/scripting/bin/samba_upgradedns b/source4/scripting/bin/samba_upgradedns
index d00b67daca1..316ad930721 100755
--- a/source4/scripting/bin/samba_upgradedns
+++ b/source4/scripting/bin/samba_upgradedns
@@ -446,7 +446,7 @@ if __name__ == '__main__':
                 dns_key_version_number = None
 
             secretsdb_setup_dns(ldbs.secrets, names,
-                                paths.private_dir, realm=names.realm,
+                                paths.binddns_dir, realm=names.realm,
                                 dnsdomain=names.dnsdomain,
                                 dns_keytab_path=paths.dns_keytab, dnspass=dnspass,
                                 key_version_number=dns_key_version_number)
@@ -454,7 +454,7 @@ if __name__ == '__main__':
         else:
             logger.info("dns-%s account already exists" % hostname)
 
-        dns_keytab_path = os.path.join(paths.private_dir, paths.dns_keytab)
+        dns_keytab_path = os.path.join(paths.binddns_dir, paths.dns_keytab)
         if os.path.isfile(dns_keytab_path) and paths.bind_gid is not None:
             try:
                 os.chmod(dns_keytab_path, 0640)
@@ -476,7 +476,7 @@ if __name__ == '__main__':
         create_named_conf(paths, names.realm, dnsdomain, opts.dns_backend, logger)
 
         create_named_txt(paths.namedtxt, names.realm, dnsdomain, dnsname,
-                         paths.private_dir, paths.dns_keytab)
+                         paths.binddns_dir, paths.dns_keytab)
         logger.info("See %s for an example configuration include file for BIND", paths.namedconf)
         logger.info("and %s for further documentation required for secure DNS "
                     "updates", paths.namedtxt)
diff --git a/source4/scripting/bin/samba_upgradeprovision b/source4/scripting/bin/samba_upgradeprovision
index 99e97b7f28f..d11175314c6 100755
--- a/source4/scripting/bin/samba_upgradeprovision
+++ b/source4/scripting/bin/samba_upgradeprovision
@@ -207,7 +207,7 @@ creds.set_kerberos_state(DONT_USE_KERBEROS)
 
 
 
-def check_for_DNS(refprivate, private, dns_backend):
+def check_for_DNS(refprivate, private, refbinddns_dir, binddns_dir, dns_backend):
     """Check if the provision has already the requirement for dynamic dns
 
     :param refprivate: The path to the private directory of the reference
@@ -229,17 +229,17 @@ def check_for_DNS(refprivate, private, dns_backend):
 
     namedfile = lp.get("dnsupdate:path")
     if not namedfile:
-       namedfile = "%s/named.conf.update" % private
+       namedfile = "%s/named.conf.update" % binddns_dir
     if not os.path.exists(namedfile):
-        destdir = "%s/new_dns" % private
-        dnsdir = "%s/dns" % private
+        destdir = "%s/new_dns" % binddns_dir
+        dnsdir = "%s/dns" % binddns_dir
 
         if not os.path.exists(destdir):
             os.mkdir(destdir)
         if not os.path.exists(dnsdir):
             os.mkdir(dnsdir)
-        shutil.copy("%s/named.conf" % refprivate, "%s/named.conf" % destdir)
-        shutil.copy("%s/named.txt" % refprivate, "%s/named.txt" % destdir)
+        shutil.copy("%s/named.conf" % refbinddns_dir, "%s/named.conf" % destdir)
+        shutil.copy("%s/named.txt" % refbinddns_dir, "%s/named.txt" % destdir)
         message(SIMPLE, "It seems that your provision did not integrate "
                 "new rules for dynamic dns update of domain related entries")
         message(SIMPLE, "A copy of the new bind configuration files and "
@@ -1793,7 +1793,9 @@ if __name__ == '__main__':
         # 20)
         updateOEMInfo(ldbs.sam, str(names.rootdn))
         # 21)
-        check_for_DNS(newpaths.private_dir, paths.private_dir, names.dns_backend)
+        check_for_DNS(newpaths.private_dir, paths.private_dir,
+                      newpaths.binddns_dir, paths.binddns_dir,
+                      names.dns_backend)
         # 22)
         update_provision_usn(ldbs.sam, minUSN, maxUSN, names.invocation)
         if opts.full and (names.policyid is None or names.policyid_dc is None):
-- 
2.14.1


>From dab4fb5baaabfb7dcd6e8a6ef54277b7e69e2e5f Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 23 Aug 2017 15:36:23 +0200
Subject: [PATCH 6/6] python:samba: Add code to remove obsolete files in the
 private dir

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957

Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Andrew Bartlet <abartlet at samba.org>
---
 source4/scripting/bin/samba_upgradedns | 35 ++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/source4/scripting/bin/samba_upgradedns b/source4/scripting/bin/samba_upgradedns
index 316ad930721..cac6815d3ec 100755
--- a/source4/scripting/bin/samba_upgradedns
+++ b/source4/scripting/bin/samba_upgradedns
@@ -20,6 +20,7 @@
 
 import sys
 import os
+import errno
 import optparse
 import logging
 import grp
@@ -209,6 +210,37 @@ def import_zone_data(samdb, logger, zone, serial, domaindn, forestdn,
             raise
         logger.debug("Added DNS record %s" % (fqdn))
 
+def cleanup_remove_file(file_path):
+    try:
+        os.remove(file_path)
+    except OSError as e:
+        if e.errno not in [errno.EEXIST, errno.ENOENT]:
+            pass
+        else:
+            logger.debug("Could not remove %s: %s" % (file_path, e.strerror))
+
+def cleanup_remove_dir(dir_path):
+    try:
+        for root, dirs, files in os.walk(dir_path, topdown=False):
+            for name in files:
+                os.remove(os.path.join(root, name))
+            for name in dirs:
+                os.rmdir(os.path.join(root, name))
+        os.rmdir(dir_path)
+    except OSError as e:
+        if e.errno not in [errno.EEXIST, errno.ENOENT]:
+            pass
+        else:
+            logger.debug("Could not delete dir %s: %s" % (dir_path, e.strerror))
+
+def cleanup_obsolete_dns_files(paths):
+    cleanup_remove_file(os.path.join(paths.private_dir, "named.conf"))
+    cleanup_remove_file(os.path.join(paths.private_dir, "named.conf.update"))
+    cleanup_remove_file(os.path.join(paths.private_dir, "named.txt"))
+    cleanup_remove_file(os.path.join(paths.private_dir, "dns.keytab"))
+
+    cleanup_remove_dir(os.path.join(paths.private_dir, "dns"))
+
 
 # dnsprovision creates application partitions for AD based DNS mainly if the existing
 # provision was created using earlier snapshots of samba4 which did not have support
@@ -477,6 +509,9 @@ if __name__ == '__main__':
 
         create_named_txt(paths.namedtxt, names.realm, dnsdomain, dnsname,
                          paths.binddns_dir, paths.dns_keytab)
+
+        cleanup_obsolete_dns_files(paths)
+
         logger.info("See %s for an example configuration include file for BIND", paths.namedconf)
         logger.info("and %s for further documentation required for secure DNS "
                     "updates", paths.namedtxt)
-- 
2.14.1



More information about the samba-technical mailing list