[PATCH] lsa4_srv: Factor out dcesrc_lsa_valid_AccountRight()

vl at samba.org vl at samba.org
Thu Apr 27 15:28:55 UTC 2017


Hi!

Push appreciated!

Thanks, Volker
-------------- next part --------------
From 59034e3d049b073308d76460f11433555c36f348 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 27 Apr 2017 13:37:42 +0200
Subject: [PATCH] lsa4_srv: Factor out dcesrc_lsa_valid_AccountRight()

The previous code in dcesrv_lsa_AddRemoveAccountRights had the following snippet:

if (sec_privilege_id(rights->names[i].string) == SEC_PRIV_INVALID) {
        if (sec_right_bit(rights->names[i].string) == 0) {
                talloc_free(msg);
                return NT_STATUS_NO_SUCH_PRIVILEGE;
        }
        talloc_free(msg);
        return NT_STATUS_NO_SUCH_PRIVILEGE;
}

If I'm not mistaken, the inner if-statement is essentially dead code,
as regardless of the outcome of the if-condition we execute the same
code. The effect of this is that you can't "net rpc rights grant" a right,
for example SeInteractiveLogonRight. A quick test against a W2k12 server
shows that W2k12 allows this call.

This patch changes the semantics of dcesrv_lsa_AddRemoveAccountRights
to also allow "rights" to be granted and revoked. At the same
time, it centralizes the check for validity of user input from
dcesrv_lsa_EnumAccountsWithUserRight into dcesrc_lsa_valid_AccountRight
too.

Signed-off-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
 source4/rpc_server/lsa/dcesrv_lsa.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/source4/rpc_server/lsa/dcesrv_lsa.c b/source4/rpc_server/lsa/dcesrv_lsa.c
index f013d99..3cccd57 100644
--- a/source4/rpc_server/lsa/dcesrv_lsa.c
+++ b/source4/rpc_server/lsa/dcesrv_lsa.c
@@ -99,6 +99,24 @@ struct lsa_trusted_domain_state {
 	struct ldb_dn *trusted_domain_user_dn;
 };
 
+static bool dcesrc_lsa_valid_AccountRight(const char *right)
+{
+	enum sec_privilege priv_id;
+	uint32_t right_bit;
+
+	priv_id = sec_privilege_id(right);
+	if (priv_id != SEC_PRIV_INVALID) {
+		return true;
+	}
+
+	right_bit = sec_right_bit(right);
+	if (right_bit != 0) {
+		return true;
+	}
+
+	return false;
+}
+
 /*
   this is based on the samba3 function make_lsa_object_sd()
   It uses the same logic, but with samba4 helper functions
@@ -2949,12 +2967,10 @@ static NTSTATUS dcesrv_lsa_AddRemoveAccountRights(struct dcesrv_call_state *dce_
 	}
 
 	for (i=0;i<rights->count;i++) {
-		if (sec_privilege_id(rights->names[i].string) == SEC_PRIV_INVALID) {
-			if (sec_right_bit(rights->names[i].string) == 0) {
-				talloc_free(msg);
-				return NT_STATUS_NO_SUCH_PRIVILEGE;
-			}
+		bool ok;
 
+		ok = dcesrc_lsa_valid_AccountRight(rights->names[i].string);
+		if (!ok) {
 			talloc_free(msg);
 			return NT_STATUS_NO_SUCH_PRIVILEGE;
 		}
@@ -3856,6 +3872,7 @@ static NTSTATUS dcesrv_lsa_EnumAccountsWithUserRight(struct dcesrv_call_state *d
 	struct ldb_message **res;
 	const char * const attrs[] = { "objectSid", NULL};
 	const char *privname;
+	bool ok;
 
 	DCESRV_PULL_HANDLE(h, r->in.handle, LSA_HANDLE_POLICY);
 
@@ -3866,7 +3883,9 @@ static NTSTATUS dcesrv_lsa_EnumAccountsWithUserRight(struct dcesrv_call_state *d
 	}
 
 	privname = r->in.name->string;
-	if (sec_privilege_id(privname) == SEC_PRIV_INVALID && sec_right_bit(privname) == 0) {
+
+	ok = dcesrc_lsa_valid_AccountRight(privname);
+	if (!ok) {
 		return NT_STATUS_NO_SUCH_PRIVILEGE;
 	}
 
-- 
2.1.4



More information about the samba-technical mailing list