From 1d25f2fda99391b7b1c0a813106b2df6bd7ace73 Mon Sep 17 00:00:00 2001 From: Dimitrios Apostolou Date: Mon, 17 Feb 2014 10:13:24 +0100 Subject: [PATCH] Regex check every admit entry, slow but needed for backwards compatibility Various small refactorings and cleanups were needed. --- cf-serverd/server_access.c | 130 +++++++++++++++++++++----------- cf-serverd/server_access.h | 19 ++--- cf-serverd/server_classic.c | 13 ---- cf-serverd/server_transform.c | 138 ++++++++++++++++------------------ 4 files changed, 162 insertions(+), 138 deletions(-) diff --git a/cf-serverd/server_access.c b/cf-serverd/server_access.c index 608255bdae..c95c7633ff 100644 --- a/cf-serverd/server_access.c +++ b/cf-serverd/server_access.c @@ -5,6 +5,7 @@ #include /* FILE_SEPARATOR */ #include /* FuzzySetMatch */ #include /* MapAddress */ +#include /* StringMatchFull TODO REMOVE */ #include struct acl *paths_acl; @@ -18,15 +19,16 @@ struct acl *query_acl; * 2. #hostname matches the subdomain expression in {admit,deny}_hostnames * 3. #key is found exactly as it is in {admit,deny}_keys * - * @return Default is deny. If a match is found in #acl->admit_* then return - * true, unless a match is also found in #acl->deny_* in which case - * return false. + * @return Default is false, i.e. deny. If a match is found in #acl->admit.* + * then return true, unless a match is also found in #acl->deny.* in + * which case return false. * * @TODO preprocess our global ACL the moment a client connects, and store in * ServerConnectionState a list of objects that he can access. That way * only his relevant resources will be stored in e.g. {admit,deny}_paths * lists, and running through these two lists on every file request will - * be much faster. */ + * be much faster. + */ bool access_CheckResource(const struct resource_acl *acl, const char *ipaddr, const char *hostname, const char *key) @@ -41,24 +43,27 @@ bool access_CheckResource(const struct resource_acl *acl, /* First we check for admission, secondly for denial, so that denial takes * precedence. */ - char *rule; - if (acl->admit_ips) + const char *rule; + if (acl->admit.ips) { - /* TODO still using legacy code here, doing linear search over all IPs - * in textual representation... too CPU intensive! TODO store the ACL - * as one list of struct sockaddr_storage, together with CIDR + /* Still using legacy code here, doing linear search over all IPs in + * textual representation... too CPU intensive! TODO store the ACL as + * one list of struct sockaddr_storage, together with CIDR notation * subnet length. */ bool found_rule = false; - for (int i = 0; i < acl->admit_ips->len; i++) + for (int i = 0; i < StrList_Len(acl->admit.ips); i++) { - if (FuzzySetMatch(acl->admit_ips->list[i]->str, + if (FuzzySetMatch(StrList_At(acl->admit.ips, i), MapAddress(ipaddr)) - == 0) + == 0 || + /* Legacy regex matching, TODO DEPRECATE */ + StringMatchFull(StrList_At(acl->admit.ips, i), + MapAddress(ipaddr))) { found_rule = true; - rule = acl->admit_ips->list[i]->str; + rule = StrList_At(acl->admit.ips, i); break; } } @@ -69,29 +74,46 @@ bool access_CheckResource(const struct resource_acl *acl, access = true; } } - if (!access && acl->admit_keys != NULL) + if (!access && acl->admit.keys != NULL) { - bool ret = StrList_BinarySearch(acl->admit_keys, key, &pos); + bool ret = StrList_BinarySearch(acl->admit.keys, key, &pos); if (ret) { - rule = acl->admit_keys->list[pos]->str; + rule = acl->admit.keys->list[pos]->str; Log(LOG_LEVEL_DEBUG, "access_Check: admit key: %s", rule); access = true; } } - if (!access && acl->admit_hostnames != NULL && hostname != NULL) + if (!access && acl->admit.hostnames != NULL && hostname != NULL) { size_t hostname_len = strlen(hostname); - size_t pos = StrList_SearchShortestPrefix(acl->admit_hostnames, + size_t pos = StrList_SearchShortestPrefix(acl->admit.hostnames, hostname, hostname_len, false); + + /* === Legacy regex matching, slow, TODO DEPRECATE === */ + bool regex_match = false; + if (pos == (size_t) -1) + { + for (int i = 0; i < StrList_Len(acl->admit.hostnames); i++) + { + if (StringMatchFull(StrList_At(acl->admit.hostnames, i), + hostname)) + { + pos = i; + break; + } + } + } + /* === === */ + if (pos != (size_t) -1) { - const char *rule = acl->admit_hostnames->list[pos]->str; - size_t rule_len = acl->admit_hostnames->list[pos]->len; + rule = acl->admit.hostnames->list[pos]->str; + size_t rule_len = acl->admit.hostnames->list[pos]->len; /* The rule in the access list has to be an exact match, or be a - * subdomain match (i.e. the rule begins with '.'). */ - if (rule_len == hostname_len || rule[0] == '.') + * subdomain match (i.e. the rule begins with '.') or a regex. */ + if (rule_len == hostname_len || rule[0] == '.' || regex_match) { Log(LOG_LEVEL_DEBUG, "access_Check: admit hostname: %s", rule); access = true; @@ -101,17 +123,20 @@ bool access_CheckResource(const struct resource_acl *acl, /* If access has been granted, we might need to deny it based on ACL. */ - if (access && acl->deny_ips != NULL) + if (access && acl->deny.ips != NULL) { bool found_rule = false; - for (int i = 0; i < acl->deny_ips->len; i++) + for (int i = 0; i < StrList_Len(acl->deny.ips); i++) { - if (FuzzySetMatch(acl->deny_ips->list[i]->str, + if (FuzzySetMatch(StrList_At(acl->deny.ips, i), MapAddress(ipaddr)) - == 0) + == 0 || + /* Legacy regex matching, TODO DEPRECATE */ + StringMatchFull(StrList_At(acl->deny.ips, i), + MapAddress(ipaddr))) { found_rule = true; - rule = acl->deny_ips->list[i]->str; + rule = acl->deny.ips->list[i]->str; break; } } @@ -122,29 +147,45 @@ bool access_CheckResource(const struct resource_acl *acl, access = false; } } - if (access && acl->deny_keys != NULL) + if (access && acl->deny.keys != NULL) { - bool ret = StrList_BinarySearch(acl->deny_keys, key, &pos); + bool ret = StrList_BinarySearch(acl->deny.keys, key, &pos); if (ret) { - rule = acl->deny_keys->list[pos]->str; + rule = StrList_At(acl->deny.keys, pos); Log(LOG_LEVEL_DEBUG, "access_Check: deny key: %s", rule); access = false; } } - if (access && acl->deny_hostnames != NULL && hostname != NULL) + if (access && acl->deny.hostnames != NULL && hostname != NULL) { size_t hostname_len = strlen(hostname); - size_t pos = StrList_SearchShortestPrefix(acl->deny_hostnames, + size_t pos = StrList_SearchShortestPrefix(acl->deny.hostnames, hostname, hostname_len, false); + /* === Legacy regex matching, slow, TODO DEPRECATE === */ + bool regex_match = false; + if (pos == (size_t) -1) + { + for (int i = 0; i < StrList_Len(acl->deny.hostnames); i++) + { + if (StringMatchFull(StrList_At(acl->deny.hostnames, i), + hostname)) + { + pos = i; + break; + } + } + } + /* === === */ + if (pos != (size_t) -1) { - const char *rule = acl->deny_hostnames->list[pos]->str; - size_t rule_len = acl->deny_hostnames->list[pos]->len; + rule = acl->deny.hostnames->list[pos]->str; + size_t rule_len = acl->deny.hostnames->list[pos]->len; /* The rule in the access list has to be an exact match, or be a - * subdomain match (i.e. the rule begins with '.'). */ - if (rule_len == hostname_len || rule[0] == '.') + * subdomain match (i.e. the rule begins with '.') or a regex. */ + if (rule_len == hostname_len || rule[0] == '.' || regex_match) { Log(LOG_LEVEL_DEBUG, "access_Check: deny hostname: %s", rule); access = false; @@ -170,6 +211,7 @@ bool acl_CheckPath(const struct acl *acl, const char *reqpath, bool access = false; /* Deny access by default */ size_t reqpath_len = strlen(reqpath); + /* CHECK 1: Search for parent directory or exact entry in ACL. */ size_t pos = StrList_SearchLongestPrefix(acl->resource_names, reqpath, reqpath_len, FILE_SEPARATOR, true); @@ -188,7 +230,7 @@ bool acl_CheckPath(const struct acl *acl, const char *reqpath, ret == true ? "true" : "false"); } - /* Put back special variables, so that we can find the entry in the ACL, + /* CHECK 2: replace ACL entry parts with special variables (if applicable), * e.g. turn "/path/to/192.168.1.1.json" * to "/path/to/$(connection.ip).json" */ char mangled_path[PATH_MAX]; @@ -319,7 +361,7 @@ size_t acl_SortedInsert(struct acl **a, const char *handle) acl->len++; /* 4. Initialise all ACLs for the resource as empty. */ - acl->acls[position] = (struct resource_acl) { 0 }; /* NULL acls <=> empty */ + acl->acls[position] = (struct resource_acl) { {0}, {0} }; /* NULL acls <=> empty */ Log(LOG_LEVEL_DEBUG, "Inserted in ACL position %zu: %s", position, handle); @@ -334,12 +376,12 @@ void acl_Free(struct acl *a) size_t i; for (i = 0; i < a->len; i++) { - StrList_Free(&a->acls[i].admit_ips); - StrList_Free(&a->acls[i].admit_hostnames); - StrList_Free(&a->acls[i].admit_keys); - StrList_Free(&a->acls[i].deny_ips); - StrList_Free(&a->acls[i].deny_hostnames); - StrList_Free(&a->acls[i].deny_keys); + StrList_Free(&a->acls[i].admit.ips); + StrList_Free(&a->acls[i].admit.hostnames); + StrList_Free(&a->acls[i].admit.keys); + StrList_Free(&a->acls[i].deny.ips); + StrList_Free(&a->acls[i].deny.hostnames); + StrList_Free(&a->acls[i].deny.keys); } free(a); diff --git a/cf-serverd/server_access.h b/cf-serverd/server_access.h index 88bbf8651f..0775779107 100644 --- a/cf-serverd/server_access.h +++ b/cf-serverd/server_access.h @@ -21,14 +21,11 @@ * @note: Currently these lists are binary searched, so after filling them up * make sure you call StrList_Sort() to sort them. */ -struct resource_acl +struct admitdeny_acl { - StrList *admit_ips; - StrList *admit_hostnames; - StrList *admit_keys; - StrList *deny_ips; - StrList *deny_hostnames; - StrList *deny_keys; + StrList *ips; /* admit_ips, deny_ips */ + StrList *hostnames; /* admit_hostnames, deny_hostnames */ + StrList *keys; /* admit_keys, deny_keys */ }; enum acl_type @@ -58,8 +55,12 @@ struct acl //TODO enum acl_type resource_type; size_t len; /* Length of the following arrays */ size_t alloc_len; /* Used for realloc() economy */ - StrList *resource_names; /* paths, class names, variables etc */ - struct resource_acl acls[]; + StrList *resource_names; /* paths, class names, variables etc */ + struct resource_acl + { + struct admitdeny_acl admit; + struct admitdeny_acl deny; + } acls[]; }; diff --git a/cf-serverd/server_classic.c b/cf-serverd/server_classic.c index e2ea964c76..f8b295a1cd 100644 --- a/cf-serverd/server_classic.c +++ b/cf-serverd/server_classic.c @@ -203,14 +203,6 @@ static int AccessControl(EvalContext *ctx, const char *req_path, ServerConnectio res = true; } - /* Check if the accesslist allows everything (will not be covered by - * first check if transrequest is not absolute which is IMPOSSIBLE SO - * THIS IS CHECK IS ALWAYS COVERED BY THE FIRST ONE). */ - /* if (strcmp(transpath, "/") == 0) */ - /* { */ - /* res = true; */ - /* } */ - if (res) { Log(LOG_LEVEL_VERBOSE, "Found a matching rule in access list (%s in %s)", transrequest, transpath); @@ -250,11 +242,6 @@ static int AccessControl(EvalContext *ctx, const char *req_path, ServerConnectio } } - /* Make sure again that the entry in the admit path (WAT?) is a prefix of - * the requested path. What if transpath is uninitialised because there - * were no ADMIT entries? Oh man this is horrible... */ - /* if (strncmp(transpath, transrequest, strlen(transpath)) == 0) */ - /* { */ for (Auth *dp = SV.deny; dp != NULL; dp = dp->next) { strncpy(transpath, dp->path, CF_BUFSIZE - 1); diff --git a/cf-serverd/server_transform.c b/cf-serverd/server_transform.c index 874a9b36e3..5faac02137 100644 --- a/cf-serverd/server_transform.c +++ b/cf-serverd/server_transform.c @@ -99,35 +99,35 @@ void Summarize() StrList_At(paths_acl->resource_names, i)); const struct resource_acl *racl = &paths_acl->acls[i]; - for (j = 0; j < StrList_Len(racl->admit_ips); j++) + for (j = 0; j < StrList_Len(racl->admit.ips); j++) { Log(LOG_LEVEL_VERBOSE, "\t\tadmit_ips: %s", - StrList_At(racl->admit_ips, j)); + StrList_At(racl->admit.ips, j)); } - for (j = 0; j < StrList_Len(racl->admit_hostnames); j++) + for (j = 0; j < StrList_Len(racl->admit.hostnames); j++) { Log(LOG_LEVEL_VERBOSE, "\t\tadmit_hostnames: %s", - StrList_At(racl->admit_hostnames, j)); + StrList_At(racl->admit.hostnames, j)); } - for (j = 0; j < StrList_Len(racl->admit_keys); j++) + for (j = 0; j < StrList_Len(racl->admit.keys); j++) { Log(LOG_LEVEL_VERBOSE, "\t\tadmit_keys: %s", - StrList_At(racl->admit_keys, j)); + StrList_At(racl->admit.keys, j)); } - for (j = 0; j < StrList_Len(racl->deny_ips); j++) + for (j = 0; j < StrList_Len(racl->deny.ips); j++) { Log(LOG_LEVEL_VERBOSE, "\t\tdeny_ips: %s", - StrList_At(racl->deny_ips, j)); + StrList_At(racl->deny.ips, j)); } - for (j = 0; j < StrList_Len(racl->deny_hostnames); j++) + for (j = 0; j < StrList_Len(racl->deny.hostnames); j++) { Log(LOG_LEVEL_VERBOSE, "\t\tdeny_hostnames: %s", - StrList_At(racl->deny_hostnames, j)); + StrList_At(racl->deny.hostnames, j)); } - for (j = 0; j < StrList_Len(racl->deny_keys); j++) + for (j = 0; j < StrList_Len(racl->deny.keys); j++) { Log(LOG_LEVEL_VERBOSE, "\t\tdeny_keys: %s", - StrList_At(racl->deny_keys, j)); + StrList_At(racl->deny.keys, j)); } } @@ -151,7 +151,7 @@ void Summarize() for (ip = ptr->accesslist; ip != NULL; ip = ip->next) { - Log(LOG_LEVEL_VERBOSE, "\t\tadmit hosts: %s", ip->name); + Log(LOG_LEVEL_VERBOSE, "\t\tadmit: %s", ip->name); } } @@ -167,7 +167,7 @@ void Summarize() for (ip = ptr->accesslist; ip != NULL; ip = ip->next) { - Log(LOG_LEVEL_VERBOSE, "\t\tdeny hosts: %s", ip->name); + Log(LOG_LEVEL_VERBOSE, "\t\tdeny: %s", ip->name); } } @@ -696,8 +696,9 @@ static enum admit_type AdmitType(const char *s) { return ADMIT_TYPE_KEY; } - /* IPv4 or IPv6 subnet mask */ - else if (s[strspn(s, "0123456789abcdef.:/")] == '\0') + /* IPv4 or IPv6 subnet mask or regex. */ + /* TODO change this to "0123456789abcdef.:/", no regex allowed. */ + else if (s[strspn(s, "0123456789abcdef.:/[-]*()\\")] == '\0') { return ADMIT_TYPE_IP; } @@ -707,6 +708,34 @@ static enum admit_type AdmitType(const char *s) } } +static size_t racl_SmartAppend(struct admitdeny_acl *ad, const char *entry) +{ + size_t ret; + + switch (AdmitType(entry)) + { + case ADMIT_TYPE_IP: + /* TODO convert IP string to binary representation. */ + ret = StrList_Append(&ad->ips, entry); + break; + case ADMIT_TYPE_KEY: + ret = StrList_Append(&ad->keys, entry); + break; + case ADMIT_TYPE_HOSTNAME: + /* TODO clean up possible regex, if it starts with ".*" + * then store two entries: entry, and *dot*entry. */ + ret = StrList_Append(&ad->hostnames, entry); + break; + default: + Log(LOG_LEVEL_WARNING, + "Access rule 'admit: %s' is not IP, hostname or key, ignoring", + entry); + ret = (size_t) -1; + } + + return ret; +} + /** * Add access rules to the given ACL #acl according to the constraints in the * particular access promise. @@ -716,7 +745,7 @@ static enum admit_type AdmitType(const char *s) */ static void AccessPromise_AddAccessConstraints(const EvalContext *ctx, const Promise *pp, - struct resource_acl *acl, + struct resource_acl *racl, Auth *ap, Auth *dp) { Rlist *rp; @@ -780,32 +809,32 @@ static void AccessPromise_AddAccessConstraints(const EvalContext *ctx, if (strcmp(cp->lval, CF_REMACCESS_BODIES[REMOTE_ACCESS_ADMITIPS].lval) == 0) { - ret = StrList_Append(&acl->admit_ips, RlistScalarValue(rp)); + ret = StrList_Append(&racl->admit.ips, RlistScalarValue(rp)); continue; } if (strcmp(cp->lval, CF_REMACCESS_BODIES[REMOTE_ACCESS_DENYIPS].lval) == 0) { - ret = StrList_Append(&acl->deny_ips, RlistScalarValue(rp)); + ret = StrList_Append(&racl->deny.ips, RlistScalarValue(rp)); continue; } if (strcmp(cp->lval, CF_REMACCESS_BODIES[REMOTE_ACCESS_ADMITHOSTNAMES].lval) == 0) { - ret = StrList_Append(&acl->admit_hostnames, RlistScalarValue(rp)); + ret = StrList_Append(&racl->admit.hostnames, RlistScalarValue(rp)); continue; } if (strcmp(cp->lval, CF_REMACCESS_BODIES[REMOTE_ACCESS_DENYHOSTNAMES].lval) == 0) { - ret = StrList_Append(&acl->deny_hostnames, RlistScalarValue(rp)); + ret = StrList_Append(&racl->deny.hostnames, RlistScalarValue(rp)); continue; } if (strcmp(cp->lval, CF_REMACCESS_BODIES[REMOTE_ACCESS_ADMITKEYS].lval) == 0) { - ret = StrList_Append(&acl->admit_keys, RlistScalarValue(rp)); + ret = StrList_Append(&racl->admit.keys, RlistScalarValue(rp)); continue; } if (strcmp(cp->lval, CF_REMACCESS_BODIES[REMOTE_ACCESS_DENYKEYS].lval) == 0) { - ret = StrList_Append(&acl->deny_keys, RlistScalarValue(rp)); + ret = StrList_Append(&racl->deny.keys, RlistScalarValue(rp)); continue; } @@ -813,48 +842,13 @@ static void AccessPromise_AddAccessConstraints(const EvalContext *ctx, if (strcmp(cp->lval, CF_REMACCESS_BODIES[REMOTE_ACCESS_ADMIT].lval) == 0) { - switch (AdmitType(RlistScalarValue(rp))) - { - case ADMIT_TYPE_IP: - /* TODO convert IP string to binary representation. */ - ret = StrList_Append(&acl->admit_ips, RlistScalarValue(rp)); - break; - case ADMIT_TYPE_KEY: - ret = StrList_Append(&acl->admit_keys, RlistScalarValue(rp)); - break; - case ADMIT_TYPE_HOSTNAME: - /* TODO clean up possible regex, if it starts with ".*" - * then store two entries: itself, and *dot*itself. */ - ret = StrList_Append(&acl->admit_hostnames, RlistScalarValue(rp)); - break; - default: - Log(LOG_LEVEL_WARNING, - "Access rule 'admit: %s' is not IP, hostname or key, ignoring", - RlistScalarValue(rp)); - } - + ret = racl_SmartAppend(&racl->admit, RlistScalarValue(rp)); PrependItem(&(ap->accesslist), RlistScalarValue(rp), NULL); continue; } if (strcmp(cp->lval, CF_REMACCESS_BODIES[REMOTE_ACCESS_DENY].lval) == 0) { - switch (AdmitType(RlistScalarValue(rp))) - { - case ADMIT_TYPE_IP: - ret = StrList_Append(&acl->deny_ips, RlistScalarValue(rp)); - break; - case ADMIT_TYPE_KEY: - ret = StrList_Append(&acl->deny_keys, RlistScalarValue(rp)); - break; - case ADMIT_TYPE_HOSTNAME: - ret = StrList_Append(&acl->deny_hostnames, RlistScalarValue(rp)); - break; - default: - Log(LOG_LEVEL_WARNING, - "Access rule 'deny: %s' is not IP, hostname or key, ignoring", - RlistScalarValue(rp)); - } - + ret = racl_SmartAppend(&racl->deny, RlistScalarValue(rp)); PrependItem(&(dp->accesslist), RlistScalarValue(rp), NULL); continue; } @@ -881,23 +875,23 @@ static void AccessPromise_AddAccessConstraints(const EvalContext *ctx, } } - StrList_Finalise(&acl->admit_ips); - StrList_Sort(acl->admit_ips, string_Compare); + StrList_Finalise(&racl->admit.ips); + StrList_Sort(racl->admit.ips, string_Compare); - StrList_Finalise(&acl->admit_hostnames); - StrList_Sort(acl->admit_hostnames, string_CompareFromEnd); + StrList_Finalise(&racl->admit.hostnames); + StrList_Sort(racl->admit.hostnames, string_CompareFromEnd); - StrList_Finalise(&acl->admit_keys); - StrList_Sort(acl->admit_keys, string_Compare); + StrList_Finalise(&racl->admit.keys); + StrList_Sort(racl->admit.keys, string_Compare); - StrList_Finalise(&acl->deny_ips); - StrList_Sort(acl->deny_ips, string_Compare); + StrList_Finalise(&racl->deny.ips); + StrList_Sort(racl->deny.ips, string_Compare); - StrList_Finalise(&acl->deny_hostnames); - StrList_Sort(acl->deny_hostnames, string_CompareFromEnd); + StrList_Finalise(&racl->deny.hostnames); + StrList_Sort(racl->deny.hostnames, string_CompareFromEnd); - StrList_Finalise(&acl->deny_keys); - StrList_Sort(acl->deny_keys, string_Compare); + StrList_Finalise(&racl->deny.keys); + StrList_Sort(racl->deny.keys, string_Compare); } /* It is allowed to have duplicate handles (paths or class names or variables