lib/ldb: Avoid printing secret attributes in ldb trace logs

These are printed when Samba has debug level 10, which is often used for debugging.

Instead, print a note to say that this attribute has been skipped.

Andrew Bartlett

3 files changed, 80 insertions, 13 deletions

diff --git a/lib/ldb/common/ldb.c b/lib/ldb/common/ldb.c
index 887a8967b1..49eccb919d 100644
--- a/lib/ldb/common/ldb.c
+++ b/lib/ldb/common/ldb.c
@@ -726,6 +726,7 @@ static void ldb_trace_request(struct ldb_context *ldb, struct ldb_request *req)
 {
 	TALLOC_CTX *tmp_ctx = talloc_new(req);
 	unsigned int i;
+	struct ldb_ldif ldif;
 
 	switch (req->operation) {
 	case LDB_SEARCH:
@@ -765,18 +766,36 @@ static void ldb_trace_request(struct ldb_context *ldb, struct ldb_request *req)
 		ldb_debug_add(ldb, " data: %s\n", req->op.extended.data?"yes":"no");
 		break;
 	case LDB_ADD:
+		ldif.changetype = LDB_CHANGETYPE_ADD;
+		ldif.msg = discard_const_p(struct ldb_message, req->op.add.message);
+
 		ldb_debug_add(ldb, "ldb_trace_request: ADD\n");
+
+		/* 
+		 * The choice to call
+		 * ldb_ldif_write_redacted_trace_string() is CRITICAL
+		 * for security.  It ensures that we do not output
+		 * passwords into debug logs 
+		 */
+
 		ldb_debug_add(req->handle->ldb, "%s\n", 
-			      ldb_ldif_message_string(req->handle->ldb, tmp_ctx, 
-						      LDB_CHANGETYPE_ADD, 
-						      req->op.add.message));
+			      ldb_ldif_write_redacted_trace_string(req->handle->ldb, tmp_ctx, &ldif));
 		break;
 	case LDB_MODIFY:
+		ldif.changetype = LDB_CHANGETYPE_MODIFY;
+		ldif.msg = discard_const_p(struct ldb_message, req->op.mod.message);
+
 		ldb_debug_add(ldb, "ldb_trace_request: MODIFY\n");
+
+		/* 
+		 * The choice to call
+		 * ldb_ldif_write_redacted_trace_string() is CRITICAL
+		 * for security.  It ensures that we do not output
+		 * passwords into debug logs 
+		 */
+
 		ldb_debug_add(req->handle->ldb, "%s\n", 
-			      ldb_ldif_message_string(req->handle->ldb, tmp_ctx, 
-						      LDB_CHANGETYPE_MODIFY,
-						      req->op.mod.message));
+			      ldb_ldif_write_redacted_trace_string(req->handle->ldb, tmp_ctx, &ldif));
 		break;
 	case LDB_REQ_REGISTER_CONTROL:
 		ldb_debug_add(ldb, "ldb_trace_request: REGISTER_CONTROL\n");
diff --git a/lib/ldb/common/ldb_ldif.c b/lib/ldb/common/ldb_ldif.c
index c083401c6e..419906ba82 100644
--- a/lib/ldb/common/ldb_ldif.c
+++ b/lib/ldb/common/ldb_ldif.c
@@ -270,18 +270,20 @@ static const struct {
 #define CHECK_RET do { if (ret < 0) { talloc_free(mem_ctx); return ret; } total += ret; } while (0)
 
 /*
-  write to ldif, using a caller supplied write method
+  write to ldif, using a caller supplied write method, and only printing secrets if we are not in a trace
 */
-int ldb_ldif_write(struct ldb_context *ldb,
-		   int (*fprintf_fn)(void *, const char *, ...), 
-		   void *private_data,
-		   const struct ldb_ldif *ldif)
+static int ldb_ldif_write_trace(struct ldb_context *ldb,
+				int (*fprintf_fn)(void *, const char *, ...), 
+				void *private_data,
+				const struct ldb_ldif *ldif, 
+				bool in_trace)
 {
 	TALLOC_CTX *mem_ctx;
 	unsigned int i, j;
 	int total=0, ret;
 	char *p;
 	const struct ldb_message *msg;
+	const char * const * secret_attributes = ldb_get_opaque(ldb, LDB_SECRET_ATTRIBUTE_LIST_OPAQUE);
 
 	mem_ctx = talloc_named_const(NULL, 0, "ldb_ldif_write");
 
@@ -328,6 +330,14 @@ int ldb_ldif_write(struct ldb_context *ldb,
 				break;
 			}
 		}
+		
+		if (in_trace && secret_attributes && ldb_attr_in_list(secret_attributes, msg->elements[i].name)) {
+			/* Deliberatly skip printing this password */
+			ret = fprintf_fn(private_data, "# %s::: REDACTED SECRET ATTRIBUTE",
+					 msg->elements[i].name);
+			CHECK_RET;
+			continue;
+		}
 
 		for (j=0;j<msg->elements[i].num_values;j++) {
 			struct ldb_val v;
@@ -383,6 +393,18 @@ int ldb_ldif_write(struct ldb_context *ldb,
 
 
 /*
+  write to ldif, using a caller supplied write method
+*/
+int ldb_ldif_write(struct ldb_context *ldb,
+		   int (*fprintf_fn)(void *, const char *, ...), 
+		   void *private_data,
+		   const struct ldb_ldif *ldif)
+{
+	return ldb_ldif_write_trace(ldb, fprintf_fn, private_data, ldif, false);
+}
+
+
+/*
   pull a ldif chunk, which is defined as a piece of data ending in \n\n or EOF
   this routine removes any RFC2849 continuations and comments
 
@@ -727,7 +749,6 @@ struct ldb_ldif *ldb_ldif_read(struct ldb_context *ldb,
 	char *chunk=NULL, *s;
 	struct ldb_val value;
 	unsigned flags = 0;
-
 	value.data = NULL;
 
 	ldif = talloc(ldb, struct ldb_ldif);
@@ -1002,6 +1023,20 @@ static int ldif_printf_string(void *private_data, const char *fmt, ...)
 	return talloc_get_size(state->string) - oldlen;
 }
 
+char *ldb_ldif_write_redacted_trace_string(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, 
+					   const struct ldb_ldif *ldif)
+{
+	struct ldif_write_string_state state;
+	state.string = talloc_strdup(mem_ctx, "");
+	if (!state.string) {
+		return NULL;
+	}
+	if (ldb_ldif_write_trace(ldb, ldif_printf_string, &state, ldif, true) == -1) {
+		return NULL;
+	}
+	return state.string;
+}
+
 char *ldb_ldif_write_string(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, 
 			    const struct ldb_ldif *ldif)
 {
diff --git a/lib/ldb/common/ldb_modules.c b/lib/ldb/common/ldb_modules.c
index 8904d5a94a..440365688b 100644
--- a/lib/ldb/common/ldb_modules.c
+++ b/lib/ldb/common/ldb_modules.c
@@ -709,8 +709,21 @@ int ldb_module_send_entry(struct ldb_request *req,
 	if ((req->handle->ldb->flags & LDB_FLG_ENABLE_TRACING) &&
 	    req->handle->nesting == 0) {
 		char *s;
+		struct ldb_ldif ldif;
+		
+		ldif.changetype = LDB_CHANGETYPE_NONE;
+		ldif.msg = discard_const_p(struct ldb_message, msg);
+
 		ldb_debug_add(req->handle->ldb, "ldb_trace_response: ENTRY\n");
-		s = ldb_ldif_message_string(req->handle->ldb, msg, LDB_CHANGETYPE_NONE, msg);
+
+		/* 
+		 * The choice to call
+		 * ldb_ldif_write_redacted_trace_string() is CRITICAL
+		 * for security.  It ensures that we do not output
+		 * passwords into debug logs 
+		 */
+
+		s = ldb_ldif_write_redacted_trace_string(req->handle->ldb, msg, &ldif);
 		ldb_debug_add(req->handle->ldb, "%s\n", s);
 		talloc_free(s);
 		ldb_debug_end(req->handle->ldb, LDB_DEBUG_TRACE);