Smarter mangling of filters, comments, better handling of unknown server responses

1 files changed, 100 insertions, 57 deletions

diff --git a/proxy.c b/proxy.c
index 2f8d950..95a7fbe 100644
--- a/proxy.c
+++ b/proxy.c
@@ -156,6 +156,11 @@ void proxy_init()
 }
 #undef SETSTR
 
+/**
+ * Initialize default mapping of attributes for those which weren't
+ * set in the config file. For LDAP, this is mostly a 1:1 mapping,
+ * for AD, we pull some stunts.
+ */
 void proxy_initDefaultMap(server_t *server)
 {
 	attr_map_t *m = &server->map;
@@ -176,6 +181,16 @@ void proxy_initDefaultMap(server_t *server)
 	}
 }
 
+static void proxy_killClient(epoll_client_t *client)
+{
+	client->kill = TRUE;
+	if (client->ssl) {
+		SSL_shutdown(client->ssl);
+	} else {
+		shutdown(client->fd, SHUT_RDWR);
+	}
+}
+
 BOOL proxy_fromClient(epoll_client_t *client, const size_t maxLen)
 {
 	unsigned long messageId, op;
@@ -190,12 +205,7 @@ BOOL proxy_fromClient(epoll_client_t *client, const size_t maxLen)
 	case SearchRequest:
 		return proxy_clientSearchRequest(client, messageId, res, maxLen);
 	case UnbindRequest:
-		client->kill = TRUE;
-		if (client->ssl) {
-			SSL_shutdown(client->ssl);
-		} else {
-			shutdown(client->fd, SHUT_RDWR);
-		}
+		proxy_killClient(client);
 		return TRUE;
 	}
 	return TRUE;
@@ -222,24 +232,32 @@ void proxy_removeClient(epoll_client_t * const client)
 void proxy_removeServer(epoll_server_t * const server)
 {
 	if (server->fixedClient != NULL) {
-		server->fixedClient->kill = TRUE;
-		if (server->fixedClient->sbFill == 0) shutdown(server->fixedClient->fd, SHUT_RDWR);
+		proxy_killClient(server->fixedClient);
 		server->fixedClient->fixedServer = NULL;
 	}
 }
 
-//void hexdump(epoll_server_t *server, const size_t start, const size_t maxLen)
-//{
-//	for (size_t i = start; i < maxLen; ++i) {
-//		const uint8_t c = server->readBuffer[i];
-//		if (c >= 32 && c <= 126) {
-//			putchar(c);
-//		} else {
-//			printf("[%X]", (int)c);
-//		}
-//	}
-//	putchar('\n');
-//}
+static void hexdump(epoll_server_t *server, const size_t start, const size_t maxLen)
+{
+	for (size_t i = start; i < maxLen; ++i) {
+		const uint8_t c = server->readBuffer[i];
+		if (c >= 32 && c <= 126) {
+			putchar(c);
+		} else {
+			printf("[%X]", (int)c);
+		}
+	}
+	putchar('\n');
+}
+
+static pending_t* proxy_getPendingFromServer(unsigned long serverMessageId)
+{
+	for (int i = 0; i < _pendingCount; ++i) {
+		if (_pendingRequest[i].client == NULL) continue;
+		if (_pendingRequest[i].serverMessageId == serverMessageId) return &_pendingRequest[i];
+	}
+	return NULL;
+}
 
 BOOL proxy_fromServer(epoll_server_t *server, const size_t maxLen)
 {
@@ -261,8 +279,17 @@ BOOL proxy_fromServer(epoll_server_t *server, const size_t maxLen)
 		//scan_ldapstring(server->readBuffer + res,const char* max,struct string* s);
 		return TRUE;
 	}
-	plog(DEBUG_WARNING, "[Server] Unsupported op in reply: %lu; dropping connection.", op);
-	return FALSE;
+	pending_t *pending = proxy_getPendingFromServer(messageId);
+	plog(DEBUG_WARNING, "[Server] Unsupported op in reply: %lu; dropping pending %p.", op, pending);
+	if (_debugLevel >= DEBUG_VERBOSE) {
+		printf("[Server] Message content was: ");
+		hexdump(server, 0, maxLen);
+	}
+	if (pending != NULL && pending->client != NULL) {
+		proxy_killClient(pending->client);
+		pending->client = NULL;
+	}
+	return TRUE; // See if we can recover by skipping this message...
 }
 
 //
@@ -284,15 +311,6 @@ static pending_t* proxy_getFreePendingSlot(epoll_client_t *client)
 	return NULL;
 }
 
-static pending_t* proxy_getPendingFromServer(unsigned long serverMessageId)
-{
-	for (int i = 0; i < _pendingCount; ++i) {
-		if (_pendingRequest[i].client == NULL) continue;
-		if (_pendingRequest[i].serverMessageId == serverMessageId) return &_pendingRequest[i];
-	}
-	return NULL;
-}
-
 /*
 static void pref(int spaces, char prefix)
 {
@@ -328,9 +346,9 @@ static inline int iequals(const struct string * const a, const struct string * c
 static BOOL request_isUserFilter(struct Filter *filter);
 static BOOL request_isServerCheck(struct AttributeDescriptionList* adl);
 static BOOL request_getGroupFilter(struct Filter *filter, struct string *wantedGroupName, uint32_t *wantedGroupId, BOOL *wantsMember);
-static void request_replaceFilter(server_t *server, struct Filter **filter);
+static void request_replaceFilter(server_t *server, struct Filter **filter, const BOOL negated);
 static void request_replaceAdl(server_t *server, struct AttributeDescriptionList **adl, attr_t *attr);
-static BOOL request_replaceAttribute(server_t *server, struct string *attribute, struct string *value, attr_t *attr);
+static BOOL request_replaceAttribute(server_t *server, struct string *attribute, struct string *value, attr_t *attr, const BOOL negated);
 
 /**
  * Checks whether the filter contains attributes that indicate this is a search for a user.
@@ -422,33 +440,49 @@ static BOOL request_getGroupFilter(struct Filter *filter, struct string *wantedG
 /**
  * Gets passed original filter from client.
  */
-static void request_replaceFilter(server_t *server, struct Filter **filter)
+static void request_replaceFilter(server_t *server, struct Filter **filter, BOOL negated)
 {
 	while (*filter != NULL) {
 		BOOL del = FALSE;
 		switch ((*filter)->type) {
 		case NOT:
+			request_replaceFilter(server, &(*filter)->x, !negated);
+			if ((*filter)->x == NULL) {
+				del = TRUE;
+			}
+			break;
 		case AND:
 		case OR:
-			request_replaceFilter(server, &(*filter)->x);
+#ifdef FILTER_COLLAPSING
+			// This collapses nested filters of same type, which was suspected to cause issues
+			// with certain LDAP servers (which was not the case in the end)
+			{
+				struct Filter *f = (*filter)->x, **p = &(*filter)->x;
+				while (f) {
+					if (f->type == (*filter)->type) {
+						struct Filter **l = &f->x;
+						while (*l != NULL) l = &(*l)->next;
+						*l = f->next;
+						*p = f->x;
+						f->x = NULL;
+						f->next = NULL;
+						free_ldapsearchfilter(f);
+						f = *p;
+					} else {
+						p = &f->next;
+						f = f->next;
+					}
+				}
+			}
+#endif
+			request_replaceFilter(server, &(*filter)->x, negated);
 			if ((*filter)->x == NULL) {
 				del = TRUE;
 			}
 			break;
 		case PRESENT:
-			if (!server->plainLdap && iequals(&(*filter)->ava.desc, &s_uid)) {
-				// HACK HACK for some AD server versions. The following filter will act as if no filter
-				// was given at all, returning the entire AD catalogue, which is not what we want:
-				// &(&(!([objectsid == xx]),[objectsid \exist]),[samaccountname \exist],[objectclass == user],[samaccountname == yy])
-				// however, the following works fine:
-				// &(&(!([objectsid == xx]),[objectsid \exist]),[objectclass == user],[samaccountname == yy])
-				// so we get rid of that one exist/present operator
-				del = TRUE;
-				break;
-			}
-			// no break;
 		case SUBSTRING:
-			if (!request_replaceAttribute(server, &(*filter)->ava.desc, NULL, NULL)) {
+			if (!request_replaceAttribute(server, &(*filter)->ava.desc, NULL, NULL, negated)) {
 				del = TRUE;
 			}
 			break;
@@ -456,7 +490,7 @@ static void request_replaceFilter(server_t *server, struct Filter **filter)
 		case GREATEQUAL:
 		case LESSEQUAL:
 		case APPROX:
-			if (!request_replaceAttribute(server, &(*filter)->ava.desc, &(*filter)->ava.value, NULL)) {
+			if (!request_replaceAttribute(server, &(*filter)->ava.desc, &(*filter)->ava.value, NULL, negated)) {
 				del = TRUE;
 			}
 			break;
@@ -478,7 +512,7 @@ static void request_replaceAdl(server_t *server, struct AttributeDescriptionList
 {
 	while (*adl != NULL) {
 		struct AttributeDescriptionList *next = NULL;
-		if (attr == NULL) request_replaceAttribute(server, &(*adl)->a, NULL, NULL);
+		if (attr == NULL) request_replaceAttribute(server, &(*adl)->a, NULL, NULL, FALSE);
 		else if (iequals(&(*adl)->a, &s_homedirectory)) {
 			attr->homeDirectory = TRUE;
 			if (server->plainLdap) {
@@ -493,7 +527,7 @@ static void request_replaceAdl(server_t *server, struct AttributeDescriptionList
 		elifSETATTR(gecos, gecos);
 		elifSETATTR(realaccount, realAccount);
 		elifSETATTR(loginshell, loginShell);
-		else request_replaceAttribute(server, &(*adl)->a, NULL, attr);
+		else request_replaceAttribute(server, &(*adl)->a, NULL, attr, FALSE);
 		if (*adl == NULL) break;
 		if (next == NULL) adl = &(*adl)->next; // If next is not NULL, we removed an entry, so we don't need to shift
 	}
@@ -510,7 +544,7 @@ static void request_replaceAdl(server_t *server, struct AttributeDescriptionList
 /**
  * @return FALSE = delete attribute, TRUE = keep
  */
-static BOOL request_replaceAttribute(server_t *server, struct string *attribute, struct string *value, attr_t *attr)
+static BOOL request_replaceAttribute(server_t *server, struct string *attribute, struct string *value, attr_t *attr, const BOOL negated)
 {
 	if (iequals(attribute, &s_uid)) {
 		*attribute = server->map.uid;
@@ -529,8 +563,8 @@ static BOOL request_replaceAttribute(server_t *server, struct string *attribute,
 	} else if (iequals(attribute, &s_uidnumber)) {
 		*attribute = server->map.uidnumber;
 		if (value == NULL) return TRUE;
-		if (value != NULL && value->l == 1 && value->s[0] == '0') {
-			// Query for user with uidNumber == 0 - root; replace with something that
+		if (value != NULL && !negated && value->l == 1 && value->s[0] == '0') {
+			// Saftey measure: Query for user with uidNumber == 0 - root; replace with something that
 			// should never return anything
 			*value = s_uid;
 		}
@@ -704,13 +738,14 @@ static void response_replaceAdl(server_t *server, struct string *attribute, stru
 
 static void response_replaceAttribute(server_t *server, const struct string * const attribute, struct string * const value)
 {
+	if (value == NULL) return;
 	// Attributes already remapped here!
 	if (iequals(attribute, &s_uid)) {
-		if (value != NULL) fixNumeric(value);
+		fixNumeric(value);
 	} else if (iequals(attribute, &s_uidnumber)) {
 		if (!server->plainLdap) {
+			plog(DEBUG_TRACE, "Replacing uidnumber from objectsid len=%d", (int)value->l);
 			// If this is AD, this must have been objectSid before - convert SID to number
-			if (value == NULL) return;
 			if (value->l != SIDLEN) return;
 			// It we don't have the servers SID base yet and we see a valid one, store it
 			if (server->sid[0] == 0 && value->s[0] == 1 && value->s[1] == 5 && value->s[7] == 5) {
@@ -844,7 +879,7 @@ static BOOL proxy_clientSearchRequest(epoll_client_t *client, const unsigned lon
 		}
 		memset(&pending->attr, -1, sizeof(pending->attr));
 	}
-	request_replaceFilter(server, &req.filter);
+	request_replaceFilter(server, &req.filter, FALSE);
 	if (_debugLevel >= DEBUG_TRACE) {
 		printf("[Client] Search request (translated): ");
 		helper_printfilter(req.filter);
@@ -884,6 +919,7 @@ static BOOL proxy_serverSearchResult(epoll_server_t *server, const unsigned long
 		bodyLen = maxLen - offset;
 		body = server->readBuffer + offset;
 		plog(DEBUG_TRACE, "[Server] SRDONE");
+		// pending->client will be released at end of function
 	} else {
 		// Transform reply
 		struct SearchResultEntry sre;
@@ -910,6 +946,13 @@ static BOOL proxy_serverSearchResult(epoll_server_t *server, const unsigned long
 		}
 		fmt_ldapsearchresultentry(bodyBuffer, &sre);
 		free_ldapsearchresultentry(&sre);
+		if (_debugLevel >= DEBUG_TRACE) {
+			const size_t res = scan_ldapsearchresultentry(bodyBuffer, bodyBuffer + bodyLen, &sre);
+			if (res != 0) {
+				helper_printpal(sre.attributes);
+				free_ldapsearchresultentry(&sre);
+			}
+		}
 		body = bodyBuffer;
 	}
 	// Build header and fire away
@@ -918,7 +961,7 @@ static BOOL proxy_serverSearchResult(epoll_server_t *server, const unsigned long
 	fmt_ldapmessage(buffer, pending->clientMessageId, type, bodyLen);
 	client_send(pending->client, buffer, headerLen, TRUE);
 	client_send(pending->client, body, bodyLen, FALSE);
-	if (type == SearchResultDone) pending->client = NULL;
+	if (type == SearchResultDone) pending->client = NULL; // Release pending
 	return TRUE;
 }