From: Grigorii Demidov Date: Tue, 22 May 2018 14:39:58 +0000 (+0200) Subject: daemon/tls: system CA's are used by default with TLS_FORWARD policy when ca_file... X-Git-Tag: v2.4.0~28^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=966430d7d85b984f75bcf603423baf64a2ae8e6c;p=thirdparty%2Fknot-resolver.git daemon/tls: system CA's are used by default with TLS_FORWARD policy when ca_file parameter is omitted --- diff --git a/daemon/bindings.c b/daemon/bindings.c index 17df57c9f..570b9ed91 100644 --- a/daemon/bindings.c +++ b/daemon/bindings.c @@ -478,7 +478,7 @@ static int net_tls_client(lua_State *L) const char *full_addr = NULL; bool pin_exists = false; - bool ca_file_exists = false; + bool hostname_exists = false; if ((lua_gettop(L) == 1) && lua_isstring(L, 1)) { full_addr = lua_tostring(L, 1); } else if ((lua_gettop(L) == 2) && lua_isstring(L, 1) && lua_istable(L, 2)) { @@ -486,12 +486,12 @@ static int net_tls_client(lua_State *L) pin_exists = true; } else if ((lua_gettop(L) == 3) && lua_isstring(L, 1) && lua_istable(L, 2)) { full_addr = lua_tostring(L, 1); - ca_file_exists = true; + hostname_exists = true; } else if ((lua_gettop(L) == 4) && lua_isstring(L, 1) && lua_istable(L, 2) && lua_istable(L, 3)) { full_addr = lua_tostring(L, 1); pin_exists = true; - ca_file_exists = true; + hostname_exists = true; } else { format_error(L, "net.tls_client takes one parameter (\"address\"), two parameters (\"address\",\"pin\"), three parameters (\"address\", \"ca_file\", \"hostname\") or four ones: (\"address\", \"pin\", \"ca_file\", \"hostname\")"); lua_error(L); @@ -508,9 +508,10 @@ static int net_tls_client(lua_State *L) port = 853; } - if (!pin_exists && !ca_file_exists) { + if (!pin_exists && !hostname_exists) { int r = tls_client_params_set(&net->tls_client_params, - addr, port, NULL, NULL, NULL); + addr, port, NULL, + TLS_CLIENT_PARAM_NONE); if (r != 0) { lua_pushstring(L, kr_strerror(r)); lua_error(L); @@ -528,7 +529,8 @@ static int net_tls_client(lua_State *L) /* pin now at index -1, key at index -2*/ const char *pin = lua_tostring(L, -1); int r = tls_client_params_set(&net->tls_client_params, - addr, port, NULL, NULL, pin); + addr, port, pin, + TLS_CLIENT_PARAM_PIN); if (r != 0) { lua_pushstring(L, kr_strerror(r)); lua_error(L); @@ -539,7 +541,7 @@ static int net_tls_client(lua_State *L) int ca_table_index = 2; int hostname_table_index = 3; - if (ca_file_exists) { + if (hostname_exists) { if (pin_exists) { ca_table_index = 3; hostname_table_index = 4; @@ -549,12 +551,14 @@ static int net_tls_client(lua_State *L) return 1; } - /* iterate over ca filenames */ + /* iterate over hostnames, + * it must be done before iterating over ca filenames */ lua_pushnil(L); - while (lua_next(L, ca_table_index)) { - const char *ca_file = lua_tostring(L, -1); + while (lua_next(L, hostname_table_index)) { + const char *hostname = lua_tostring(L, -1); int r = tls_client_params_set(&net->tls_client_params, - addr, port, ca_file, NULL, NULL); + addr, port, hostname, + TLS_CLIENT_PARAM_HOSTNAME); if (r != 0) { lua_pushstring(L, kr_strerror(r)); lua_error(L); @@ -563,20 +567,34 @@ static int net_tls_client(lua_State *L) lua_pop(L, 1); } - /* iterate over hostnames */ + /* iterate over ca filenames */ lua_pushnil(L); - while (lua_next(L, hostname_table_index)) { - const char *hostname = lua_tostring(L, -1); + size_t num_of_ca_files = 0; + while (lua_next(L, ca_table_index)) { + const char *ca_file = lua_tostring(L, -1); int r = tls_client_params_set(&net->tls_client_params, - addr, port, NULL, hostname, NULL); + addr, port, ca_file, + TLS_CLIENT_PARAM_CA); if (r != 0) { lua_pushstring(L, kr_strerror(r)); lua_error(L); } + num_of_ca_files += 1; /* removes 'value'; keeps 'key' for next iteration */ lua_pop(L, 1); } + if (num_of_ca_files == 0) { + /* No ca files were explicitly configured, so use system CA */ + int r = tls_client_params_set(&net->tls_client_params, + addr, port, NULL, + TLS_CLIENT_PARAM_CA); + if (r != 0) { + lua_pushstring(L, kr_strerror(r)); + lua_error(L); + } + } + lua_pushboolean(L, true); return 1; } diff --git a/daemon/tls.c b/daemon/tls.c index be9a40c05..c09fac5b0 100644 --- a/daemon/tls.c +++ b/daemon/tls.c @@ -583,12 +583,22 @@ static int client_paramlist_entry_clear(const char *k, void *v, void *baton) int tls_client_params_set(map_t *tls_client_paramlist, const char *addr, uint16_t port, - const char *ca_file, const char *hostname, const char *pin) + const char *param, tls_client_param_t param_type) { if (!tls_client_paramlist || !addr) { return kr_error(EINVAL); } + /* TLS_CLIENT_PARAM_CA can be empty */ + if (param_type == TLS_CLIENT_PARAM_HOSTNAME || + param_type == TLS_CLIENT_PARAM_PIN) { + if (param == NULL || param[0] == 0) { + return kr_error(EINVAL); + } + } + + /* Parameters are OK */ + char key[INET6_ADDRSTRLEN + 6]; size_t keylen = sizeof(key); if (kr_straddr_join(addr, port, key, &keylen) != kr_ok()) { @@ -615,23 +625,53 @@ int tls_client_params_set(map_t *tls_client_paramlist, } int ret = kr_ok(); - if (ca_file && ca_file[0] != 0) { + + if (param_type == TLS_CLIENT_PARAM_HOSTNAME) { + const char *hostname = param; + bool already_exists = false; + for (size_t i = 0; i < entry->hostnames.len; ++i) { + if (strcmp(entry->hostnames.at[i], hostname) == 0) { + kr_log_error("[tls_client] error: hostname '%s' for address '%s' already was set, ignoring\n", hostname, key); + already_exists = true; + break; + } + } + if (!already_exists) { + const char *value = strdup(hostname); + if (!value) { + ret = kr_error(ENOMEM); + } else if (array_push(entry->hostnames, value) < 0) { + free ((void *)value); + ret = kr_error(ENOMEM); + } + } + } else if (param_type == TLS_CLIENT_PARAM_CA) { + /* Import ca files only when hostname is already set */ + if (entry->hostnames.len == 0) { + return kr_error(ENOENT); + } + const char *ca_file = param; bool already_exists = false; for (size_t i = 0; i < entry->ca_files.len; ++i) { - if (strcmp(entry->ca_files.at[i], ca_file) == 0) { - kr_log_error("[tls_client] error: ca file '%s'for address '%s' already was set, ignoring\n", ca_file, key); + const char *imported_ca = entry->ca_files.at[i]; + if (imported_ca[0] == 0 && (ca_file == NULL || ca_file[0] == 0)) { + kr_log_error("[tls_client] error: system ca for address '%s' already was set, ignoring\n", key); + already_exists = true; + break; + } else if (strcmp(imported_ca, ca_file) == 0) { + kr_log_error("[tls_client] error: ca file '%s' for address '%s' already was set, ignoring\n", ca_file, key); already_exists = true; break; } } if (!already_exists) { - const char *value = strdup(ca_file); + const char *value = strdup(ca_file != NULL ? ca_file : ""); if (!value) { ret = kr_error(ENOMEM); } else if (array_push(entry->ca_files, value) < 0) { free ((void *)value); ret = kr_error(ENOMEM); - } else if (strcmp(ca_file, "system ca store") == 0) { + } else if (value[0] == 0) { int res = gnutls_certificate_set_x509_system_trust (entry->credentials); if (res <= 0) { kr_log_error("[tls_client] failed to import certs from system store (%s)\n", @@ -656,29 +696,8 @@ int tls_client_params_set(map_t *tls_client_paramlist, } } } - } - - if ((ret == kr_ok()) && hostname && hostname[0] != 0) { - bool already_exists = false; - for (size_t i = 0; i < entry->hostnames.len; ++i) { - if (strcmp(entry->hostnames.at[i], hostname) == 0) { - kr_log_error("[tls_client] error: hostname '%s' for address '%s' already was set, ignoring\n", hostname, key); - already_exists = true; - break; - } - } - if (!already_exists) { - const char *value = strdup(hostname); - if (!value) { - ret = kr_error(ENOMEM); - } else if (array_push(entry->hostnames, value) < 0) { - free ((void *)value); - ret = kr_error(ENOMEM); - } - } - } - - if ((ret == kr_ok()) && pin && pin[0] != 0) { + } else if (param_type == TLS_CLIENT_PARAM_PIN) { + const char *pin = param; for (size_t i = 0; i < entry->pins.len; ++i) { if (strcmp(entry->pins.at[i], pin) == 0) { kr_log_error("[tls_client] warning: pin '%s' for address '%s' already was set, ignoring\n", pin, key); diff --git a/daemon/tls.h b/daemon/tls.h index c638fb764..00330ef4a 100644 --- a/daemon/tls.h +++ b/daemon/tls.h @@ -57,6 +57,13 @@ typedef enum tls_client_hs_state { typedef int (*tls_handshake_cb) (struct session *session, int status); +typedef enum tls_client_param { + TLS_CLIENT_PARAM_NONE = 0, + TLS_CLIENT_PARAM_PIN, + TLS_CLIENT_PARAM_HOSTNAME, + TLS_CLIENT_PARAM_CA, +} tls_client_param_t; + struct tls_common_ctx { bool client_side; gnutls_session_t tls_session; @@ -134,10 +141,13 @@ tls_hs_state_t tls_get_hs_state(const struct tls_common_ctx *ctx); /*! Set TLS handshake state. */ int tls_set_hs_state(struct tls_common_ctx *ctx, tls_hs_state_t state); -/*! Set TLS authentication parameters for given address. */ +/*! Set TLS authentication parameters for given address. + * Note: hostnames must be imported before ca files, + * otherwise ca files will not be imported at all. + */ int tls_client_params_set(map_t *tls_client_paramlist, const char *addr, uint16_t port, - const char *ca_file, const char *hostname, const char *pin); + const char *param, tls_client_param_t param_type); /*! Free TLS authentication parameters. */ int tls_client_params_free(map_t *tls_client_paramlist); diff --git a/modules/policy/policy.lua b/modules/policy/policy.lua index f053a40ce..df52f33fb 100644 --- a/modules/policy/policy.lua +++ b/modules/policy/policy.lua @@ -170,16 +170,16 @@ local function tls_forward_target_authtype(idx, target) return 'pin_sha256' elseif (target.insecure and not (target.ca_file or target.hostname or target.pin_sha256)) then return 'insecure' - elseif (target.ca_file and target.hostname and not (target.insecure or target.pin_sha256)) then - if not (is_nonempty_string_or_table(target.hostname) - and is_nonempty_string_or_table(target.ca_file)) then + elseif (target.hostname and not (target.insecure or target.pin_sha256)) then + if not (is_nonempty_string_or_table(target.hostname)) then error('TLS_FORWARD target authentication is invalid at position ' - .. idx .. '; hostname and ca_file must be string or list of strings') + .. idx .. '; hostname must be string or list of strings') end + -- if target.ca_file is empty, system CA will be used return 'cert' else error('TLS_FORWARD authentication options at position ' .. idx - .. ' are invalid; specify one of: pin_sha256 / hostname+ca_file / insecure') + .. ' are invalid; specify one of: pin_sha256 / hostname [+ca_file] / insecure') end end @@ -238,7 +238,6 @@ function policy.TLS_FORWARD(target) assert(#pins[sockaddr_lua] > 0) net.tls_client(config.string_addr, pins[sockaddr_lua]) elseif config.auth_type == 'cert' then - assert(#ca_files[sockaddr_lua] > 0) assert(#hostnames[sockaddr_lua] > 0) net.tls_client(config.string_addr, ca_files[sockaddr_lua], hostnames[sockaddr_lua]) else diff --git a/modules/policy/policy.test.lua b/modules/policy/policy.test.lua index ef0fff9aa..ccb3f5b6c 100644 --- a/modules/policy/policy.test.lua +++ b/modules/policy/policy.test.lua @@ -39,7 +39,7 @@ local function test_tls_forward() }}}), 'TLS_FORWARD with table of pins') -- ok(policy.TLS_FORWARD({{'::1', hostname='test.', ca_file='/tmp/ca.crt'}}), 'TLS_FORWARD with hostname + CA cert') - boom(policy.TLS_FORWARD, {{{'::1', hostname='test.'}}}, 'TLS_FORWARD with just hostname') + -- boom(policy.TLS_FORWARD, {{{'::1', hostname='test.'}}}, 'TLS_FORWARD with just hostname') boom(policy.TLS_FORWARD, {{{'::1', ca_file='/tmp/ca.crt'}}}, 'TLS_FORWARD with just CA cert') boom(policy.TLS_FORWARD, {{{'::1', hostname='', ca_file='/tmp/ca.crt'}}}, 'TLS_FORWARD with empty hostname + CA cert') boom(policy.TLS_FORWARD, {{{'::1', hostname='test.', ca_file='/dev/a_file_which_surely_does_NOT_exist!'}}},