From 6f0e193c0895a62c93329df4a15bd9e5e84e080c Mon Sep 17 00:00:00 2001
From: Simon Rettberg
Date: Mon, 15 Mar 2021 15:27:37 +0100
Subject: Fix and improve numeric uid handling

---
 proxy.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

(limited to 'proxy.c')

diff --git a/proxy.c b/proxy.c
index 5652e91..f3588ef 100644
--- a/proxy.c
+++ b/proxy.c
@@ -71,13 +71,33 @@ static struct string s_bogusfieldname42, s_bogusfieldname43;
 static struct string str_ADUSER, str_ADUSERDN;
 
 // HACK - we pefix usernames that start with something other than a-z_ with "_x_"
+// This is for requests - remove the prefix that the client added
 static void fixUnNumeric(struct string *value)
 {
-	if (value == NULL || value->l < 4) return;
+	if (value == NULL) return;
+	if (value->s[0] >= '0' && value->s[0] <= '9') {
+		// This is a query from the client for an actually numeric ID - this should not
+		// happen, so let's break the request to make sure it doesn't yield anything
+		char *n = tmpbuffer_get();
+		size_t len = 0;
+		n[len++] = 'f';
+		n[len++] = 'a';
+		n[len++] = 'k';
+		n[len++] = 'e';
+		for (size_t i = 0; i < value->l && i < 30; ++i) {
+			n[len++] = value->s[i] + (value->s[i] < 64 ? 10 : -10);
+		}
+		value->s = n;
+		value->l = len;
+		return;
+	}
+	// Look for actual expected prefix and remove
+	if (value->l < 4) return;
 	if (value->s[0] != '_' || value->s[1] != 'x' || value->s[2] != '_') return;
 	value->s += 3;
 	value->l -= 3;
 }
+// For server responses, if they're numeric, add the prefix
 static void fixNumeric(struct string *value)
 {
 	size_t i;
@@ -542,7 +562,7 @@ static BOOL request_replaceAttribute(server_t *server, struct string *attribute,
 	if (iequals(attribute, &s_uid)) {
 		*attribute = server->map.uid;
 		if (attr) attr->hasUser = TRUE;
-		// If uid is of format s[0-9]+, we assume that it's a numeric account name in AD, as a workaround
+		// If uid is of format _x_*, we assume that it's a numeric account name in AD, as a workaround
 		if (value == NULL) return TRUE;
 		if (server->fixNumeric) {
 			fixUnNumeric(value);
@@ -668,7 +688,6 @@ static struct PartialAttributeList* response_addPal(struct PartialAttributeList
 static void response_replacePal(server_t *server, struct PartialAttributeList **pal, attr_t *attr)
 {
 	struct string *username = NULL;
-	BOOL wasNumeric = FALSE;
 	while (*pal != NULL) {
 		BOOL del = FALSE;
 		if (0) { } // Remove fields we don't want from AD/LDAP
@@ -715,7 +734,6 @@ static void response_replacePal(server_t *server, struct PartialAttributeList **
 		// Fetch user name so we can add our fake fields later
 		if (username == NULL && iequals(&(*pal)->type, &s_uid)) {
 			username = &(*pal)->values->a;
-			if (server->fixNumeric && username->l > 1 && username->s[0] == 's' && isInt(username, 1)) wasNumeric = TRUE;
 		}
 		pal = &(*pal)->next;
 	}
@@ -740,11 +758,13 @@ static void response_replacePal(server_t *server, struct PartialAttributeList **
 		if (attr->bogusFieldName43) {
 			ADDATTR(bogusFieldName43, "%s", user);
 		}
-		if (wasNumeric) user++; // From here on, user is the real AD/ldap username, no leading 's'
+		if (server->fixNumeric && strncmp(user, "_x_", 3) == 0) {
+			user += 3;
+		}
 		if (attr->homeMount && server->homeTemplate[0] != '\0') {
 			ADDATTR(homeMount, server->homeTemplate, user, user, user, user, user, user);
 		}
-		// Do this here so user++ will have been executed
+		// Do this here so user += 3 will have been executed
 		ADDATTR(realAccount, "%s", user);
 	}
 	ADDATTR(loginShell, "/bin/bash");
@@ -1032,7 +1052,7 @@ static BOOL proxy_clientBindRequest(epoll_client_t *client, const unsigned long
 			server_t *server = server_getFromBase(&name);
 			if (server == NULL
 					|| (incorrect = (strncmp(password.s, "\x08\x0a\x0d\x7fINCORRECT", 13) == 0))
-					|| (server->fixNumeric && isInt(&name, 0))) {
+					|| (server->fixNumeric && isdigit(name.s[0]))) {
 				// The INCORRECT part is some weird thing I saw pam_ldap do - probably to identify misconfigured
 				// LDAP servers/accounts that will accept any password - save the round trip to AD and deny
 				if (!incorrect) plog(DEBUG_WARNING, "[Client] Numeric account or invalid binddn for %.*s", (int)name.l, name.s);
-- 
cgit v1.2.3-55-g7522