[PATCH] Create a 'binddns dir' for files used by the bind_dlz module and named
Andreas Schneider
asn at samba.org
Tue Sep 12 14:25:06 UTC 2017
On Tuesday, 12 September 2017 14:48:23 CEST Andreas Schneider via samba-
technical wrote:
> On Wednesday, 6 September 2017 11:27:38 CEST Andreas Schneider via samba-
>
> technical wrote:
> > On Tuesday, 5 September 2017 12:31:20 CEST Andreas Schneider via samba-
> >
> > technical wrote:
> > > On Monday, 4 September 2017 21:22:39 CEST Andrew Bartlett wrote:
> > > > Andreas,
> > >
> > > Andrew,
> > >
> > > > I know this won't make you very happy, but I think this is a 4.8 patch
> > > > at this point. You can of course patch Fedora packages, but I fear
> > > > further dragons, given the fight it has given so far, and while parts
> > > > of the DLZ mode are tested (thankfully!) the whole integration is not
> > > > verified in make test.
> >
> > Hi,
> >
> > > would you accept it if Marc and I would do manual testing and fix
> > > remaining
> > > issues.
> > >
> > > This means upgrading from 4.6 to 4.7 with my patchset and check if it
> > > works
> > > seemlessly.
> > > Switching backends etc.
> > >
> > > I do not feel very happy with the current code and giving named broad
> > > access to keytab and AD partitions.
> > >
> > > Also when switching from bind_dlz to the internal DNS we should remove
> > > files which give the named group full access to AD.
> >
> > Jeremy pushed the last patchset from this thread to master.
> >
> > Yesterday I asked Marc for help. He tested the feature and we discussed
> > several aspects, especially security concerns, like file and directory
> > permission.
> > We fixed some issues we found during extensive testing and we improved the
> > messages samba-tool and samba_upgradedns print so that the user knows what
> > he has to do.
> > We also found some things we need to fix in the documentation in the wiki.
> >
> >
> > The attached patchset addresses the remaining issues. Marc will answer and
> > add the test plan we created and he followed.
> >
> > I hope this gives you the confidence in the changes that we can include
> > them in 4.7.
> >
> > Please review carefully.
>
> Ping!
With another fix. We need to honor make DESTDIR=/home/abuild/rpmbuild install.
> > Thanks,
> >
> > Andreas
> >
> > P.S.: The internel DNS server doesn't work in 4.7 and master
> >
> > https://bugzilla.samba.org/show_bug.cgi?id=13019
--
Andreas Schneider GPG-ID: CC014E3D
Samba Team asn at samba.org
www.samba.org
-------------- next part --------------
>From 86051302a61987cf9a1619718caab3284ecf5877 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Tue, 5 Sep 2017 14:18:44 +0200
Subject: [PATCH 01/10] wafsamba: Do not chmod already existing dirs on install
This might break backward compatibility.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957
Signed-off-by: Andreas Schneider <asn at samba.org>
---
buildtools/wafsamba/wafsamba.py | 2 --
1 file changed, 2 deletions(-)
diff --git a/buildtools/wafsamba/wafsamba.py b/buildtools/wafsamba/wafsamba.py
index 57913af2bd7..f91adca1a0c 100644
--- a/buildtools/wafsamba/wafsamba.py
+++ b/buildtools/wafsamba/wafsamba.py
@@ -900,8 +900,6 @@ def INSTALL_DIR(bld, path, chmod=0o755):
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, chmod=0o755):
--
2.14.1
>From 05921ff542a602da33164ff1d9c5eb46386e44a7 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Tue, 5 Sep 2017 20:36:47 +0200
Subject: [PATCH 02/10] samba:provision: Give a hint to copy the krb5.conf and
not symlink it
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957
Signed-off-by: Andreas Schneider <asn at samba.org>
---
python/samba/provision/__init__.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
index f820f6ab675..8a8c033105f 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -2200,6 +2200,9 @@ def provision(logger, session_info, smbconf=None,
realm=names.realm)
logger.info("A Kerberos configuration suitable for Samba AD has been "
"generated at %s", paths.krb5conf)
+ logger.info("Merge the contents of this file with your system "
+ "krb5.conf or replace it with this one. Do not create a "
+ "symlink!")
if serverrole == "active directory domain controller":
create_dns_update_list(lp, logger, paths)
--
2.14.1
>From 44100f933073a1d924541fa95eca857fe7dacfef 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 03/10] dynconfig: Fix location of the default 'binddns dir'
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957
Signed-off-by: Andreas Schneider <asn at samba.org>
---
dynconfig/wscript | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/dynconfig/wscript b/dynconfig/wscript
index fee37eaaf5f..54977e42bd4 100644
--- a/dynconfig/wscript
+++ b/dynconfig/wscript
@@ -174,6 +174,12 @@ dynconfig = {
'OPTION': '--with-privatedir',
'HELPTEXT': 'Where to put sam.ldb and other private files',
},
+ 'BINDDNS_DIR' : {
+ 'STD-PATH': '${PREFIX}/bind-dns',
+ 'FHS-PATH': '${LOCALSTATEDIR}/lib/samba/bind-dns',
+ 'OPTION': '--with-bind-dns-dir',
+ 'HELPTEXT': 'bind-dns config directory',
+ },
'LOCKDIR' : {
'STD-PATH': '${LOCALSTATEDIR}/lock',
'FHS-PATH': '${LOCALSTATEDIR}/lock/samba',
@@ -192,12 +198,6 @@ 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',
--
2.14.1
>From 789d3a701fbb960539334a920131b78eb613be47 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 04/10] s4:bind_dlz: Try the 'binddns dir' first
The directory is normally empty if you did not provision or call
samba_upgradedns for the bind_dlz module.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957
Signed-off-by: Andreas Schneider <asn at samba.org>
---
source4/dns_server/dlz_bind9.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/source4/dns_server/dlz_bind9.c b/source4/dns_server/dlz_bind9.c
index 75a9ce0f648..5079257c9fe 100644
--- a/source4/dns_server/dlz_bind9.c
+++ b/source4/dns_server/dlz_bind9.c
@@ -682,9 +682,9 @@ _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 = talloc_asprintf(state,
+ "%s/dns/sam.ldb",
+ lpcfg_binddns_dir(state->lp));
if (state->options.url == NULL) {
result = ISC_R_NOMEMORY;
goto failed;
@@ -693,7 +693,7 @@ _PUBLIC_ isc_result_t dlz_create(const char *dlzname,
if (!file_exist(state->options.url)) {
state->options.url = talloc_asprintf(state,
"%s/dns/sam.ldb",
- lpcfg_binddns_dir(state->lp));
+ lpcfg_private_dir(state->lp));
if (state->options.url == NULL) {
result = ISC_R_NOMEMORY;
goto failed;
@@ -1324,7 +1324,7 @@ _PUBLIC_ isc_boolean_t dlz_ssumatch(const char *signer, const char *name, const
keytab_file = talloc_asprintf(tmp_ctx,
"%s/dns.keytab",
- lpcfg_private_dir(state->lp));
+ lpcfg_binddns_dir(state->lp));
if (keytab_file == NULL) {
state->log(ISC_LOG_ERROR, "samba_dlz: Out of memory!");
talloc_free(tmp_ctx);
@@ -1334,7 +1334,7 @@ _PUBLIC_ isc_boolean_t dlz_ssumatch(const char *signer, const char *name, const
if (!file_exist(keytab_file)) {
keytab_file = talloc_asprintf(tmp_ctx,
"%s/dns.keytab",
- lpcfg_binddns_dir(state->lp));
+ lpcfg_private_dir(state->lp));
if (keytab_file == NULL) {
state->log(ISC_LOG_ERROR, "samba_dlz: Out of memory!");
talloc_free(tmp_ctx);
--
2.14.1
>From 35232209be8f9fa8219d9af33bb0ea755fa189cd Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 6 Sep 2017 07:23:57 +0200
Subject: [PATCH 05/10] python:provision: Change the group of the 'binddns dir'
too
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957
Signed-off-by: Andreas Schneider <asn at samba.org>
---
python/samba/provision/__init__.py | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
index 8a8c033105f..07c24795477 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -2238,6 +2238,14 @@ def provision(logger, session_info, smbconf=None,
# chown the dns.keytab in the bind-dns directory
if paths.bind_gid is not None:
+ try:
+ os.chmod(paths.binddns_dir, 0770)
+ os.chown(paths.binddns_dir, -1, paths.bind_gid)
+ except OSError:
+ if not os.environ.has_key('SAMBA_SELFTEST'):
+ logger.info("Failed to chown %s to bind gid %u",
+ paths.binddns_dir, paths.bind_gid)
+
try:
os.chmod(bind_dns_keytab_path, 0640)
os.chown(bind_dns_keytab_path, -1, paths.bind_gid)
--
2.14.1
>From 798d7bc6959699ab1f62dc0cfb63e20dcbca39b1 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 6 Sep 2017 07:25:04 +0200
Subject: [PATCH 06/10] python:provision: Do not change the owner of the
sam.ldb.d dir
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957
Signed-off-by: Andreas Schneider <asn at samba.org>
---
python/samba/provision/sambadns.py | 3 ---
1 file changed, 3 deletions(-)
diff --git a/python/samba/provision/sambadns.py b/python/samba/provision/sambadns.py
index d4cb93a89ea..c95583162e7 100644
--- a/python/samba/provision/sambadns.py
+++ b/python/samba/provision/sambadns.py
@@ -864,9 +864,6 @@ def create_samdb_copy(samdb, logger, paths, names, domainsid, domainguid):
# Give bind read/write permissions dns partitions
if paths.bind_gid is not None:
try:
- os.chown(samldb_dir, -1, paths.bind_gid)
- os.chmod(samldb_dir, 0750)
-
for dirname, dirs, files in os.walk(dns_dir):
for d in dirs:
dpath = os.path.join(dirname, d)
--
2.14.1
>From 377ce500daa83c2d20bd9bc7edb4a39e460e3aa5 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 6 Sep 2017 10:06:40 +0200
Subject: [PATCH 07/10] samba_upgradedns: Change the group of the 'binddns dir'
too
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957
Signed-off-by: Andreas Schneider <asn at samba.org>
---
source4/scripting/bin/samba_upgradedns | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/source4/scripting/bin/samba_upgradedns b/source4/scripting/bin/samba_upgradedns
index 2582da0f6bc..db3ef5c6d65 100755
--- a/source4/scripting/bin/samba_upgradedns
+++ b/source4/scripting/bin/samba_upgradedns
@@ -505,6 +505,13 @@ if __name__ == '__main__':
# chown the dns.keytab in the bind-dns directory
if paths.bind_gid is not None:
+ try:
+ os.chmod(paths.binddns_dir, 0o770)
+ os.chown(paths.binddns_dir, -1, paths.bind_gid)
+ except OSError:
+ if not os.environ.has_key('SAMBA_SELFTEST'):
+ logger.info("Failed to chown %s to bind gid %u",
+ paths.binddns_dir, paths.bind_gid)
try:
os.chmod(bind_dns_keytab_path, 0640)
os.chown(bind_dns_keytab_path, -1, paths.bind_gid)
--
2.14.1
>From cdf9e1dfb0c94f5d8015ec4b9b23201068a7b955 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 6 Sep 2017 07:25:40 +0200
Subject: [PATCH 08/10] samba_upgradedns: Print better hints after we migrated
the config
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957
Signed-off-by: Andreas Schneider <asn at samba.org>
---
source4/scripting/bin/samba_upgradedns | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/source4/scripting/bin/samba_upgradedns b/source4/scripting/bin/samba_upgradedns
index db3ef5c6d65..3369bcfed93 100755
--- a/source4/scripting/bin/samba_upgradedns
+++ b/source4/scripting/bin/samba_upgradedns
@@ -442,6 +442,12 @@ if __name__ == '__main__':
# Special stuff for DLZ backend
if opts.dns_backend == "BIND9_DLZ":
+ config_migration = False
+
+ if (paths.private_dir != paths.binddns_dir and
+ os.path.isfile(os.path.join(paths.private_dir, "named.conf"))):
+ config_migration = True
+
# Check if dns-HOSTNAME account exists and create it if required
secrets_msgs = ldbs.secrets.search(expression='(samAccountName=dns-%s)' % hostname, attrs=['secret'])
msg = ldbs.sam.search(base=domaindn, scope=ldb.SCOPE_DEFAULT,
@@ -537,9 +543,15 @@ if __name__ == '__main__':
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)
+ if config_migration:
+ logger.info("ATTENTION: The BIND configuration and keytab has been moved to: %s",
+ paths.binddns_dir)
+ logger.info(" Please update your BIND configuration accordingly.")
+ else:
+ 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)
+
elif opts.dns_backend == "SAMBA_INTERNAL":
# Check if dns-HOSTNAME account exists and delete it if required
try:
--
2.14.1
>From a459b0c6875a2cd6fac4c89a23073a92d78b60b4 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Tue, 5 Sep 2017 11:47:27 +0200
Subject: [PATCH 09/10] samba_upgradedns: When we setup the internal dns
cleanup bind-dns dir
Make sure to remove everything from the bind-dns directory to avoid
possible security issues with the named group having write access to all
AD partions
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957
Signed-off-by: Andreas Schneider <asn at samba.org>
---
source4/scripting/bin/samba_upgradedns | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/source4/scripting/bin/samba_upgradedns b/source4/scripting/bin/samba_upgradedns
index 3369bcfed93..261d8a1922d 100755
--- a/source4/scripting/bin/samba_upgradedns
+++ b/source4/scripting/bin/samba_upgradedns
@@ -553,6 +553,23 @@ if __name__ == '__main__':
"updates", paths.namedtxt)
elif opts.dns_backend == "SAMBA_INTERNAL":
+ # Make sure to remove everything from the bind-dns directory to avoid
+ # possible security issues with the named group having write access
+ # to all AD partions
+ cleanup_remove_file(os.path.join(paths.binddns_dir, "dns.keytab"))
+ cleanup_remove_file(os.path.join(paths.binddns_dir, "named.conf"))
+ cleanup_remove_file(os.path.join(paths.binddns_dir, "named.conf.update"))
+ cleanup_remove_file(os.path.join(paths.binddns_dir, "named.txt"))
+
+ cleanup_remove_dir(os.path.dirname(paths.dns))
+
+ try:
+ os.chmod(paths.private_dir, 0o700)
+ os.chown(paths.private_dir, -1, 0)
+ except:
+ logger.warn("Failed to restore owner and permissions for %s",
+ (paths.private_dir))
+
# Check if dns-HOSTNAME account exists and delete it if required
try:
dn_str = 'samAccountName=dns-%s,CN=Principals' % hostname
--
2.14.1
>From f9cdbd5946161b79b08a0d5043d00708e7ebda06 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Tue, 12 Sep 2017 15:56:44 +0200
Subject: [PATCH 10/10] wafsamba: We need to honor DESTDIR in INSTALL_DIR
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12957
Signed-off-by: Andreas Schneider <asn at samba.org>
---
buildtools/wafsamba/wafsamba.py | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/buildtools/wafsamba/wafsamba.py b/buildtools/wafsamba/wafsamba.py
index f91adca1a0c..23fd3c48342 100644
--- a/buildtools/wafsamba/wafsamba.py
+++ b/buildtools/wafsamba/wafsamba.py
@@ -885,29 +885,30 @@ 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):
+def INSTALL_DIR(bld, path, chmod=0o755, env=None):
"""Install a directory if it doesn't exist, always set permissions."""
if not path:
return []
+ destpath = bld.get_install_path(path, env)
+
if bld.is_install > 0:
- path = bld.EXPAND_VARIABLES(path)
- if not os.path.isdir(path):
+ if not os.path.isdir(destpath):
try:
- os.makedirs(path)
- os.chmod(path, chmod)
+ os.makedirs(destpath)
+ os.chmod(destpath, chmod)
except OSError, e:
- if not os.path.isdir(path):
+ if not os.path.isdir(destpath):
raise Utils.WafError("Cannot create the folder '%s' (error: %s)" % (path, e))
Build.BuildContext.INSTALL_DIR = INSTALL_DIR
-def INSTALL_DIRS(bld, destdir, dirs, chmod=0o755):
+def INSTALL_DIRS(bld, destdir, dirs, chmod=0o755, env=None):
'''install a set of directories'''
destdir = bld.EXPAND_VARIABLES(destdir)
dirs = bld.EXPAND_VARIABLES(dirs)
for d in TO_LIST(dirs):
- INSTALL_DIR(bld, os.path.join(destdir, d), chmod)
+ INSTALL_DIR(bld, os.path.join(destdir, d), chmod, env)
Build.BuildContext.INSTALL_DIRS = INSTALL_DIRS
--
2.14.1
More information about the samba-technical
mailing list