More kludge ACLs!

Rather than killing off the nasty 'kludge ACLs' stuff, this patch
extends it, to ensure that LSA secrets and the registry are also
protected.

Andrew Bartlett
(This used to be commit 2f2b110fb870132099bad1d4c16ed8962affb3ce)

5 files changed, 237 insertions, 124 deletions

diff --git a/source4/dsdb/samdb/ldb_modules/kludge_acl.c b/source4/dsdb/samdb/ldb_modules/kludge_acl.c
index e3e1f7ac88..e418031271 100644
--- a/source4/dsdb/samdb/ldb_modules/kludge_acl.c
+++ b/source4/dsdb/samdb/ldb_modules/kludge_acl.c
@@ -46,42 +46,15 @@
  *
  */
 
-enum user_is {
-	ANONYMOUS,
-	USER,
-	ADMINISTRATOR,
-	SYSTEM
-};
-
 struct kludge_private_data {
 	const char **password_attrs;
 };
 
-static enum user_is what_is_user(struct ldb_module *module) 
+static enum security_user_level what_is_user(struct ldb_module *module) 
 {
 	struct auth_session_info *session_info
 		= (struct auth_session_info *)ldb_get_opaque(module->ldb, "sessionInfo");
-	if (!session_info) {
-		return ANONYMOUS;
-	}
-	
-	if (security_token_is_system(session_info->security_token)) {
-		return SYSTEM;
-	}
-
-	if (security_token_is_anonymous(session_info->security_token)) {
-		return ANONYMOUS;
-	}
-
-	if (security_token_has_builtin_administrators(session_info->security_token)) {
-		return ADMINISTRATOR;
-	}
-
-	if (security_token_has_nt_authenticated_users(session_info->security_token)) {
-		return USER;
-	}
-
-	return ANONYMOUS;
+	return security_session_user_level(session_info);
 }
 
 static const char *user_name(TALLOC_CTX *mem_ctx, struct ldb_module *module) 
@@ -104,7 +77,7 @@ struct kludge_acl_context {
 	void *up_context;
 	int (*up_callback)(struct ldb_context *, void *, struct ldb_reply *);
 
-	enum user_is user_type;
+	enum security_user_level user_type;
 	bool allowedAttributes;
 	bool allowedAttributesEffective;
 	bool allowedChildClasses;
@@ -272,8 +245,8 @@ static int kludge_acl_callback(struct ldb_context *ldb, void *context, struct ld
 	if (data && data->password_attrs) /* if we are not initialized just get through */
 	{
 		switch (ac->user_type) {
-		case SYSTEM:
-		case ADMINISTRATOR:
+		case SECURITY_SYSTEM:
+		case SECURITY_ADMINISTRATOR:
 			if (ac->allowedAttributesEffective) {
 				ret = kludge_acl_allowedAttributes(ldb, ares->message, "allowedAttributesEffective");
 				if (ret != LDB_SUCCESS) {
@@ -359,7 +332,7 @@ static int kludge_acl_search(struct ldb_module *module, struct ldb_request *req)
 	   so we don't allow a search for 'sambaPassword=penguin',
 	   just as we would not allow that attribute to be returned */
 	switch (ac->user_type) {
-	case SYSTEM:
+	case SECURITY_SYSTEM:
 		break;
 	default:
 		/* remove password attributes */
@@ -391,10 +364,10 @@ static int kludge_acl_search(struct ldb_module *module, struct ldb_request *req)
 /* ANY change type */
 static int kludge_acl_change(struct ldb_module *module, struct ldb_request *req)
 {
-	enum user_is user_type = what_is_user(module);
+	enum security_user_level user_type = what_is_user(module);
 	switch (user_type) {
-	case SYSTEM:
-	case ADMINISTRATOR:
+	case SECURITY_SYSTEM:
+	case SECURITY_ADMINISTRATOR:
 		return ldb_next_request(module, req);
 	default:
 		ldb_asprintf_errstring(module->ldb,
diff --git a/source4/libcli/security/security.h b/source4/libcli/security/security.h
index d9485c825f..c7f2a09311 100644
--- a/source4/libcli/security/security.h
+++ b/source4/libcli/security/security.h
@@ -18,4 +18,12 @@
 */
 
 #include "librpc/gen_ndr/security.h"
+
+enum security_user_level {
+	SECURITY_ANONYMOUS,
+	SECURITY_USER,
+	SECURITY_ADMINISTRATOR,
+	SECURITY_SYSTEM
+};
+
 #include "libcli/security/proto.h"
diff --git a/source4/libcli/security/security_token.c b/source4/libcli/security/security_token.c
index e126340c46..0680c54258 100644
--- a/source4/libcli/security/security_token.c
+++ b/source4/libcli/security/security_token.c
@@ -23,6 +23,7 @@
 #include "includes.h"
 #include "dsdb/samdb/samdb.h"
 #include "libcli/security/security.h"
+#include "auth/session.h"
 
 /*
   return a blank security token
@@ -141,3 +142,29 @@ bool security_token_has_nt_authenticated_users(const struct security_token *toke
 {
 	return security_token_has_sid_string(token, SID_NT_AUTHENTICATED_USERS);
 }
+
+enum security_user_level security_session_user_level(struct auth_session_info *session_info) 
+{
+	if (!session_info) {
+		return SECURITY_ANONYMOUS;
+	}
+	
+	if (security_token_is_system(session_info->security_token)) {
+		return SECURITY_SYSTEM;
+	}
+
+	if (security_token_is_anonymous(session_info->security_token)) {
+		return SECURITY_ANONYMOUS;
+	}
+
+	if (security_token_has_builtin_administrators(session_info->security_token)) {
+		return SECURITY_ADMINISTRATOR;
+	}
+
+	if (security_token_has_nt_authenticated_users(session_info->security_token)) {
+		return SECURITY_USER;
+	}
+
+	return SECURITY_ANONYMOUS;
+}
+
diff --git a/source4/rpc_server/lsa/dcesrv_lsa.c b/source4/rpc_server/lsa/dcesrv_lsa.c
index 4d381ea978..429c413b98 100644
--- a/source4/rpc_server/lsa/dcesrv_lsa.c
+++ b/source4/rpc_server/lsa/dcesrv_lsa.c
@@ -99,8 +99,21 @@ static NTSTATUS dcesrv_lsa_Delete(struct dcesrv_call_state *dce_call, TALLOC_CTX
 	int ret;
 
 	DCESRV_PULL_HANDLE(h, r->in.handle, DCESRV_HANDLE_ANY);
+
 	if (h->wire_handle.handle_type == LSA_HANDLE_SECRET) {
 		struct lsa_secret_state *secret_state = h->data;
+
+		/* Ensure user is permitted to delete this... */
+		switch (security_session_user_level(dce_call->conn->auth_state.session_info))
+		{
+		case SECURITY_SYSTEM:
+		case SECURITY_ADMINISTRATOR:
+			break;
+		default:
+			/* Users and annonymous are not allowed delete things */
+			return NT_STATUS_ACCESS_DENIED;
+		}
+
 		ret = ldb_delete(secret_state->sam_ldb, 
 				 secret_state->secret_dn);
 		talloc_free(h);
@@ -446,6 +459,8 @@ static NTSTATUS dcesrv_lsa_ClearAuditLog(struct dcesrv_call_state *dce_call, TAL
 
 /* 
   lsa_CreateAccount 
+
+  This call does not seem to have any long-term effects, hence no database operations
 */
 static NTSTATUS dcesrv_lsa_CreateAccount(struct dcesrv_call_state *dce_call, TALLOC_CTX *mem_ctx,
 				  struct lsa_CreateAccount *r)
@@ -1673,6 +1688,16 @@ static NTSTATUS dcesrv_lsa_CreateSecret(struct dcesrv_call_state *dce_call, TALL
 	DCESRV_PULL_HANDLE(policy_handle, r->in.handle, LSA_HANDLE_POLICY);
 	ZERO_STRUCTP(r->out.sec_handle);
 	
+	switch (security_session_user_level(dce_call->conn->auth_state.session_info))
+	{
+	case SECURITY_SYSTEM:
+	case SECURITY_ADMINISTRATOR:
+		break;
+	default:
+		/* Users and annonymous are not allowed create secrets */
+		return NT_STATUS_ACCESS_DENIED;
+	}
+
 	policy_state = policy_handle->data;
 
 	if (!r->in.name.string) {
@@ -1819,6 +1844,16 @@ static NTSTATUS dcesrv_lsa_OpenSecret(struct dcesrv_call_state *dce_call, TALLOC
 		return NT_STATUS_INVALID_PARAMETER;
 	}
 	
+	switch (security_session_user_level(dce_call->conn->auth_state.session_info))
+	{
+	case SECURITY_SYSTEM:
+	case SECURITY_ADMINISTRATOR:
+		break;
+	default:
+		/* Users and annonymous are not allowed to access secrets */
+		return NT_STATUS_ACCESS_DENIED;
+	}
+
 	secret_state = talloc(mem_ctx, struct lsa_secret_state);
 	if (!secret_state) {
 		return NT_STATUS_NO_MEMORY;
@@ -1850,10 +1885,10 @@ static NTSTATUS dcesrv_lsa_OpenSecret(struct dcesrv_call_state *dce_call, TALLOC
 		}
 	
 	} else {
+		secret_state->global = false;
 		secret_state->sam_ldb = talloc_reference(secret_state, 
 				 secrets_db_connect(mem_ctx, dce_call->conn->dce_ctx->lp_ctx));
 
-		secret_state->global = false;
 		name = r->in.name.string;
 		if (strlen(name) < 1) {
 			return NT_STATUS_INVALID_PARAMETER;
@@ -2085,6 +2120,17 @@ static NTSTATUS dcesrv_lsa_QuerySecret(struct dcesrv_call_state *dce_call, TALLO
 
 	DCESRV_PULL_HANDLE(h, r->in.sec_handle, LSA_HANDLE_SECRET);
 
+	/* Ensure user is permitted to read this... */
+	switch (security_session_user_level(dce_call->conn->auth_state.session_info))
+	{
+	case SECURITY_SYSTEM:
+	case SECURITY_ADMINISTRATOR:
+		break;
+	default:
+		/* Users and annonymous are not allowed to read secrets */
+		return NT_STATUS_ACCESS_DENIED;
+	}
+
 	secret_state = h->data;
 
 	/* pull all the user attributes */
diff --git a/source4/rpc_server/winreg/rpc_winreg.c b/source4/rpc_server/winreg/rpc_winreg.c
index 681e3b918f..9993dc14c1 100644
--- a/source4/rpc_server/winreg/rpc_winreg.c
+++ b/source4/rpc_server/winreg/rpc_winreg.c
@@ -26,6 +26,7 @@
 #include "rpc_server/common/common.h"
 #include "librpc/gen_ndr/ndr_security.h"
 #include "param/param.h"
+#include "libcli/security/security.h"
 
 enum handle_types { HTYPE_REGVAL, HTYPE_REGKEY };
 
@@ -120,32 +121,39 @@ static WERROR dcesrv_winreg_CreateKey(struct dcesrv_call_state *dce_call,
 
 	newh = dcesrv_handle_new(dce_call->context, HTYPE_REGKEY);
 
-	/* the security descriptor is optional */
-	if (r->in.secdesc != NULL) {
-		DATA_BLOB sdblob;
-		enum ndr_err_code ndr_err;
-		sdblob.data = r->in.secdesc->sd.data;
-		sdblob.length = r->in.secdesc->sd.len;
-		if (sdblob.data == NULL) {
-			return WERR_INVALID_PARAM;
+	switch (security_session_user_level(dce_call->conn->auth_state.session_info))
+	{
+	case SECURITY_SYSTEM:
+	case SECURITY_ADMINISTRATOR:
+		/* the security descriptor is optional */
+		if (r->in.secdesc != NULL) {
+			DATA_BLOB sdblob;
+			enum ndr_err_code ndr_err;
+			sdblob.data = r->in.secdesc->sd.data;
+			sdblob.length = r->in.secdesc->sd.len;
+			if (sdblob.data == NULL) {
+				return WERR_INVALID_PARAM;
+			}
+			ndr_err = ndr_pull_struct_blob_all(&sdblob, mem_ctx, NULL, &sd,
+							   (ndr_pull_flags_fn_t)ndr_pull_security_descriptor);
+			if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+				return WERR_INVALID_PARAM;
+			}
 		}
-		ndr_err = ndr_pull_struct_blob_all(&sdblob, mem_ctx, NULL, &sd,
-						   (ndr_pull_flags_fn_t)ndr_pull_security_descriptor);
-		if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
-			return WERR_INVALID_PARAM;
+		
+		error = reg_key_add_name(newh, (struct registry_key *)h->data,
+					 r->in.name.name, NULL, r->in.secdesc?&sd:NULL,
+					 (struct registry_key **)&newh->data);
+		if (W_ERROR_IS_OK(error)) {
+			r->out.new_handle = &newh->wire_handle;
+		} else {
+			talloc_free(newh);
 		}
+		
+		return error;
+	default:
+		return WERR_ACCESS_DENIED;
 	}
-
-	error = reg_key_add_name(newh, (struct registry_key *)h->data,
-				 r->in.name.name, NULL, r->in.secdesc?&sd:NULL,
-				 (struct registry_key **)&newh->data);
-	if (W_ERROR_IS_OK(error)) {
-		r->out.new_handle = &newh->wire_handle;
-	} else {
-		talloc_free(newh);
-	}
-
-	return error;
 }
 
 
@@ -160,7 +168,14 @@ static WERROR dcesrv_winreg_DeleteKey(struct dcesrv_call_state *dce_call,
 
 	DCESRV_PULL_HANDLE_FAULT(h, r->in.handle, HTYPE_REGKEY);
 
-	return reg_key_del((struct registry_key *)h->data, r->in.key.name);
+	switch (security_session_user_level(dce_call->conn->auth_state.session_info))
+	{
+	case SECURITY_SYSTEM:
+	case SECURITY_ADMINISTRATOR:
+		return reg_key_del((struct registry_key *)h->data, r->in.key.name);
+	default:
+		return WERR_ACCESS_DENIED;
+	}
 }
 
 
@@ -176,9 +191,16 @@ static WERROR dcesrv_winreg_DeleteValue(struct dcesrv_call_state *dce_call,
 
 	DCESRV_PULL_HANDLE_FAULT(h, r->in.handle, HTYPE_REGKEY);
 
-	key = h->data;
-
-	return reg_del_value(key, r->in.value.name);
+	switch (security_session_user_level(dce_call->conn->auth_state.session_info))
+	{
+	case SECURITY_SYSTEM:
+	case SECURITY_ADMINISTRATOR:
+		key = h->data;
+		
+		return reg_del_value(key, r->in.value.name);
+	default:
+		return WERR_ACCESS_DENIED;
+	}
 }
 
 
@@ -289,7 +311,14 @@ static WERROR dcesrv_winreg_FlushKey(struct dcesrv_call_state *dce_call,
 
 	DCESRV_PULL_HANDLE_FAULT(h, r->in.handle, HTYPE_REGKEY);
 
-	return reg_key_flush(h->data);
+	switch (security_session_user_level(dce_call->conn->auth_state.session_info))
+	{
+	case SECURITY_SYSTEM:
+	case SECURITY_ADMINISTRATOR:
+		return reg_key_flush(h->data);
+	default:
+		return WERR_ACCESS_DENIED;
+	}
 }
 
 
@@ -342,23 +371,31 @@ static WERROR dcesrv_winreg_OpenKey(struct dcesrv_call_state *dce_call,
 
 	DCESRV_PULL_HANDLE_FAULT(h, r->in.parent_handle, HTYPE_REGKEY);
 
-	if (r->in.keyname.name && strcmp(r->in.keyname.name, "") == 0) {
-		newh = talloc_reference(dce_call->context, h);
-		result = WERR_OK;
-	} else {
-		newh = dcesrv_handle_new(dce_call->context, HTYPE_REGKEY);
-		result = reg_open_key(newh, (struct registry_key *)h->data,
-				      r->in.keyname.name,
-				      (struct registry_key **)&newh->data);
-	}
-
-	if (W_ERROR_IS_OK(result)) {
-		r->out.handle = &newh->wire_handle;
-	} else {
-		talloc_free(newh);
+	switch (security_session_user_level(dce_call->conn->auth_state.session_info))
+	{
+	case SECURITY_SYSTEM:
+	case SECURITY_ADMINISTRATOR:
+	case SECURITY_USER:
+		if (r->in.keyname.name && strcmp(r->in.keyname.name, "") == 0) {
+			newh = talloc_reference(dce_call->context, h);
+			result = WERR_OK;
+		} else {
+			newh = dcesrv_handle_new(dce_call->context, HTYPE_REGKEY);
+			result = reg_open_key(newh, (struct registry_key *)h->data,
+					      r->in.keyname.name,
+					      (struct registry_key **)&newh->data);
+		}
+		
+		if (W_ERROR_IS_OK(result)) {
+			r->out.handle = &newh->wire_handle;
+		} else {
+			talloc_free(newh);
+		}
+		return result;
+	default:
+		return WERR_ACCESS_DENIED;
 	}
 
-	return result;
 }
 
 
@@ -376,17 +413,25 @@ static WERROR dcesrv_winreg_QueryInfoKey(struct dcesrv_call_state *dce_call,
 
 	DCESRV_PULL_HANDLE_FAULT(h, r->in.handle, HTYPE_REGKEY);
 
-	k = h->data;
-
-	ret = reg_key_get_info(mem_ctx, k, &classname, r->out.num_subkeys,
-			       r->out.num_values, r->out.last_changed_time,
-			       r->out.max_subkeylen, r->out.max_valnamelen, 
-			       r->out.max_valbufsize);
-
-	if (r->out.classname != NULL)
-		r->out.classname->name = classname;
-
-	return ret;
+	switch (security_session_user_level(dce_call->conn->auth_state.session_info))
+	{
+	case SECURITY_SYSTEM:
+	case SECURITY_ADMINISTRATOR:
+	case SECURITY_USER:
+		k = h->data;
+		
+		ret = reg_key_get_info(mem_ctx, k, &classname, r->out.num_subkeys,
+				       r->out.num_values, r->out.last_changed_time,
+				       r->out.max_subkeylen, r->out.max_valnamelen, 
+				       r->out.max_valbufsize);
+		
+		if (r->out.classname != NULL)
+			r->out.classname->name = classname;
+		
+		return ret;
+	default:
+		return WERR_ACCESS_DENIED;
+	}
 }
 
 
@@ -405,35 +450,43 @@ static WERROR dcesrv_winreg_QueryValue(struct dcesrv_call_state *dce_call,
 
 	DCESRV_PULL_HANDLE_FAULT(h, r->in.handle, HTYPE_REGKEY);
 
-	key = h->data;
-
-	result = reg_key_get_value_by_name(mem_ctx, key, r->in.value_name.name,
-					   &value_type, &value_data);
-
-	if (!W_ERROR_IS_OK(result)) {
-		return result;
-	}
-
-	/* Just asking for the size of the buffer */
-	r->out.type = talloc(mem_ctx, uint32_t);
-	if (!r->out.type) {
-		return WERR_NOMEM;
-	}
-	*r->out.type = value_type;
-	r->out.length = talloc(mem_ctx, uint32_t);
-	if (!r->out.length) {
-		return WERR_NOMEM;
-	}
-	*r->out.length = value_data.length;
-	if (r->in.data == NULL) {
-		r->out.size = talloc(mem_ctx, uint32_t);
-		*r->out.size = value_data.length;
-	} else {
-		r->out.size = r->in.size;
-		r->out.data = value_data.data;
+	switch (security_session_user_level(dce_call->conn->auth_state.session_info))
+	{
+	case SECURITY_SYSTEM:
+	case SECURITY_ADMINISTRATOR:
+	case SECURITY_USER:
+		key = h->data;
+		
+		result = reg_key_get_value_by_name(mem_ctx, key, r->in.value_name.name,
+						   &value_type, &value_data);
+		
+		if (!W_ERROR_IS_OK(result)) {
+			return result;
+		}
+		
+		/* Just asking for the size of the buffer */
+		r->out.type = talloc(mem_ctx, uint32_t);
+		if (!r->out.type) {
+			return WERR_NOMEM;
+		}
+		*r->out.type = value_type;
+		r->out.length = talloc(mem_ctx, uint32_t);
+		if (!r->out.length) {
+			return WERR_NOMEM;
+		}
+		*r->out.length = value_data.length;
+		if (r->in.data == NULL) {
+			r->out.size = talloc(mem_ctx, uint32_t);
+			*r->out.size = value_data.length;
+		} else {
+			r->out.size = r->in.size;
+			r->out.data = value_data.data;
+		}
+		
+		return WERR_OK;
+	default:
+		return WERR_ACCESS_DENIED;
 	}
-
-	return WERR_OK;
 }
 
 
@@ -497,11 +550,17 @@ static WERROR dcesrv_winreg_SetValue(struct dcesrv_call_state *dce_call,
 
 	key = h->data;
 
-	data.data = r->in.data;
-	data.length = r->in.size;
-	result = reg_val_set(key, r->in.name.name, r->in.type, data);
-
-	return result;
+	switch (security_session_user_level(dce_call->conn->auth_state.session_info))
+	{
+	case SECURITY_SYSTEM:
+	case SECURITY_ADMINISTRATOR:
+		data.data = r->in.data;
+		data.length = r->in.size;
+		result = reg_val_set(key, r->in.name.name, r->in.type, data);
+		return result;
+	default:
+		return WERR_ACCESS_DENIED;
+	}
 }