[SCM] Samba Shared Repository - branch master updated
Stefan Metzmacher
metze at samba.org
Wed Dec 14 19:13:05 UTC 2016
The branch, master has been updated
via a7598fb rpc_server:netlogon Move from memcache to a tdb cache
from cf536e3 torture: Add ServerReqChallengeReuseGlobal2 to rpc.netlogon
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit a7598fb53b3a5238d49edbcbdfd218311ae73807
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed Nov 9 15:17:00 2016 +1300
rpc_server:netlogon Move from memcache to a tdb cache
This allows the netlogon server to be moved into a multi-process model
while still supporting clients that use a challenge from a different
network connection.
Pair-Programmed-With: Andrew Bartlett <abartlet at samba.org>
Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Autobuild-User(master): Stefan Metzmacher <metze at samba.org>
Autobuild-Date(master): Wed Dec 14 20:12:14 CET 2016 on sn-devel-144
-----------------------------------------------------------------------
Summary of changes:
libcli/auth/schannel_state.h | 12 ++
libcli/auth/schannel_state_tdb.c | 261 ++++++++++++++++++++++++++
librpc/idl/schannel.idl | 7 +
source4/rpc_server/netlogon/dcerpc_netlogon.c | 92 +++++----
4 files changed, 325 insertions(+), 47 deletions(-)
Changeset truncated at 500 lines:
diff --git a/libcli/auth/schannel_state.h b/libcli/auth/schannel_state.h
index f9d02dd..a333098 100644
--- a/libcli/auth/schannel_state.h
+++ b/libcli/auth/schannel_state.h
@@ -39,4 +39,16 @@ NTSTATUS schannel_check_creds_state(TALLOC_CTX *mem_ctx,
struct netr_Authenticator *return_authenticator,
struct netlogon_creds_CredentialState **creds_out);
+NTSTATUS schannel_get_challenge(struct loadparm_context *lp_ctx,
+ struct netr_Credential *client_challenge,
+ struct netr_Credential *server_challenge,
+ const char *computer_name);
+
+NTSTATUS schannel_save_challenge(struct loadparm_context *lp_ctx,
+ const struct netr_Credential *client_challenge,
+ const struct netr_Credential *server_challenge,
+ const char *computer_name);
+
+NTSTATUS schannel_delete_challenge(struct loadparm_context *lp_ctx,
+ const char *computer_name);
#endif
diff --git a/libcli/auth/schannel_state_tdb.c b/libcli/auth/schannel_state_tdb.c
index 2d3481d..d884279 100644
--- a/libcli/auth/schannel_state_tdb.c
+++ b/libcli/auth/schannel_state_tdb.c
@@ -272,6 +272,267 @@ NTSTATUS schannel_save_creds_state(TALLOC_CTX *mem_ctx,
return status;
}
+
+/*
+ * Create a very lossy hash of the computer name.
+ *
+ * The idea here is to compress the computer name into small space so
+ * that malicious clients cannot fill the database with junk, as only a
+ * maximum of 16k of entries are possible.
+ *
+ * Collisions are certainly possible, and the design behaves in the
+ * same way as when the hostname is reused, but clients that use the
+ * same connection do not go via the cache, and the cache only needs
+ * to function between the ReqChallenge and ServerAuthenticate
+ * packets.
+ */
+static void hash_computer_name(const char *computer_name,
+ char keystr[16])
+{
+ unsigned int hash;
+ TDB_DATA computer_tdb_data = {
+ .dptr = (uint8_t *)discard_const_p(char, computer_name),
+ .dsize = strlen(computer_name)
+ };
+ hash = tdb_jenkins_hash(&computer_tdb_data);
+
+ /* we are using 14 bits of the digest to index our connections, so
+ that we use at most 16,384 buckets.*/
+ snprintf(keystr, 15, "CHALLENGE/%x%x", hash & 0xFF,
+ (hash & 0xFF00 >> 8) & 0x3f);
+ return;
+}
+
+
+static
+NTSTATUS schannel_store_challenge_tdb(struct db_context *db_sc,
+ TALLOC_CTX *mem_ctx,
+ const struct netr_Credential *client_challenge,
+ const struct netr_Credential *server_challenge,
+ const char *computer_name)
+{
+ enum ndr_err_code ndr_err;
+ DATA_BLOB blob;
+ TDB_DATA value;
+ char *name_upper = NULL;
+ NTSTATUS status;
+ char keystr[16] = { 0, };
+ struct netlogon_cache_entry cache_entry;
+
+ if (strlen(computer_name) > 255) {
+ /*
+ * We don't make this a limit at 15 chars as Samba has
+ * a test showing this can be longer :-(
+ */
+ return STATUS_BUFFER_OVERFLOW;
+ }
+
+ name_upper = strupper_talloc(mem_ctx, computer_name);
+ if (name_upper == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ hash_computer_name(name_upper, keystr);
+
+ cache_entry.computer_name = name_upper;
+ cache_entry.client_challenge = *client_challenge;
+ cache_entry.server_challenge = *server_challenge;
+
+ ndr_err = ndr_push_struct_blob(&blob, mem_ctx, &cache_entry,
+ (ndr_push_flags_fn_t)ndr_push_netlogon_cache_entry);
+ if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+ return NT_STATUS_UNSUCCESSFUL;
+ }
+
+ value.dptr = blob.data;
+ value.dsize = blob.length;
+
+ status = dbwrap_store_bystring(db_sc, keystr, value, TDB_REPLACE);
+ if (!NT_STATUS_IS_OK(status)) {
+ DEBUG(0,("%s: failed to stored challenge info for '%s' "
+ "with key %s - %s\n",
+ __func__, cache_entry.computer_name, keystr,
+ nt_errstr(status)));
+ return status;
+ }
+
+ DEBUG(3,("%s: stored challenge info for '%s' with key %s\n",
+ __func__, cache_entry.computer_name, keystr));
+
+ if (DEBUGLEVEL >= 10) {
+ NDR_PRINT_DEBUG(netlogon_cache_entry, &cache_entry);
+ }
+
+ return NT_STATUS_OK;
+}
+
+/********************************************************************
+ Fetch a single challenge from the TDB.
+ ********************************************************************/
+
+static
+NTSTATUS schannel_fetch_challenge_tdb(struct db_context *db_sc,
+ TALLOC_CTX *mem_ctx,
+ struct netr_Credential *client_challenge,
+ struct netr_Credential *server_challenge,
+ const char *computer_name)
+{
+ NTSTATUS status;
+ TDB_DATA value;
+ enum ndr_err_code ndr_err;
+ DATA_BLOB blob;
+ char keystr[16] = { 0, };
+ struct netlogon_cache_entry cache_entry;
+ char *name_upper = NULL;
+
+ name_upper = strupper_talloc(mem_ctx, computer_name);
+ if (name_upper == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ hash_computer_name(name_upper, keystr);
+
+ status = dbwrap_fetch_bystring(db_sc, mem_ctx, keystr, &value);
+ if (!NT_STATUS_IS_OK(status)) {
+ DEBUG(3,("%s: Failed to find entry for %s with key %s - %s\n",
+ __func__, name_upper, keystr, nt_errstr(status)));
+ goto done;
+ }
+
+ blob = data_blob_const(value.dptr, value.dsize);
+
+ ndr_err = ndr_pull_struct_blob_all(&blob, mem_ctx, &cache_entry,
+ (ndr_pull_flags_fn_t)ndr_pull_netlogon_cache_entry);
+ if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+ status = ndr_map_error2ntstatus(ndr_err);
+ DEBUG(3,("%s: Failed to parse entry for %s with key %s - %s\n",
+ __func__, name_upper, keystr, nt_errstr(status)));
+ goto done;
+ }
+
+ if (DEBUGLEVEL >= 10) {
+ NDR_PRINT_DEBUG(netlogon_cache_entry, &cache_entry);
+ }
+
+ if (strcmp(cache_entry.computer_name, name_upper) != 0) {
+ status = NT_STATUS_NOT_FOUND;
+
+ DEBUG(1, ("%s: HASH COLLISION with key %s ! "
+ "Wanted to fetch record for %s but got %s.",
+ __func__, keystr, name_upper,
+ cache_entry.computer_name));
+ } else {
+
+ DEBUG(3,("%s: restored key %s for %s\n",
+ __func__, keystr, cache_entry.computer_name));
+
+ *client_challenge = cache_entry.client_challenge;
+ *server_challenge = cache_entry.server_challenge;
+ }
+ done:
+
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
+ }
+
+ return NT_STATUS_OK;
+}
+
+/******************************************************************************
+ Wrapper around schannel_fetch_session_key_tdb()
+ Note we must be root here.
+
+*******************************************************************************/
+
+NTSTATUS schannel_get_challenge(struct loadparm_context *lp_ctx,
+ struct netr_Credential *client_challenge,
+ struct netr_Credential *server_challenge,
+ const char *computer_name)
+{
+ TALLOC_CTX *frame = talloc_stackframe();
+ struct db_context *db_sc;
+ NTSTATUS status;
+
+ db_sc = open_schannel_session_store(frame, lp_ctx);
+ if (!db_sc) {
+ TALLOC_FREE(frame);
+ return NT_STATUS_ACCESS_DENIED;
+ }
+
+ status = schannel_fetch_challenge_tdb(db_sc, frame,
+ client_challenge,
+ server_challenge,
+ computer_name);
+ TALLOC_FREE(frame);
+ return status;
+}
+
+/******************************************************************************
+ Wrapper around dbwrap_delete_bystring()
+ Note we must be root here.
+
+ This allows the challenge to be removed from the TDB, which should be
+ as soon as the TDB or in-memory copy it is used, to avoid reuse.
+*******************************************************************************/
+
+NTSTATUS schannel_delete_challenge(struct loadparm_context *lp_ctx,
+ const char *computer_name)
+{
+ TALLOC_CTX *frame = talloc_stackframe();
+ struct db_context *db_sc;
+ char *name_upper;
+ char keystr[16] = { 0, };
+
+ db_sc = open_schannel_session_store(frame, lp_ctx);
+ if (!db_sc) {
+ TALLOC_FREE(frame);
+ return NT_STATUS_ACCESS_DENIED;
+ }
+
+ name_upper = strupper_talloc(frame, computer_name);
+ if (!name_upper) {
+ TALLOC_FREE(frame);
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ hash_computer_name(name_upper, keystr);
+
+ /* Now delete it, we do not want to permit fetch of this twice */
+ dbwrap_delete_bystring(db_sc, keystr);
+
+ TALLOC_FREE(frame);
+ return NT_STATUS_OK;
+}
+
+/******************************************************************************
+ Wrapper around schannel_store_session_key_tdb()
+ Note we must be root here.
+*******************************************************************************/
+
+NTSTATUS schannel_save_challenge(struct loadparm_context *lp_ctx,
+ const struct netr_Credential *client_challenge,
+ const struct netr_Credential *server_challenge,
+ const char *computer_name)
+{
+ TALLOC_CTX *frame = talloc_stackframe();
+ struct db_context *db_sc;
+ NTSTATUS status;
+
+ db_sc = open_schannel_session_store(frame, lp_ctx);
+ if (!db_sc) {
+ TALLOC_FREE(frame);
+ return NT_STATUS_ACCESS_DENIED;
+ }
+
+ status = schannel_store_challenge_tdb(db_sc, frame,
+ client_challenge,
+ server_challenge,
+ computer_name);
+
+ TALLOC_FREE(frame);
+ return status;
+}
+
/********************************************************************
Validate an incoming authenticator against the credentials for the
remote machine stored in the schannel database.
diff --git a/librpc/idl/schannel.idl b/librpc/idl/schannel.idl
index ed64f0a..fa688f6 100644
--- a/librpc/idl/schannel.idl
+++ b/librpc/idl/schannel.idl
@@ -27,6 +27,13 @@ interface schannel
dom_sid *sid;
} netlogon_creds_CredentialState;
+ /* This is used in the schannel_cache.tdb */
+ typedef [public] struct {
+ [string,charset(UTF16)] uint16 *computer_name;
+ netr_Credential server_challenge;
+ netr_Credential client_challenge;
+ } netlogon_cache_entry;
+
/* MS-NRPC 2.2.1.3.1 NL_AUTH_MESSAGE */
typedef [v1_enum] enum {
diff --git a/source4/rpc_server/netlogon/dcerpc_netlogon.c b/source4/rpc_server/netlogon/dcerpc_netlogon.c
index 498caa9..416acdc 100644
--- a/source4/rpc_server/netlogon/dcerpc_netlogon.c
+++ b/source4/rpc_server/netlogon/dcerpc_netlogon.c
@@ -27,7 +27,6 @@
#include "auth/auth_sam_reply.h"
#include "dsdb/samdb/samdb.h"
#include "../lib/util/util_ldb.h"
-#include "../lib/util/memcache.h"
#include "../libcli/auth/schannel.h"
#include "libcli/security/security.h"
#include "param/param.h"
@@ -46,14 +45,13 @@
#define DCESRV_INTERFACE_NETLOGON_BIND(call, iface) \
dcesrv_interface_netlogon_bind(call, iface)
+
static NTSTATUS dcesrv_interface_netlogon_bind(struct dcesrv_call_state *dce_call,
const struct dcesrv_interface *iface)
{
return dcesrv_interface_bind_reject_connect(dce_call, iface);
}
-static struct memcache *global_challenge_table;
-
struct netlogon_server_pipe_state {
struct netr_Credential client_challenge;
struct netr_Credential server_challenge;
@@ -64,29 +62,10 @@ static NTSTATUS dcesrv_netr_ServerReqChallenge(struct dcesrv_call_state *dce_cal
{
struct netlogon_server_pipe_state *pipe_state =
talloc_get_type(dce_call->context->private_data, struct netlogon_server_pipe_state);
- DATA_BLOB key, val;
+ NTSTATUS ntstatus;
ZERO_STRUCTP(r->out.return_credentials);
- if (global_challenge_table == NULL) {
- /*
- * We maintain a global challenge table
- * with a fixed size (8k)
- *
- * This is required for the strange clients
- * which use different connections for
- * netr_ServerReqChallenge() and netr_ServerAuthenticate3()
- *
- */
- global_challenge_table = memcache_init(talloc_autofree_context(),
- 8192);
- if (global_challenge_table == NULL) {
- return NT_STATUS_NO_MEMORY;
- }
- }
-
- /* destroyed on pipe shutdown */
-
if (pipe_state) {
talloc_free(pipe_state);
dce_call->context->private_data = NULL;
@@ -104,10 +83,13 @@ static NTSTATUS dcesrv_netr_ServerReqChallenge(struct dcesrv_call_state *dce_cal
dce_call->context->private_data = pipe_state;
- key = data_blob_string_const(r->in.computer_name);
- val = data_blob_const(pipe_state, sizeof(*pipe_state));
-
- memcache_add(global_challenge_table, SINGLETON_CACHE, key, val);
+ ntstatus = schannel_save_challenge(dce_call->conn->dce_ctx->lp_ctx,
+ &pipe_state->client_challenge,
+ &pipe_state->server_challenge,
+ r->in.computer_name);
+ if (!NT_STATUS_IS_OK(ntstatus)) {
+ return ntstatus;
+ }
return NT_STATUS_OK;
}
@@ -117,7 +99,6 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3(struct dcesrv_call_state *dce_ca
{
struct netlogon_server_pipe_state *pipe_state =
talloc_get_type(dce_call->context->private_data, struct netlogon_server_pipe_state);
- DATA_BLOB challenge_key;
bool challenge_valid = false;
struct netlogon_server_pipe_state challenge;
struct netlogon_creds_CredentialState *creds;
@@ -142,7 +123,6 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3(struct dcesrv_call_state *dce_ca
ZERO_STRUCTP(r->out.return_credentials);
*r->out.rid = 0;
- challenge_key = data_blob_string_const(r->in.computer_name);
if (pipe_state != NULL) {
dce_call->context->private_data = NULL;
@@ -156,11 +136,11 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3(struct dcesrv_call_state *dce_ca
* netr_ServerAuthenticate3() on the same dcerpc connection.
*/
challenge = *pipe_state;
- TALLOC_FREE(pipe_state);
+
challenge_valid = true;
+
} else {
- DATA_BLOB val;
- bool ok;
+ NTSTATUS ntstatus;
/*
* Fallback and try to get the challenge from
@@ -168,17 +148,25 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3(struct dcesrv_call_state *dce_ca
*
* If too many clients are using this code path,
* they may destroy their cache entries as the
- * global_challenge_table memcache has a fixed size.
+ * TDB has a fixed size limited via a lossy hash
+ *
+ * The TDB used is the schannel store, which is
+ * initialised at startup.
+ *
+ * NOTE: The challenge is deleted from the DB as soon as it is
+ * fetched, to prevent reuse.
*
- * Note: this handles global_challenge_table == NULL fine
*/
- ok = memcache_lookup(global_challenge_table, SINGLETON_CACHE,
- challenge_key, &val);
- if (ok && val.length == sizeof(challenge)) {
- memcpy(&challenge, val.data, sizeof(challenge));
- challenge_valid = true;
- } else {
+
+ ntstatus = schannel_get_challenge(dce_call->conn->dce_ctx->lp_ctx,
+ &challenge.client_challenge,
+ &challenge.server_challenge,
+ r->in.computer_name);
+
+ if (!NT_STATUS_IS_OK(ntstatus)) {
ZERO_STRUCT(challenge);
+ } else {
+ challenge_valid = true;
}
}
@@ -232,15 +220,25 @@ static NTSTATUS dcesrv_netr_ServerAuthenticate3(struct dcesrv_call_state *dce_ca
}
/*
- * At this point we can cleanup the cache entry,
- * if we fail the client needs to call netr_ServerReqChallenge
- * again.
+ * This talloc_free is important to prevent re-use of the
+ * challenge. We have to delay it this far due to NETApp
+ * servers per:
+ * https://bugzilla.samba.org/show_bug.cgi?id=11291
+ */
+ TALLOC_FREE(pipe_state);
+
+ /*
+ * At this point we must also cleanup the TDB cache
+ * entry, if we fail the client needs to call
+ * netr_ServerReqChallenge again.
*
- * Note: this handles global_challenge_table == NULL
- * and also a non existing record just fine.
+ * Note: this handles a non existing record just fine,
+ * the r->in.computer_name might not be the one used
+ * in netr_ServerReqChallenge(), but we are trying to
+ * just tidy up the normal case to prevent re-use.
*/
- memcache_delete(global_challenge_table,
- SINGLETON_CACHE, challenge_key);
+ schannel_delete_challenge(dce_call->conn->dce_ctx->lp_ctx,
+ r->in.computer_name);
/*
* According to Microsoft (see bugid #6099)
--
Samba Shared Repository
More information about the samba-cvs
mailing list