[PATCH] Improve renamedc, add chgkrbtgtpass commands

Jelmer Vernooij jelmer at samba.org
Mon Feb 23 17:37:07 MST 2015


LGTM, but one nitpicky comment:

Having a 'if __name__ == "__main__"' section in
source4/scripting/devel/chgkrbtgtpass seems pointless to me, since
we're not going to import from it and I don't see in a difference in
what code is outside of the if vs what is inside.

Jelmer

On Mon, Feb 23, 2015 at 04:56:18PM +1300, Andrew Bartlett wrote:
> Attached is a patch to 
>  - improve the renamedc script now that we have a handler for one-way
> links, to add tests for the behaviour it has.
>  - explain the reason why we change the DC password twice in the
> chgtdcpass script
>  - Add a new script to change the krbtgt password
> 
> This is an interim step until they can be cleaned up enough to be a
> standard part of samba-tool, but should help a little in the meantime. 
> 
> Please review/push
> 
> 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
> 
> 
> 

> From 979225af38d7d50506def3961234c47df0984c61 Mon Sep 17 00:00:00 2001
> From: Andrew Bartlett <abartlet at samba.org>
> Date: Mon, 23 Feb 2015 16:10:31 +1300
> Subject: [PATCH 1/4] s4/scripting/bin/renamedc: Fix up rename DC script
> 
> We now have a reliable handler for backlinks so this we can now rename both objects
> 
> Signed-off-by: Andrew Bartlett <abartlet at samba.org>
> ---
>  source4/scripting/bin/renamedc | 60 ++++++++++++++++++------------------------
>  1 file changed, 26 insertions(+), 34 deletions(-)
> 
> diff --git a/source4/scripting/bin/renamedc b/source4/scripting/bin/renamedc
> index 1fa19b2..4494401 100755
> --- a/source4/scripting/bin/renamedc
> +++ b/source4/scripting/bin/renamedc
> @@ -74,27 +74,27 @@ if __name__ == '__main__':
>      if opts.oldname is None or opts.newname is None:
>          raise Exception("Option oldname or newname is missing")
>      res = ldbs.sam.search(expression="(&(name=%s)(serverReferenceBL=*))" % opts.oldname)
> -    if res is None or len(res) != 1:
> -        raise Exception("Wrong number of result returned, are you sure of the old name %s" %
> -                opts.oldname)
> +    if len(res) != 1:
> +        raise Exception("Wrong number of result returned (%d), are you sure of the old name %s" %
> +                (len(res), opts.oldname))
>  
>      # Ok got it then check that the new name is not used as well
>      res2 = ldbs.sam.search(expression="(&(name=%s)(objectclass=computer))" % opts.newname)
>      if len(res2) != 0:
>          raise Exception("Seems that %s is a name that already exists, pick another one" %
> -                opts.newname)
> +                        opts.newname)
>  
>      names = find_provision_key_parameters(ldbs.sam, ldbs.secrets, ldbs.idmap,
>                                              paths, smbconf, lp)
>  
>      # First rename the entry
>      # provision put the name in upper case so let's do it too !
> -    newdn = str(res[0].dn).replace("CN=%s" % opts.oldname, "CN=%s" % opts.newname.upper())
> -    dnobj = ldb.Dn(ldbs.sam, newdn)
> -    ldbs.sam.rename(res[0].dn, dnobj)
> +    newdn = ldb.Dn(ldbs.sam, str(res[0].dn))
> +    newdn.set_component(0, "cn", opts.newname.upper())
> +    ldbs.sam.rename(res[0].dn, newdn)
>  
>      # Then change password and samaccountname and dnshostname
> -    msg = ldb.Message(dnobj)
> +    msg = ldb.Message(newdn)
>      machinepass = samba.generate_random_password(128, 255)
>      mputf16 = machinepass.encode('utf-16-le')
>  
> @@ -114,8 +114,8 @@ if __name__ == '__main__':
>      ldbs.sam.modify(msg)
>  
>      # Do a self join one more time to resync the secrets file
> -    res = ldbs.sam.search(expression=("distinguishedName=%s" % newdn),
> -            attrs=["msDs-keyVersionNumber", "serverReferenceBL"])
> +    res = ldbs.sam.search(base=newdn, scope=ldb.SCOPE_BASE,
> +                          attrs=["msDs-keyVersionNumber", "serverReferenceBL"])
>      assert(len(res) == 1)
>      kvno = int(str(res[0]["msDs-keyVersionNumber"]))
>      serverbldn = ldb.Dn(ldbs.sam, str(res[0]["serverReferenceBL"]))
> @@ -135,12 +135,12 @@ if __name__ == '__main__':
>                  key_version_number=kvno,
>                  secure_channel_type=secChanType)
>  
> -    # Update RID set reference as there is no back link for the moment.
> +    # Update RID set reference so we don't have to runtime fixup until the next dbcheck as there is no back link.
>  
> -    res = ldbs.sam.search(expression="(objectClass=rIDSet)", base=newdn, attrs=[])
> +    res = ldbs.sam.search(expression="(objectClass=rIDSet)", base=newdn, scope=ldb.SCOPE_ONELEVEL, attrs=[])
>      assert(len(res) == 1)
>      newridset = str(res[0].dn)
> -    msg = ldb.Message(dnobj)
> +    msg = ldb.Message(newdn)
>  
>      msg["rIDSetReferences"] = ldb.MessageElement(newridset,
>                                              ldb.FLAG_MOD_REPLACE,
> @@ -148,26 +148,17 @@ if __name__ == '__main__':
>      ldbs.sam.modify(msg)
>  
>      # Update the server's sites configuration
> -    if False:
> -        # Desactivated for the moment we have a couple of issues with site 
> -        # renaming first one is that it's currently forbidden
> -        # second one is that a lot of links are not backlinked
> -        # and so won't be updated when the DN change (ie. fmsowner ...)
> -        serverbl = str(serverbldn)
> -        dnparts = serverbl.split(",")
> -        dnparts[0] = "CN=%s" % opts.newname.upper()
> -        newserverref = ",".join(dnparts)
> -
> -        newserverrefdn = ldb.Dn(ldbs.sam, newserverref)
> -
> -        ldbs.sam.rename(serverbldn, newserverrefdn)
> -
> -        msg = ldb.Message(newserverrefdn)
> -        msg["dNSHostName"] = ldb.MessageElement("%s.%s" % (opts.newname,
> -                                                names.dnsdomain),
> -                                                ldb.FLAG_MOD_REPLACE,
> -                                                "dNSHostName")
> -        ldbs.sam.modify(msg)
> +    newserverrefdn = ldb.Dn(ldbs.sam, str(serverbldn))
> +    newserverrefdn.set_component(0, "cn", opts.newname.upper())
> +
> +    ldbs.sam.rename(serverbldn, newserverrefdn)
> +
> +    msg = ldb.Message(newserverrefdn)
> +    msg["dNSHostName"] = ldb.MessageElement("%s.%s" % (opts.newname,
> +                                                       names.dnsdomain),
> +                                            ldb.FLAG_MOD_REPLACE,
> +                                            "dNSHostName")
> +    ldbs.sam.modify(msg)
>  
>      try:
>          ldbs.sam.transaction_prepare_commit()
> @@ -175,7 +166,7 @@ if __name__ == '__main__':
>      except Exception:
>          ldbs.sam.rollback()
>          ldbs.secrets.rollback()
> -        sys.exit(1)
> +        raise
>  
>      try:
>          ldbs.sam.transaction_commit()
> @@ -183,6 +174,7 @@ if __name__ == '__main__':
>      except Exception:
>          ldbs.sam.rollback()
>          ldbs.secrets.rollback()
> +        raise
>  
>      # All good so far
>      #print lp.get("private dir")
> -- 
> 2.1.3
> 

> From a14613994dc48d14709be5ee2755cabba3b27ed6 Mon Sep 17 00:00:00 2001
> From: Andrew Bartlett <abartlet at samba.org>
> Date: Mon, 23 Feb 2015 15:45:53 +1300
> Subject: [PATCH 2/4] selftest: Improve renamedc tests to confirm more than
>  just the exit code
> 
> This now confirms that the DC has been renamed
> 
> Signed-off-by: Andrew Bartlett <abartlet at samba.org>
> ---
>  testprogs/blackbox/renamedc.sh | 41 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/testprogs/blackbox/renamedc.sh b/testprogs/blackbox/renamedc.sh
> index 2eab85e..8741867 100755
> --- a/testprogs/blackbox/renamedc.sh
> +++ b/testprogs/blackbox/renamedc.sh
> @@ -10,12 +10,21 @@ fi
>  PREFIX="$1"
>  shift 1
>  
> +samba4bindir="$BINDIR"
> +ldbsearch="ldbsearch"
> +if [ -x "$samba4bindir/ldbsearch" ]; then
> +	ldbsearch="$samba4bindir/ldbsearch"
> +fi
> +
>  . `dirname $0`/subunit.sh
>  
>  if [ ! -d $PREFIX/renamedc_test ]; then
> -	$PYTHON $BINDIR/samba-tool domain provision --host-name=bar --domain=FOO --realm=foo.example.com --targetdir="$PREFIX/renamedc_test" --server-role="dc" --use-ntvfs
> +	mkdir -p $PREFIX/renamedc_test
>  fi
>  
> +testprovision() {
> +    $PYTHON $BINDIR/samba-tool domain provision --host-name=bar --domain=FOO --realm=foo.example.com --targetdir="$PREFIX/renamedc_test" --server-role="dc" --use-ntvfs
> +}
>  
>  testrenamedc() {
>  	$PYTHON $SRCDIR/source4/scripting/bin/renamedc \
> @@ -24,6 +33,21 @@ testrenamedc() {
>  		-s $PREFIX/renamedc_test/etc/smb.conf
>  }
>  
> +confirmrenamedc() {
> +    $ldbsearch -H $PREFIX/renamedc_test/private/sam.ldb -s base -b 'cn=RAYMONBAR,ou=domain controllers,dc=foo,dc=example,dc=com'
> +}
> +
> +confirmrenamedc_server() {
> +    $ldbsearch -H $PREFIX/renamedc_test/private/sam.ldb -s base -b 'cn=RAYMONBAR,CN=Servers,CN=Default-First-Site-Name,CN=Sites,CN=configuration,dc=foo,dc=example,dc=com'
> +}
> +
> +confirmrenamedc_sAMAccountName() {
> +    $ldbsearch -H $PREFIX/renamedc_test/private/sam.ldb -s base -b 'cn=RAYMONBAR,ou=domain controllers,dc=foo,dc=example,dc=com' sAMAccountName | grep 'sAMAccountName: RAYMONBAR\$'
> +}
> +
> +confirmrenamedc_dNSHostName() {
> +    $ldbsearch -H $PREFIX/renamedc_test/private/sam.ldb -s base -b 'cn=RAYMONBAR,ou=domain controllers,dc=foo,dc=example,dc=com' dNSHostName | grep 'dNSHostName: RAYMONBAR.foo.example.com'
> +}
>  
>  testrenamedc2() {
>  	$PYTHON $SRCDIR/source4/scripting/bin/renamedc \
> @@ -32,8 +56,19 @@ testrenamedc2() {
>  		-s $PREFIX/renamedc_test/etc/smb.conf
>  }
>  
> -testit "renamedc" testrenamedc
> -testit "renamedc2" testrenamedc2
> +dbcheck() {
> +	$BINDIR/samba-tool dbcheck --cross-ncs -s $PREFIX/renamedc_test/etc/smb.conf
> +}
> +
> +
> +testit "renameprovision" testprovision || failed=`expr $failed + 1`
> +testit "renamedc" testrenamedc || failed=`expr $failed + 1`
> +testit "confirmrenamedc" confirmrenamedc || failed=`expr $failed + 1`
> +testit "confirmrenamedc_server" confirmrenamedc_server || failed=`expr $failed + 1`
> +testit "confirmrenamedc_sAMAccountName" confirmrenamedc_sAMAccountName || failed=`expr $failed + 1`
> +testit "confirmrenamedc_dNSHostName" confirmrenamedc_dNSHostName || failed=`expr $failed + 1`
> +testit "dbcheck" dbcheck || failed=`expr $failed + 1`
> +testit "renamedc2" testrenamedc2 || failed=`expr $failed + 1`
>  
>  if [ $failed -eq 0 ]; then
>  	rm -rf $PREFIX/renamedc_test
> -- 
> 2.1.3
> 

> From 4761ad371349837c2d6865ae319ec4bad71f6f7f Mon Sep 17 00:00:00 2001
> From: Andrew Bartlett <abartlet at samba.org>
> Date: Mon, 23 Feb 2015 16:22:29 +1300
> Subject: [PATCH 3/4] testprogs-test_chgdcpass.sh: Improve comments to explain
>  why we check about changing the password twice
> 
> Signed-off-by: Andrew Bartlett <abartlet at samba.org>
> ---
>  testprogs/blackbox/test_chgdcpass.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/testprogs/blackbox/test_chgdcpass.sh b/testprogs/blackbox/test_chgdcpass.sh
> index 3516125..ca7987e 100755
> --- a/testprogs/blackbox/test_chgdcpass.sh
> +++ b/testprogs/blackbox/test_chgdcpass.sh
> @@ -96,7 +96,8 @@ test_drs options "Test drs options with new password" || failed=`expr $failed +
>  
>  testit "change dc password (2nd time)" $samba4srcdir/scripting/devel/chgtdcpass -s $PROVDIR/etc/smb.conf || failed=`expr $failed + 1`
>  
> -#This is important because it shows that the old ticket remains valid (as it must) for incoming connections after the DC pass
> +# This is important because it shows that the old ticket is discarded if the server rejects it (as it must) after the password was changed twice in succession.
> +# This also ensures we handle the case where the domain is re-provisioned etc
>  test_smbclient "Test login with kerberos ccache after 2nd password change" 'ls' -k yes || failed=`expr $failed + 1`
>  
>  #check that drs bind works after we change the password a 2nd time
> -- 
> 2.1.3
> 

> From f61107aba18530f2fb044d1fe5727babaee73c8b Mon Sep 17 00:00:00 2001
> From: Andrew Bartlett <abartlet at samba.org>
> Date: Mon, 23 Feb 2015 16:50:43 +1300
> Subject: [PATCH 4/4] s4/scripting/devel: Add tool to roll over the krbtgt
>  password
> 
> This may be handy if this key is compromised, or along with chgtdcpass to isolate test copies
> of production domains in such a way that they cannot mix.
> 
> Signed-off-by: Andrew Bartlett <abartlet at samba.org>
> ---
>  python/samba/upgradehelpers.py        | 19 +++++++++++
>  source4/scripting/devel/chgkrbtgtpass | 64 +++++++++++++++++++++++++++++++++++
>  2 files changed, 83 insertions(+)
>  create mode 100644 source4/scripting/devel/chgkrbtgtpass
> 
> diff --git a/python/samba/upgradehelpers.py b/python/samba/upgradehelpers.py
> index ed63c25..3b664fe 100644
> --- a/python/samba/upgradehelpers.py
> +++ b/python/samba/upgradehelpers.py
> @@ -637,6 +637,25 @@ def update_dns_account_password(samdb, secrets_ldb, names):
>  
>          secrets_ldb.modify(msg)
>  
> +def update_krbtgt_account_password(samdb, names):
> +    """Update (change) the password of the krbtgt account
> +
> +    :param samdb: An LDB object related to the sam.ldb file of a given provision
> +    :param names: List of key provision parameters"""
> +
> +    expression = "samAccountName=krbtgt"
> +    res = samdb.search(expression=expression, attrs=[])
> +    assert(len(res) == 1)
> +
> +    msg = ldb.Message(res[0].dn)
> +    machinepass = samba.generate_random_password(128, 255)
> +    mputf16 = machinepass.encode('utf-16-le')
> +    msg["clearTextPassword"] = ldb.MessageElement(mputf16,
> +                                                  ldb.FLAG_MOD_REPLACE,
> +                                                  "clearTextPassword")
> +
> +    samdb.modify(msg)
> +
>  def search_constructed_attrs_stored(samdb, rootdn, attrs):
>      """Search a given sam DB for calculated attributes that are
>      still stored in the db.
> diff --git a/source4/scripting/devel/chgkrbtgtpass b/source4/scripting/devel/chgkrbtgtpass
> new file mode 100644
> index 0000000..28d8846
> --- /dev/null
> +++ b/source4/scripting/devel/chgkrbtgtpass
> @@ -0,0 +1,64 @@
> +#!/usr/bin/env python
> +#
> +# Copyright (C) Matthieu Patou <mat at matws.net>  2010
> +# Copyright (C) Andrew Bartlett <abartlet at samba.org>  2015
> +#
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +
> +__docformat__ = "restructuredText"
> +
> +
> +import optparse
> +import sys
> +# Allow to run from s4 source directory (without installing samba)
> +sys.path.insert(0, "bin/python")
> +
> +import samba.getopt as options
> +from samba.credentials import DONT_USE_KERBEROS
> +from samba.auth import system_session
> +from samba import param
> +from samba.provision import find_provision_key_parameters
> +from samba.upgradehelpers import (get_paths,
> +                                  get_ldbs,
> +                                 update_krbtgt_account_password)
> +
> +parser = optparse.OptionParser("chgkrbtgtpass [options]")
> +sambaopts = options.SambaOptions(parser)
> +parser.add_option_group(sambaopts)
> +parser.add_option_group(options.VersionOptions(parser))
> +credopts = options.CredentialsOptions(parser)
> +parser.add_option_group(credopts)
> +
> +opts = parser.parse_args()[0]
> +
> +lp = sambaopts.get_loadparm()
> +smbconf = lp.configfile
> +creds = credopts.get_credentials(lp)
> +creds.set_kerberos_state(DONT_USE_KERBEROS)
> +
> +
> +if __name__ == '__main__':
> +    paths = get_paths(param, smbconf=smbconf)
> +    session = system_session()
> +
> +    ldbs = get_ldbs(paths, creds, session, lp)
> +    ldbs.startTransactions()
> +
> +    names = find_provision_key_parameters(ldbs.sam, ldbs.secrets, ldbs.idmap,
> +                                            paths, smbconf, lp)
> +
> +    update_krbtgt_account_password(ldbs.sam, names)
> +    ldbs.groupedCommit()
> -- 
> 2.1.3
> 


-- 
Jelmer Vernooij <jelmer at samba.org> - https://jelmer.uk/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150224/469070dd/attachment.pgp>


More information about the samba-technical mailing list