The branch, master has been updated
       via  a0e4bdc auth/credentials: Improve memory handling in 
cli_credentials_set_machine_account
       via  3a303ae5 selftest: Add a test for smbclient --machine-pass without 
secrets.tdb
       via  bcc29f9 auth/credentials: Avoid double-free in the failure case
      from  ba862f4 s3-smbd: Fix flooding the logs with records we don't find 
in pcap.

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit a0e4bdcb5b374a4259164aed8fdbcc7b1761f09b
Author: Andrew Bartlett <[email protected]>
Date:   Wed Aug 29 09:21:52 2012 +1000

    auth/credentials: Improve memory handling in 
cli_credentials_set_machine_account
    
    By using a tempoary talloc context this is much tidier and more reliable 
code.
    
    Andrew Bartlett
    
    Autobuild-User(master): Andrew Bartlett <[email protected]>
    Autobuild-Date(master): Wed Aug 29 03:11:10 CEST 2012 on sn-devel-104

commit 3a303ae5ab2bfef58e0ea281e3a99406ff8fd53f
Author: Andrew Bartlett <[email protected]>
Date:   Wed Aug 29 09:10:40 2012 +1000

    selftest: Add a test for smbclient --machine-pass without secrets.tdb
    
    Errors in handling the upgrade case without a matching secrets.tdb caused 
segfaults
    in the server.  This essentially tests both sides.
    
    Andrew Bartlett

commit bcc29f9e7317601737858184f5ec6243552e0c0c
Author: Andrew Bartlett <[email protected]>
Date:   Wed Aug 29 09:09:10 2012 +1000

    auth/credentials: Avoid double-free in the failure case
    
    This pointer is only valid if dbwrap_fetch returned success.
    
    Andrew Bartlett

-----------------------------------------------------------------------

Summary of changes:
 auth/credentials/credentials_secrets.c |   52 ++++++++++++++++----------------
 selftest/target/Samba4.pm              |    9 +++++
 source4/selftest/tests.py              |    2 +-
 3 files changed, 36 insertions(+), 27 deletions(-)


Changeset truncated at 500 lines:

diff --git a/auth/credentials/credentials_secrets.c 
b/auth/credentials/credentials_secrets.c
index 3304200..8c8c567 100644
--- a/auth/credentials/credentials_secrets.c
+++ b/auth/credentials/credentials_secrets.c
@@ -73,7 +73,7 @@ _PUBLIC_ NTSTATUS cli_credentials_set_secrets(struct 
cli_credentials *cred,
        /* some other parts of the system will key off this */
        cred->machine_account = true;
 
-       mem_ctx = talloc_named(cred, 0, "cli_credentials fetch machine 
password");
+       mem_ctx = talloc_named(cred, 0, "cli_credentials_set_secrets from ldb");
 
        if (!ldb) {
                /* Local secrets are stored in secrets.ldb */
@@ -209,10 +209,21 @@ _PUBLIC_ NTSTATUS 
cli_credentials_set_machine_account(struct cli_credentials *cr
        char *secrets_tdb_password = NULL;
        char *keystr;
        char *keystr_upper = NULL;
-       char *secrets_tdb = lpcfg_private_path(cred, lp_ctx, "secrets.tdb");
-       struct db_context *db_ctx = dbwrap_local_open(cred, lp_ctx, 
secrets_tdb, 0,
-                                                     TDB_DEFAULT, O_RDWR, 0600,
-                                                     DBWRAP_LOCK_ORDER_1);
+       char *secrets_tdb;
+       struct db_context *db_ctx;
+       TALLOC_CTX *tmp_ctx = talloc_named(cred, 0, 
"cli_credentials_set_secrets from ldb");
+       if (!tmp_ctx) {
+               return NT_STATUS_NO_MEMORY;
+       }
+       secrets_tdb = lpcfg_private_path(cred, lp_ctx, "secrets.tdb");
+       if (!secrets_tdb) {
+               TALLOC_FREE(tmp_ctx);
+               return NT_STATUS_NO_MEMORY;
+       }
+               
+       db_ctx = dbwrap_local_open(cred, lp_ctx, secrets_tdb, 0,
+                                  TDB_DEFAULT, O_RDWR, 0600,
+                                  DBWRAP_LOCK_ORDER_1);
        /* Bleh, nasty recursion issues: We are setting a machine
         * account here, so we don't want the 'pending' flag around
         * any more */
@@ -225,25 +236,21 @@ _PUBLIC_ NTSTATUS 
cli_credentials_set_machine_account(struct cli_credentials *cr
 
        if (db_ctx) {
                TDB_DATA dbuf;
-               keystr = talloc_asprintf(cred, "%s/%s",
+               keystr = talloc_asprintf(tmp_ctx, "%s/%s",
                                         SECRETS_MACHINE_LAST_CHANGE_TIME,
                                         domain);
-               keystr_upper = strupper_talloc(cred, keystr);
-               TALLOC_FREE(keystr);
-               status = dbwrap_fetch(db_ctx, cred, 
string_tdb_data(keystr_upper),
+               keystr_upper = strupper_talloc(tmp_ctx, keystr);
+               status = dbwrap_fetch(db_ctx, tmp_ctx, 
string_tdb_data(keystr_upper),
                                      &dbuf);
-               TALLOC_FREE(keystr_upper);
                if (NT_STATUS_IS_OK(status) && dbuf.dsize == 4) {
                        secrets_tdb_lct = IVAL(dbuf.dptr,0);
                }
-               TALLOC_FREE(dbuf.dptr);
 
-               keystr = talloc_asprintf(cred, "%s/%s",
+               keystr = talloc_asprintf(tmp_ctx, "%s/%s",
                                         SECRETS_MACHINE_PASSWORD,
                                         domain);
-               keystr_upper = strupper_talloc(cred, keystr);
-               TALLOC_FREE(keystr);
-               status = dbwrap_fetch(db_ctx, cred, 
string_tdb_data(keystr_upper),
+               keystr_upper = strupper_talloc(tmp_ctx, keystr);
+               status = dbwrap_fetch(db_ctx, tmp_ctx, 
string_tdb_data(keystr_upper),
                                      &dbuf);
                if (NT_STATUS_IS_OK(status)) {
                        secrets_tdb_password = (char *)dbuf.dptr;
@@ -269,15 +276,13 @@ _PUBLIC_ NTSTATUS 
cli_credentials_set_machine_account(struct cli_credentials *cr
        }
 
        if (secrets_tdb_password_more_recent) {
-               char *machine_account = talloc_asprintf(cred, "%s$", 
lpcfg_netbios_name(lp_ctx));
+               char *machine_account = talloc_asprintf(tmp_ctx, "%s$", 
lpcfg_netbios_name(lp_ctx));
                cli_credentials_set_password(cred, secrets_tdb_password, 
CRED_SPECIFIED);
                cli_credentials_set_domain(cred, domain, CRED_SPECIFIED);
                cli_credentials_set_realm(cred, realm, CRED_SPECIFIED);
                cli_credentials_set_workstation(cred, 
lpcfg_netbios_name(lp_ctx), CRED_SPECIFIED);
                cli_credentials_set_username(cred, machine_account, 
CRED_SPECIFIED);
-               TALLOC_FREE(machine_account);
-       } else if (NT_STATUS_EQUAL(NT_STATUS_CANT_ACCESS_DOMAIN_INFO, status)
-                  || NT_STATUS_EQUAL(NT_STATUS_NOT_FOUND, status)) {
+       } else if (!NT_STATUS_IS_OK(status)) {
                if (db_ctx) {
                        error_string = talloc_asprintf(cred,
                                                       "Failed to fetch machine 
account password from "
@@ -289,16 +294,11 @@ _PUBLIC_ NTSTATUS 
cli_credentials_set_machine_account(struct cli_credentials *cr
                                                       "secrets.ldb: %s and 
failed to open %s",
                                                       error_string, 
secrets_tdb);
                }
-       }
-       
-       TALLOC_FREE(secrets_tdb_password);
-       TALLOC_FREE(secrets_tdb);
-       TALLOC_FREE(db_ctx);
-       if (!NT_STATUS_IS_OK(status)) {
                DEBUG(1, ("Could not find machine account in secrets database: 
%s: %s\n", 
                          error_string, nt_errstr(status)));
-               talloc_free(error_string);
        }
+       
+       TALLOC_FREE(tmp_ctx);
        return status;
 }
 
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 5442281..28b40ad 100644
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -672,6 +672,7 @@ nogroup:x:65534:nobody
                LOCKDIR => $ctx->{lockdir},
                STATEDIR => $ctx->{statedir},
                CACHEDIR => $ctx->{cachedir},
+               PRIVATEDIR => $ctx->{privatedir},
                SERVERCONFFILE => $ctx->{smb_conf},
                CONFIGURATION => $configuration,
                SOCKET_WRAPPER_DEFAULT_IFACE => $ctx->{swiface},
@@ -1450,6 +1451,14 @@ sub provision_chgdcpass($$)
                warn("Unable to add wins configuration");
                return undef;
        }
+       
+       # Remove secrets.tdb from this environment to test that we still start 
up
+       # on systems without the new matching secrets.tdb records
+       unless (unlink("$ret->{PRIVATEDIR}/secrets.tdb")) {
+               warn("Unable to remove $ret->{PRIVATEDIR}/secrets.tdb added 
during provision");
+               return undef;
+       }
+           
        $ret->{DC_SERVER} = $ret->{SERVER};
        $ret->{DC_SERVER_IP} = $ret->{SERVER_IP};
        $ret->{DC_NETBIOSNAME} = $ret->{NETBIOSNAME};
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index f205fe4..b9944cb 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -304,7 +304,7 @@ planpythontestsuite("s3dc", 
"samba.tests.libsmb_samba_internal");
 # the API. These mainly test that the various command-line options of commands
 # work correctly.
 
-for env in ["s3member", "s4member", "dc"]:
+for env in ["s3member", "s4member", "dc", "chgdcpass"]:
     plantestsuite("samba4.blackbox.smbclient(%s:local)" % env, "%s:local" % 
env, [os.path.join(samba4srcdir, "utils/tests/test_smbclient.sh"), '$SERVER', 
'$SERVER_IP', '$USERNAME', '$PASSWORD', '$DOMAIN', smbclient])
 
 planpythontestsuite("none", "samba.tests.blackbox.ndrdump")


-- 
Samba Shared Repository

Reply via email to