[SAMBA4] Add hooks for NTACLs in LDB

tridge at samba.org tridge at samba.org
Fri Oct 7 12:21:06 GMT 2005


Andrew,

 > The attached patch adds the hooks required to communicate the current
 > user from the authenticated session down into LDB.  This associates a
 > session info structure with the open LDB, allowing a future ldb_ntacl
 > module to allow/deny operations on that basis.

thanks! Looks very good. Minor comments below.

 > +	server_info->account_name = talloc_strdup(server_info, "sYSTEM");
 > +	NT_STATUS_HAVE_NO_MEMORY(server_info->account_name);

the case looks a bit strange there ..

 > +	ldb = partition->private;

it's a good idea to use talloc_get_type() when fetching void* values,
to make coding errors less likely. You could change the above to:

   ldb = talloc_get_type(partition->private, struct ldb_context);

 > +	/* Connections start out anonymous */
 > +	if (!NT_STATUS_IS_OK(auth_anonymous_session_info(conn, &conn->session_info))) {
 > +		ldapsrv_terminate_connection(conn, "failed to setup anonymous session info");
 > +		return;
 > +	}

I suspect we'll need a separate flag somewhere saying if we are
currently authenticated or not, as w2k3 seems to not just start
anonymous, but seems to flag if auth has happened, and returns this:

  LDAP error 1 -  <00000000: LdapErr: DSID-0C090627, comment: In order
  to perform this operation a successful bind must be completed on the
  connection.

if we want to do that correctly then a boolean 'have_authenticated'
on the connection might be useful. Or did you plan to use a
is_anonymous_token() test?

 > +			for (part = call->conn->partitions; part; part = part->next) {
 > +				if (!part->ops->Bind) {
 > +					continue;
 > +				}
 > +				status = part->ops->Bind(part, conn);
 > +				if (!NT_STATUS_IS_OK(status)) {
 > +					result = LDAP_OPERATIONS_ERROR;
 > +					errstr = talloc_asprintf(reply, "SASL:[%s]: Failed to advise partition %s of new credentials: %s", req->creds.SASL.mechanism, part->base_dn, nt_errstr(status));
 > +				}
 > +			}


this part of the code is getting a bit deeply nested, and harder to
read. Could I suggest a helper function, perhaps
ldap_update_partitions() or similar?

 > +BOOL is_system_token(struct security_token *token) 
 > +{
 > +	TALLOC_CTX *mem_ctx = talloc_new(token);
 > +	if (dom_sid_equal(token->user_sid, dom_sid_parse_talloc(mem_ctx, SID_NT_SYSTEM))) {
 > +		talloc_free(mem_ctx);
 > +		return True;
 > +	}
 > +	talloc_free(mem_ctx);
 > +	return False;
 > +}


its a very minor thing, but I find that this pattern is clearer:

BOOL is_system_token(struct security_token *token) 
{
	TALLOC_CTX *tmp_ctx = talloc_new(token);
	BOOL ret = dom_sid_equal(token->user_sid, 
	                         dom_sid_parse_talloc(tmp_ctx, SID_NT_SYSTEM));
	talloc_free(tmp_ctx);
	return ret;
}

Cheers, Tridge


More information about the samba-technical mailing list