[PATCH] Preparatory patches for winbindd and trusts
Andreas Schneider
asn at samba.org
Thu Jan 11 13:35:47 UTC 2018
On Thursday, 11 January 2018 13:15:14 CET Andreas Schneider via samba-
technical wrote:
> On Thursday, 11 January 2018 09:17:52 CET Andreas Schneider via samba-
>
> technical wrote:
> > On Wednesday, 10 January 2018 20:44:49 CET Ralph Böhme via samba-technical
> >
> > wrote:
> > > Hi!
> > >
> > > Attached is a preparatory patchset from my winbindd-trusts branch [1],
> > > that
> > > prepares winbindd for a possible later change to remove trust
> > > enumeration.
> >
> > Hi,
> >
> > I have some minor things and an additional patch. Ping me on IRC please.
>
> Here are some additional patches to improve the code.
Updated set.
--
Andreas Schneider GPG-ID: CC014E3D
Samba Team asn at samba.org
www.samba.org
-------------- next part --------------
>From 75f90bd458d2e5b6262b0c76849212d87eaadb23 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 11 Jan 2018 09:23:05 +0100
Subject: [PATCH 1/5] s3:winbindd: Improve logic so it is easier to understand
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/winbindd/winbindd_pam.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c
index de3e3f5cc81..f60a90ceb18 100644
--- a/source3/winbindd/winbindd_pam.c
+++ b/source3/winbindd/winbindd_pam.c
@@ -1683,22 +1683,24 @@ static NTSTATUS winbindd_dual_pam_auth_samlogon(
true, /* interactive */
&authoritative,
&info3);
- if (NT_STATUS_IS_OK(result)) {
- result = map_info3_to_validation(mem_ctx,
- info3,
- &validation_level,
- &validation);
- TALLOC_FREE(info3);
- if (!NT_STATUS_IS_OK(result)) {
- goto done;
- }
- }
/*
* We need to try the remote NETLOGON server if this is
* not authoritative (for example on the RODC).
*/
if (authoritative != 0) {
+ if (NT_STATUS_IS_OK(result)) {
+ result = map_info3_to_validation(
+ mem_ctx,
+ info3,
+ &validation_level,
+ &validation);
+ TALLOC_FREE(info3);
+ if (!NT_STATUS_IS_OK(result)) {
+ goto done;
+ }
+ }
+
goto done;
}
}
--
2.15.1
>From 0290a0dfd866f1b4980c264eb273832e9b24ad1d Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 11 Jan 2018 09:27:50 +0100
Subject: [PATCH 2/5] s3:winbind: Use a goto for cleaning up at the end
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/winbindd/winbindd_pam.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c
index f60a90ceb18..c7d2ca434fc 100644
--- a/source3/winbindd/winbindd_pam.c
+++ b/source3/winbindd/winbindd_pam.c
@@ -889,14 +889,14 @@ NTSTATUS append_auth_data(TALLOC_CTX *mem_ctx,
const char *name_user)
{
struct netr_SamInfo3 *info3 = NULL;
- NTSTATUS result;
+ NTSTATUS result = NT_STATUS_UNSUCCESSFUL;
result = map_validation_to_info3(talloc_tos(),
validation_level,
validation,
&info3);
if (!NT_STATUS_IS_OK(result)) {
- return result;
+ goto out;
}
if (request_flags & WBFLAG_PAM_USER_SESSION_KEY) {
@@ -919,8 +919,7 @@ NTSTATUS append_auth_data(TALLOC_CTX *mem_ctx,
if (!NT_STATUS_IS_OK(result)) {
DEBUG(10,("Failed to append Unix Username: %s\n",
nt_errstr(result)));
- TALLOC_FREE(info3);
- return result;
+ goto out;
}
}
@@ -931,8 +930,7 @@ NTSTATUS append_auth_data(TALLOC_CTX *mem_ctx,
if (!NT_STATUS_IS_OK(result)) {
DEBUG(10,("Failed to append INFO3 (NDR): %s\n",
nt_errstr(result)));
- TALLOC_FREE(info3);
- return result;
+ goto out;
}
}
@@ -943,8 +941,7 @@ NTSTATUS append_auth_data(TALLOC_CTX *mem_ctx,
if (!NT_STATUS_IS_OK(result)) {
DEBUG(10,("Failed to append INFO3 (TXT): %s\n",
nt_errstr(result)));
- TALLOC_FREE(info3);
- return result;
+ goto out;
}
}
@@ -954,13 +951,14 @@ NTSTATUS append_auth_data(TALLOC_CTX *mem_ctx,
if (!NT_STATUS_IS_OK(result)) {
DEBUG(10,("Failed to append AFS token: %s\n",
nt_errstr(result)));
- TALLOC_FREE(info3);
- return result;
+ goto out;
}
}
+ result = NT_STATUS_OK;
+out:
TALLOC_FREE(info3);
- return NT_STATUS_OK;
+ return result;
}
static NTSTATUS winbindd_dual_pam_auth_cached(struct winbindd_domain *domain,
--
2.15.1
>From 08d643bcfd3d26237b240e539383c420a57aeb0e Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 11 Jan 2018 09:37:22 +0100
Subject: [PATCH 3/5] s3:winbind: Use a stackframe and cleanup when leaving
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/winbindd/winbindd_pam.c | 39 ++++++++++++++++++++-------------------
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c
index c7d2ca434fc..01d08c1e2b3 100644
--- a/source3/winbindd/winbindd_pam.c
+++ b/source3/winbindd/winbindd_pam.c
@@ -56,16 +56,17 @@ static NTSTATUS append_info3_as_txt(TALLOC_CTX *mem_ctx,
union netr_Validation *validation)
{
struct netr_SamInfo3 *info3 = NULL;
- char *ex;
+ char *ex = NULL;
uint32_t i;
- NTSTATUS status;
+ NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
+ TALLOC_CTX *frame = talloc_stackframe();
- status = map_validation_to_info3(talloc_tos(),
+ status = map_validation_to_info3(frame,
validation_level,
validation,
&info3);
if (!NT_STATUS_IS_OK(status)) {
- return status;
+ goto out;
}
resp->data.auth.info3.logon_time =
@@ -120,10 +121,10 @@ static NTSTATUS append_info3_as_txt(TALLOC_CTX *mem_ctx,
validation->sam6->principal_name.string);
}
- ex = talloc_strdup(mem_ctx, "");
+ ex = talloc_strdup(frame, "");
if (ex == NULL) {
- TALLOC_FREE(info3);
- return NT_STATUS_NO_MEMORY;
+ status = NT_STATUS_NO_MEMORY;
+ goto out;
}
for (i=0; i < info3->base.groups.count; i++) {
@@ -131,36 +132,36 @@ static NTSTATUS append_info3_as_txt(TALLOC_CTX *mem_ctx,
info3->base.groups.rids[i].rid,
info3->base.groups.rids[i].attributes);
if (ex == NULL) {
- TALLOC_FREE(info3);
- return NT_STATUS_NO_MEMORY;
+ status = NT_STATUS_NO_MEMORY;
+ goto out;
}
}
for (i=0; i < info3->sidcount; i++) {
char *sid;
- sid = dom_sid_string(mem_ctx, info3->sids[i].sid);
+ sid = dom_sid_string(frame, info3->sids[i].sid);
if (sid == NULL) {
- TALLOC_FREE(info3);
- return NT_STATUS_NO_MEMORY;
+ status = NT_STATUS_NO_MEMORY;
+ goto out;
}
ex = talloc_asprintf_append_buffer(ex, "%s:0x%08X\n",
sid,
info3->sids[i].attributes);
if (ex == NULL) {
- TALLOC_FREE(info3);
- return NT_STATUS_NO_MEMORY;
+ status = NT_STATUS_NO_MEMORY;
+ goto out;
}
-
- talloc_free(sid);
}
- resp->extra_data.data = ex;
resp->length += talloc_get_size(ex);
+ resp->extra_data.data = talloc_move(mem_ctx, &ex);
- TALLOC_FREE(info3);
- return NT_STATUS_OK;
+ status = NT_STATUS_OK;
+out:
+ TALLOC_FREE(frame);
+ return status;
}
static NTSTATUS append_info3_as_ndr(TALLOC_CTX *mem_ctx,
--
2.15.1
>From 1717f2be13fd30fe0732669c6a9f68d660fc263d Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 11 Jan 2018 09:06:31 +0100
Subject: [PATCH 4/5] s3:rpc_client: Clenup copy_netr_SamInfo3() code
This gets rid of some strange macro and makes sure we clenaup at the
end.
Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source3/auth/auth_util.c | 14 ++++---
source3/auth/server_info.c | 45 +++++++++++++--------
source3/rpc_client/util_netlogon.c | 80 ++++++++++++++++++++++----------------
source3/rpc_client/util_netlogon.h | 5 ++-
source3/winbindd/winbindd_pam.c | 9 +++--
5 files changed, 92 insertions(+), 61 deletions(-)
diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
index 5bb5a69dfa7..f543b33eead 100644
--- a/source3/auth/auth_util.c
+++ b/source3/auth/auth_util.c
@@ -1008,6 +1008,7 @@ static struct auth_serversupplied_info *copy_session_info_serverinfo_guest(TALLO
struct auth_serversupplied_info *server_info)
{
struct auth_serversupplied_info *dst;
+ NTSTATUS status;
dst = make_server_info(mem_ctx);
if (dst == NULL) {
@@ -1055,8 +1056,10 @@ static struct auth_serversupplied_info *copy_session_info_serverinfo_guest(TALLO
dst->lm_session_key = data_blob_talloc(dst, src->session_key.data,
src->session_key.length);
- dst->info3 = copy_netr_SamInfo3(dst, server_info->info3);
- if (!dst->info3) {
+ status = copy_netr_SamInfo3(dst,
+ server_info->info3,
+ &dst->info3);
+ if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(dst);
return NULL;
}
@@ -1433,9 +1436,10 @@ NTSTATUS make_server_info_info3(TALLOC_CTX *mem_ctx,
result->unix_name = talloc_strdup(result, found_username);
/* copy in the info3 */
- result->info3 = copy_netr_SamInfo3(result, info3);
- if (result->info3 == NULL) {
- nt_status = NT_STATUS_NO_MEMORY;
+ nt_status = copy_netr_SamInfo3(result,
+ info3,
+ &result->info3);
+ if (!NT_STATUS_IS_OK(nt_status)) {
goto out;
}
diff --git a/source3/auth/server_info.c b/source3/auth/server_info.c
index 20d43d237fa..78981751286 100644
--- a/source3/auth/server_info.c
+++ b/source3/auth/server_info.c
@@ -63,11 +63,14 @@ struct auth_serversupplied_info *make_server_info(TALLOC_CTX *mem_ctx)
NTSTATUS serverinfo_to_SamInfo2(struct auth_serversupplied_info *server_info,
struct netr_SamInfo2 *sam2)
{
- struct netr_SamInfo3 *info3;
+ struct netr_SamInfo3 *info3 = NULL;
+ NTSTATUS status;
- info3 = copy_netr_SamInfo3(sam2, server_info->info3);
- if (!info3) {
- return NT_STATUS_NO_MEMORY;
+ status = copy_netr_SamInfo3(sam2,
+ server_info->info3,
+ &info3);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
}
if (server_info->session_key.length) {
@@ -96,11 +99,14 @@ NTSTATUS serverinfo_to_SamInfo2(struct auth_serversupplied_info *server_info,
NTSTATUS serverinfo_to_SamInfo3(const struct auth_serversupplied_info *server_info,
struct netr_SamInfo3 *sam3)
{
- struct netr_SamInfo3 *info3;
+ struct netr_SamInfo3 *info3 = NULL;
+ NTSTATUS status;
- info3 = copy_netr_SamInfo3(sam3, server_info->info3);
- if (!info3) {
- return NT_STATUS_NO_MEMORY;
+ status = copy_netr_SamInfo3(sam3,
+ server_info->info3,
+ &info3);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
}
if (server_info->session_key.length) {
@@ -133,7 +139,8 @@ NTSTATUS serverinfo_to_SamInfo6(struct auth_serversupplied_info *server_info,
struct netr_SamInfo6 *sam6)
{
struct pdb_domain_info *dominfo;
- struct netr_SamInfo3 *info3;
+ struct netr_SamInfo3 *info3 = NULL;
+ NTSTATUS status;
if ((pdb_capabilities() & PDB_CAP_ADS) == 0) {
DEBUG(10,("Not adding validation info level 6 "
@@ -146,9 +153,11 @@ NTSTATUS serverinfo_to_SamInfo6(struct auth_serversupplied_info *server_info,
return NT_STATUS_NO_MEMORY;
}
- info3 = copy_netr_SamInfo3(sam6, server_info->info3);
- if (!info3) {
- return NT_STATUS_NO_MEMORY;
+ status = copy_netr_SamInfo3(sam6,
+ server_info->info3,
+ &info3);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
}
if (server_info->session_key.length) {
@@ -335,11 +344,15 @@ NTSTATUS create_info3_from_pac_logon_info(TALLOC_CTX *mem_ctx,
struct netr_SamInfo3 **pp_info3)
{
NTSTATUS status;
- struct netr_SamInfo3 *info3 = copy_netr_SamInfo3(mem_ctx,
- &logon_info->info3);
- if (info3 == NULL) {
- return NT_STATUS_NO_MEMORY;
+ struct netr_SamInfo3 *info3 = NULL;
+
+ status = copy_netr_SamInfo3(mem_ctx,
+ &logon_info->info3,
+ &info3);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
}
+
status = merge_resource_sids(logon_info, info3);
if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(info3);
diff --git a/source3/rpc_client/util_netlogon.c b/source3/rpc_client/util_netlogon.c
index ac804f84196..15c769ffe41 100644
--- a/source3/rpc_client/util_netlogon.c
+++ b/source3/rpc_client/util_netlogon.c
@@ -62,45 +62,52 @@ NTSTATUS copy_netr_SamBaseInfo(TALLOC_CTX *mem_ctx,
return NT_STATUS_OK;
}
-#undef RET_NOMEM
-
-#define RET_NOMEM(ptr) do { \
- if (!ptr) { \
- TALLOC_FREE(info3); \
- return NULL; \
- } } while(0)
-
-struct netr_SamInfo3 *copy_netr_SamInfo3(TALLOC_CTX *mem_ctx,
- const struct netr_SamInfo3 *orig)
+NTSTATUS copy_netr_SamInfo3(TALLOC_CTX *mem_ctx,
+ const struct netr_SamInfo3 *in,
+ struct netr_SamInfo3 **pout)
{
- struct netr_SamInfo3 *info3;
+ struct netr_SamInfo3 *info3 = NULL;
unsigned int i;
- NTSTATUS status;
+ NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
info3 = talloc_zero(mem_ctx, struct netr_SamInfo3);
- if (!info3) return NULL;
+ if (info3 == NULL) {
+ status = NT_STATUS_NO_MEMORY;
+ goto out;
+ }
- status = copy_netr_SamBaseInfo(info3, &orig->base, &info3->base);
+ status = copy_netr_SamBaseInfo(info3, &in->base, &info3->base);
if (!NT_STATUS_IS_OK(status)) {
- TALLOC_FREE(info3);
- return NULL;
+ goto out;
}
- if (orig->sidcount) {
- info3->sidcount = orig->sidcount;
+ if (in->sidcount) {
+ info3->sidcount = in->sidcount;
info3->sids = talloc_array(info3, struct netr_SidAttr,
- orig->sidcount);
- RET_NOMEM(info3->sids);
- for (i = 0; i < orig->sidcount; i++) {
+ in->sidcount);
+ if (info3->sids == NULL) {
+ status = NT_STATUS_NO_MEMORY;
+ goto out;
+ }
+
+ for (i = 0; i < in->sidcount; i++) {
info3->sids[i].sid = dom_sid_dup(info3->sids,
- orig->sids[i].sid);
- RET_NOMEM(info3->sids[i].sid);
- info3->sids[i].attributes =
- orig->sids[i].attributes;
+ in->sids[i].sid);
+ if (info3->sids[i].sid == NULL) {
+ status = NT_STATUS_NO_MEMORY;
+ goto out;
+ }
+ info3->sids[i].attributes = in->sids[i].attributes;
}
}
- return info3;
+ *pout = info3;
+ info3 = NULL;
+
+ status = NT_STATUS_OK;
+out:
+ TALLOC_FREE(info3);
+ return status;
}
NTSTATUS map_validation_to_info3(TALLOC_CTX *mem_ctx,
@@ -108,7 +115,7 @@ NTSTATUS map_validation_to_info3(TALLOC_CTX *mem_ctx,
union netr_Validation *validation,
struct netr_SamInfo3 **info3_p)
{
- struct netr_SamInfo3 *info3;
+ struct netr_SamInfo3 *info3 = NULL;
struct netr_SamInfo6 *info6 = NULL;
NTSTATUS status;
@@ -122,10 +129,13 @@ NTSTATUS map_validation_to_info3(TALLOC_CTX *mem_ctx,
return NT_STATUS_INVALID_PARAMETER;
}
- info3 = copy_netr_SamInfo3(mem_ctx, validation->sam3);
- if (info3 == NULL) {
- return NT_STATUS_NO_MEMORY;
+ status = copy_netr_SamInfo3(mem_ctx,
+ validation->sam3,
+ &info3);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
}
+
break;
case 6:
if (validation->sam6 == NULL) {
@@ -186,16 +196,18 @@ NTSTATUS map_info3_to_validation(TALLOC_CTX *mem_ctx,
union netr_Validation **_validation)
{
union netr_Validation *validation = NULL;
+ NTSTATUS status;
validation = talloc_zero(mem_ctx, union netr_Validation);
if (validation == NULL) {
return NT_STATUS_NO_MEMORY;
}
- validation->sam3 = copy_netr_SamInfo3(mem_ctx, info3);
- if (validation->sam3 == NULL) {
- TALLOC_FREE(validation);
- return NT_STATUS_NO_MEMORY;
+ status = copy_netr_SamInfo3(mem_ctx,
+ info3,
+ &validation->sam3);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
}
* _validation_level = 3;
diff --git a/source3/rpc_client/util_netlogon.h b/source3/rpc_client/util_netlogon.h
index 80c7bff99d1..8b3a372ee8e 100644
--- a/source3/rpc_client/util_netlogon.h
+++ b/source3/rpc_client/util_netlogon.h
@@ -25,8 +25,9 @@
NTSTATUS copy_netr_SamBaseInfo(TALLOC_CTX *mem_ctx,
const struct netr_SamBaseInfo *in,
struct netr_SamBaseInfo *out);
-struct netr_SamInfo3 *copy_netr_SamInfo3(TALLOC_CTX *mem_ctx,
- const struct netr_SamInfo3 *orig);
+NTSTATUS copy_netr_SamInfo3(TALLOC_CTX *mem_ctx,
+ const struct netr_SamInfo3 *in,
+ struct netr_SamInfo3 **pout);
NTSTATUS map_validation_to_info3(TALLOC_CTX *mem_ctx,
uint16_t validation_level,
union netr_Validation *validation,
diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c
index 01d08c1e2b3..9a61cd3a578 100644
--- a/source3/winbindd/winbindd_pam.c
+++ b/source3/winbindd/winbindd_pam.c
@@ -2916,10 +2916,11 @@ NTSTATUS winbindd_pam_auth_pac_send(struct winbindd_cli_state *state,
* returning a copy talloc'ed off
* the state->mem_ctx.
*/
- info3_copy = copy_netr_SamInfo3(state->mem_ctx,
- &logon_info->info3);
- if (info3_copy == NULL) {
- return NT_STATUS_NO_MEMORY;
+ result = copy_netr_SamInfo3(state->mem_ctx,
+ &logon_info->info3,
+ &info3_copy);
+ if (!NT_STATUS_IS_OK(result)) {
+ return result;
}
}
}
--
2.15.1
>From 98a8a90902e2e2db2c7fb7712bb0bd305be0cdc5 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 11 Jan 2018 11:00:43 +0100
Subject: [PATCH 5/5] s3:test: Always validate the join after changing the
secret
Signed-off-by: Andreas Schneider <asn at samba.org>
Signed-off-by: Ralph Boehme <slow at samba.org>
Pair-Programmed-With: Ralph Boehme <slow at samba.org>
---
source3/script/tests/test_net_cred_change.sh | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/source3/script/tests/test_net_cred_change.sh b/source3/script/tests/test_net_cred_change.sh
index de56be51b9f..794b054139e 100755
--- a/source3/script/tests/test_net_cred_change.sh
+++ b/source3/script/tests/test_net_cred_change.sh
@@ -9,8 +9,9 @@ fi
incdir=`dirname $0`/../../../testprogs/blackbox
. $incdir/subunit.sh
-testit "first change" $VALGRIND $BINDIR/wbinfo -c || failed=`expr $failed + 1`
-testit "first join" $VALGRIND $BINDIR/net rpc testjoin $@ || failed=`expr $failed + 1`
-testit "second change" $VALGRIND $BINDIR/wbinfo -c || failed=`expr $failed + 1`
+testit "1: change machine secret" $VALGRIND $BINDIR/wbinfo --change-secret || failed=`expr $failed + 1`
+testit "1: validate secret" $VALGRIND $BINDIR/net rpc testjoin $@ || failed=`expr $failed + 1`
+testit "2: change machine secret" $VALGRIND $BINDIR/wbinfo --change-secret || failed=`expr $failed + 1`
+testit "2: validate secret" $VALGRIND $BINDIR/net rpc testjoin $@ || failed=`expr $failed + 1`
testok $0 $failed
--
2.15.1
More information about the samba-technical
mailing list