From 0ee6f66cbddb69b599be4b191ec8c5531e9e9e2f Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Fri, 12 Nov 2021 16:53:13 +0200 Subject: [PATCH] Integrate extended ACLs syntax featuring 'port' and 'transport' opts This commit completes the integration of the new, extended ACL syntax featuring 'port' and 'transport' options. The runtime presentation and ACL loading code are extended to allow the syntax to be used beyond the 'allow-transfer' option (e.g. in 'acl' definitions and other 'allow-*' options) and can be used to ultimately extend the ACL support with transport-only ACLs (e.g. 'transport-acl tls-acl port 853 transport tls'). But, due to fundamental nature of such a change, it has not been completed as a part of 9.17.X release series due to it being close to 9.18 stable release status. That means that we do not have enough time to fully test it. The complete integration is planned as a part of 9.19.X release series. The code was manually verified to work as expected by temporarily enabling the extended syntax for 'acl' statements and 'allow-query' options, including ACL merging, negated ACLs. --- lib/dns/acl.c | 122 ++++++++++++++++++++++++++++++++++++++ lib/dns/include/dns/acl.h | 52 ++++++++++++++++ lib/isccfg/aclconf.c | 62 ++++++++++++++++++- lib/ns/client.c | 9 ++- 4 files changed, 241 insertions(+), 4 deletions(-) diff --git a/lib/dns/acl.c b/lib/dns/acl.c index 33fcefef0a..a3cd6d4214 100644 --- a/lib/dns/acl.c +++ b/lib/dns/acl.c @@ -71,6 +71,9 @@ dns_acl_create(isc_mem_t *mctx, int n, dns_acl_t **target) { acl->elements = isc_mem_get(mctx, n * sizeof(dns_aclelement_t)); acl->alloc = n; memset(acl->elements, 0, n * sizeof(dns_aclelement_t)); + ISC_LIST_INIT(acl->ports_and_transports); + acl->port_proto_entries = 0; + *target = acl; return (ISC_R_SUCCESS); } @@ -241,6 +244,54 @@ dns_acl_match(const isc_netaddr_t *reqaddr, const dns_name_t *reqsigner, return (ISC_R_SUCCESS); } +isc_result_t +dns_acl_match_port_transport(const isc_netaddr_t *reqaddr, + const in_port_t local_port, + const isc_nmsocket_type_t transport, + const bool encrypted, const dns_name_t *reqsigner, + const dns_acl_t *acl, const dns_aclenv_t *env, + int *match, const dns_aclelement_t **matchelt) { + isc_result_t result = ISC_R_SUCCESS; + dns_acl_port_transports_t *next; + + REQUIRE(reqaddr != NULL); + REQUIRE(DNS_ACL_VALID(acl)); + + if (!ISC_LIST_EMPTY(acl->ports_and_transports)) { + result = ISC_R_FAILURE; + for (next = ISC_LIST_HEAD(acl->ports_and_transports); + next != NULL; next = ISC_LIST_NEXT(next, link)) + { + bool match_port = true; + bool match_transport = true; + + if (next->port != 0) { + /* Port is specified. */ + match_port = (local_port == next->port); + } + if (next->transports != 0) { + /* Transport protocol is specified. */ + match_transport = + ((transport & next->transports) == + transport && + next->encrypted == encrypted); + } + + if (match_port && match_transport) { + result = next->negative ? ISC_R_FAILURE + : ISC_R_SUCCESS; + break; + } + } + } + + if (result != ISC_R_SUCCESS) { + return (result); + } + + return (dns_acl_match(reqaddr, reqsigner, acl, env, match, matchelt)); +} + /* * Merge the contents of one ACL into another. Call dns_iptable_merge() * for the IP tables, then concatenate the element arrays. @@ -347,6 +398,11 @@ dns_acl_merge(dns_acl_t *dest, dns_acl_t *source, bool pos) { dns_acl_node_count(dest) = nodes; } + /* + * Merge ports and transports + */ + dns_acl_merge_ports_transports(dest, source, pos); + return (ISC_R_SUCCESS); } @@ -449,6 +505,7 @@ dns_acl_attach(dns_acl_t *source, dns_acl_t **target) { static void destroy(dns_acl_t *dacl) { unsigned int i; + dns_acl_port_transports_t *port_proto; INSIST(!ISC_LINK_LINKED(dacl, nextincache)); @@ -470,6 +527,17 @@ destroy(dns_acl_t *dacl) { if (dacl->iptable != NULL) { dns_iptable_detach(&dacl->iptable); } + + port_proto = ISC_LIST_HEAD(dacl->ports_and_transports); + while (port_proto != NULL) { + dns_acl_port_transports_t *next = NULL; + + next = ISC_LIST_NEXT(port_proto, link); + ISC_LIST_DEQUEUE(dacl->ports_and_transports, port_proto, link); + isc_mem_put(dacl->mctx, port_proto, sizeof(*port_proto)); + port_proto = next; + } + isc_refcount_destroy(&dacl->refcount); dacl->magic = 0; isc_mem_putanddetach(&dacl->mctx, dacl, sizeof(*dacl)); @@ -707,3 +775,57 @@ dns_aclenv_detach(dns_aclenv_t **aclenvp) { dns__aclenv_destroy(aclenv); } } + +void +dns_acl_add_port_transports(dns_acl_t *acl, const in_port_t port, + const uint32_t transports, const bool encrypted, + const bool negative) { + dns_acl_port_transports_t *port_proto; + REQUIRE(DNS_ACL_VALID(acl)); + REQUIRE(port != 0 || transports != 0); + + port_proto = isc_mem_get(acl->mctx, sizeof(*port_proto)); + *port_proto = (dns_acl_port_transports_t){ .port = port, + .transports = transports, + .encrypted = encrypted, + .negative = negative }; + + ISC_LINK_INIT(port_proto, link); + + ISC_LIST_APPEND(acl->ports_and_transports, port_proto, link); + acl->port_proto_entries++; +} + +void +dns_acl_merge_ports_transports(dns_acl_t *dest, dns_acl_t *source, bool pos) { + dns_acl_port_transports_t *next; + + REQUIRE(DNS_ACL_VALID(dest)); + REQUIRE(DNS_ACL_VALID(source)); + + const bool negative = !pos; + + /* + * Merge ports and transports + */ + for (next = ISC_LIST_HEAD(source->ports_and_transports); next != NULL; + next = ISC_LIST_NEXT(next, link)) + { + const bool next_positive = !next->negative; + bool add_negative; + + /* + * Reverse sense of positives if this is a negative acl. The + * logic is used (and, thus, enforced) by dns_acl_merge(), + * from which dns_acl_merge_ports_transports() is called. + */ + if (negative && next_positive) { + add_negative = true; + } else { + add_negative = next->negative; + } + + dns_acl_add_port_transports(dest, next->port, next->transports, + next->encrypted, add_negative); + } +} diff --git a/lib/dns/include/dns/acl.h b/lib/dns/include/dns/acl.h index e122340d1b..51678eee3d 100644 --- a/lib/dns/include/dns/acl.h +++ b/lib/dns/include/dns/acl.h @@ -52,6 +52,14 @@ typedef enum { dns_aclelementtype_any } dns_aclelementtype_t; +typedef struct dns_acl_port_transports { + in_port_t port; + uint32_t transports; + bool encrypted; /* for protocols with optional encryption (e.g. HTTP) */ + bool negative; + ISC_LINK(struct dns_acl_port_transports) link; +} dns_acl_port_transports_t; + typedef struct dns_aclipprefix dns_aclipprefix_t; struct dns_aclipprefix { @@ -83,6 +91,8 @@ struct dns_acl { unsigned int length; /*%< Elements initialized */ char *name; /*%< Temporary use only */ ISC_LINK(dns_acl_t) nextincache; /*%< Ditto */ + ISC_LIST(dns_acl_port_transports_t) ports_and_transports; + size_t port_proto_entries; }; struct dns_aclenv { @@ -270,4 +280,46 @@ dns_aclelement_match(const isc_netaddr_t *reqaddr, const dns_name_t *reqsigner, * returned through 'matchelt' is not necessarily 'e' itself. */ +isc_result_t +dns_acl_match_port_transport(const isc_netaddr_t *reqaddr, + const in_port_t local_port, + const isc_nmsocket_type_t transport, + const bool encrypted, const dns_name_t *reqsigner, + const dns_acl_t *acl, const dns_aclenv_t *env, + int *match, const dns_aclelement_t **matchelt); +/*%< + * Like dns_acl_match, but able to match the server port and + * transport, as well as encryption status. + * + * Requires: + *\li 'reqaddr' is not 'NULL'; + *\li 'acl' is a valid ACL object. + */ + +void +dns_acl_add_port_transports(dns_acl_t *acl, const in_port_t port, + const uint32_t transports, const bool encrypted, + const bool negative); +/*%< + * Adds a "port-transports" entry to the specified ACL. Transports + * are specified as a bit-set 'transports' consisting of entries + * defined in the isc_nmsocket_type enumeration. + * + * Requires: + *\li 'acl' is a valid ACL object; + *\li either 'port' or 'transports' is not equal to 0. + */ + +void +dns_acl_merge_ports_transports(dns_acl_t *dest, dns_acl_t *source, bool pos); +/*%< + * Merges "port-transports" entries from the 'dest' ACL into + * the 'source' ACL. The 'pos' parameter works in a way similar to + * 'dns_acl_merge()'. + * + * Requires: + *\li 'dest' is a valid ACL object; + *\li 'source' is a valid ACL object. + */ + ISC_LANG_ENDDECLS diff --git a/lib/isccfg/aclconf.c b/lib/isccfg/aclconf.c index 9f2fea573b..e74fa3b918 100644 --- a/lib/isccfg/aclconf.c +++ b/lib/isccfg/aclconf.c @@ -640,7 +640,8 @@ cfg_acl_fromconfig2(const cfg_obj_t *acl_data, const cfg_obj_t *cctx, bool setpos; const cfg_obj_t *caml = NULL; const cfg_obj_t *obj_acl_tuple = NULL; - const cfg_obj_t *obj_port = NULL, *obj_proto = NULL; + const cfg_obj_t *obj_port = NULL, *obj_transport = NULL; + bool is_tuple = false; if (nest_level != 0) { new_nest_level = nest_level - 1; @@ -655,12 +656,13 @@ cfg_acl_fromconfig2(const cfg_obj_t *acl_data, const cfg_obj_t *cctx, caml = acl_data; } else { INSIST(cfg_obj_istuple(acl_data)); - caml = cfg_tuple_get(acl_data, "acl"); + caml = cfg_tuple_get(acl_data, "aml"); INSIST(caml != NULL); obj_acl_tuple = cfg_tuple_get(acl_data, "port-transport"); INSIST(obj_acl_tuple != NULL); obj_port = cfg_tuple_get(obj_acl_tuple, "port"); - obj_proto = cfg_tuple_get(obj_acl_tuple, "protocol"); + obj_transport = cfg_tuple_get(obj_acl_tuple, "transport"); + is_tuple = true; } if (*target != NULL) { @@ -697,6 +699,54 @@ cfg_acl_fromconfig2(const cfg_obj_t *acl_data, const cfg_obj_t *cctx, } } + if (is_tuple) { + uint16_t port = 0; + uint32_t transports = 0; + bool encrypted = false; + + if (obj_port != NULL && cfg_obj_isuint32(obj_port)) { + port = (uint16_t)cfg_obj_asuint32(obj_port); + } + + if (obj_transport != NULL && cfg_obj_isstring(obj_transport)) { + if (strcasecmp(cfg_obj_asstring(obj_transport), + "udp") == 0) { + transports = isc_nm_udpsocket; + encrypted = false; + } else if (strcasecmp(cfg_obj_asstring(obj_transport), + "tcp") == 0) { + transports = isc_nm_tcpdnssocket; + encrypted = false; + } else if (strcasecmp(cfg_obj_asstring(obj_transport), + "udp-tcp") == 0) { + /* Good ol' DNS over port 53 */ + transports = isc_nm_tcpdnssocket | + isc_nm_udpsocket; + encrypted = false; + } else if (strcasecmp(cfg_obj_asstring(obj_transport), + "tls") == 0) { + transports = isc_nm_tlsdnssocket; + encrypted = true; + } else if (strcasecmp(cfg_obj_asstring(obj_transport), + "http") == 0) { + transports = isc_nm_httpsocket; + encrypted = true; + } else if (strcasecmp(cfg_obj_asstring(obj_transport), + "http-plain") == 0) { + transports = isc_nm_httpsocket; + encrypted = false; + } else { + result = ISC_R_FAILURE; + goto cleanup; + } + } + + if (port != 0 || transports != 0) { + dns_acl_add_port_transports(dacl, port, transports, + encrypted, false); + } + } + de = dacl->elements; for (elt = cfg_list_first(caml); elt != NULL; elt = cfg_list_next(elt)) { @@ -803,6 +853,12 @@ cfg_acl_fromconfig2(const cfg_obj_t *acl_data, const cfg_obj_t *cctx, if (de->nestedacl != NULL) { dns_acl_detach(&de->nestedacl); } + /* + * Merge the port-transports entries from the + * nested ACL into its parent. + */ + dns_acl_merge_ports_transports(dacl, inneracl, + !neg); dns_acl_attach(inneracl, &de->nestedacl); dns_acl_detach(&inneracl); /* Fall through. */ diff --git a/lib/ns/client.c b/lib/ns/client.c index 0825fc3e30..f598387729 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -2553,6 +2553,7 @@ ns_client_checkaclsilent(ns_client_t *client, isc_netaddr_t *netaddr, dns_aclenv_t *env = client->manager->aclenv; isc_netaddr_t tmpnetaddr; int match; + isc_sockaddr_t local; if (acl == NULL) { if (default_allow) { @@ -2567,7 +2568,13 @@ ns_client_checkaclsilent(ns_client_t *client, isc_netaddr_t *netaddr, netaddr = &tmpnetaddr; } - result = dns_acl_match(netaddr, client->signer, acl, env, &match, NULL); + local = isc_nmhandle_localaddr(client->handle); + result = dns_acl_match_port_transport( + netaddr, isc_sockaddr_getport(&local), + isc_nm_socket_type(client->handle), + isc_nm_has_encryption(client->handle), client->signer, acl, env, + &match, NULL); + if (result != ISC_R_SUCCESS) { goto deny; /* Internal error, already logged. */ }