From c70201f1e3c85badecfbb83092e1983e50ed25e9 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 28 Nov 2019 08:14:59 +0100 Subject: [PATCH 1/6] auth-cfg: Add support for identity based CA authentication constraints Enforcing CA based constraints previously required the CA certificate file to be locally installed. This is problematic from a maintencance perspective when having many intermediate CAs, and is actually redundant if the client sends its intermediate cert in the request. The alternative was to use Distinguished Name matching in the subject identity to indirectly check for the issuing CA by some RDN field, such as OU. However, this requires trust in the intermediate CA to issue only certificates with legitime subject identities. This new approach checks for an intermediate CA by comparing the issuing identity. This does not require trust in the intermediate, as long as a path len constraint prevents that intermediate to issue further intermediate certificates. --- src/libstrongswan/credentials/auth_cfg.c | 41 ++++++++++++++++++++---- src/libstrongswan/credentials/auth_cfg.h | 2 ++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/libstrongswan/credentials/auth_cfg.c b/src/libstrongswan/credentials/auth_cfg.c index b04627e63..e278bba39 100644 --- a/src/libstrongswan/credentials/auth_cfg.c +++ b/src/libstrongswan/credentials/auth_cfg.c @@ -42,6 +42,7 @@ ENUM(auth_rule_names, AUTH_RULE_IDENTITY, AUTH_HELPER_AC_CERT, "RULE_EAP_VENDOR", "RULE_XAUTH_BACKEND", "RULE_XAUTH_IDENTITY", + "AUTH_RULE_CA_IDENTITY", "RULE_CA_CERT", "RULE_IM_CERT", "RULE_SUBJECT_CERT", @@ -88,6 +89,7 @@ static inline bool is_multi_value_rule(auth_rule_t type) case AUTH_RULE_CRL_VALIDATION: case AUTH_RULE_GROUP: case AUTH_RULE_SUBJECT_CERT: + case AUTH_RULE_CA_IDENTITY: case AUTH_RULE_CA_CERT: case AUTH_RULE_IM_CERT: case AUTH_RULE_CERT_POLICY: @@ -226,6 +228,7 @@ static void init_entry(entry_t *this, auth_rule_t type, va_list args) case AUTH_RULE_XAUTH_BACKEND: case AUTH_RULE_XAUTH_IDENTITY: case AUTH_RULE_GROUP: + case AUTH_RULE_CA_IDENTITY: case AUTH_RULE_CA_CERT: case AUTH_RULE_IM_CERT: case AUTH_RULE_SUBJECT_CERT: @@ -287,6 +290,7 @@ static bool entry_equals(entry_t *e1, entry_t *e2) return c1->equals(c1, c2); } case AUTH_RULE_IDENTITY: + case AUTH_RULE_CA_IDENTITY: case AUTH_RULE_EAP_IDENTITY: case AUTH_RULE_AAA_IDENTITY: case AUTH_RULE_XAUTH_IDENTITY: @@ -325,6 +329,7 @@ static void destroy_entry_value(entry_t *entry) switch (entry->type) { case AUTH_RULE_IDENTITY: + case AUTH_RULE_CA_IDENTITY: case AUTH_RULE_EAP_IDENTITY: case AUTH_RULE_AAA_IDENTITY: case AUTH_RULE_GROUP: @@ -406,6 +411,7 @@ static void replace(private_auth_cfg_t *this, entry_enumerator_t *enumerator, entry->value = (void*)(uintptr_t)va_arg(args, u_int); break; case AUTH_RULE_IDENTITY: + case AUTH_RULE_CA_IDENTITY: case AUTH_RULE_EAP_IDENTITY: case AUTH_RULE_AAA_IDENTITY: case AUTH_RULE_XAUTH_BACKEND: @@ -486,6 +492,7 @@ METHOD(auth_cfg_t, get, void*, case AUTH_RULE_CERT_VALIDATION_SUSPENDED: return (void*)FALSE; case AUTH_RULE_IDENTITY: + case AUTH_RULE_CA_IDENTITY: case AUTH_RULE_EAP_IDENTITY: case AUTH_RULE_AAA_IDENTITY: case AUTH_RULE_XAUTH_BACKEND: @@ -808,8 +815,8 @@ METHOD(auth_cfg_t, complies, bool, enumerator_t *e1, *e2; bool success = TRUE, group_match = FALSE; bool ca_match = FALSE, cert_match = FALSE; - identification_t *require_group = NULL; - certificate_t *require_ca = NULL, *require_cert = NULL; + identification_t *require_group = NULL, *require_ca = NULL; + certificate_t *require_cert = NULL; signature_params_t *ike_scheme = NULL, *scheme = NULL; u_int strength = 0; auth_rule_t t1, t2; @@ -824,16 +831,35 @@ METHOD(auth_cfg_t, complies, bool, case AUTH_RULE_CA_CERT: case AUTH_RULE_IM_CERT: { - certificate_t *cert; + certificate_t *cert, *ca; /* for CA certs, a match of a single cert is sufficient */ - require_ca = (certificate_t*)value; + ca = (certificate_t*)value; + require_ca = ca->get_subject(ca); e2 = create_enumerator(this); while (e2->enumerate(e2, &t2, &cert)) { if ((t2 == AUTH_RULE_CA_CERT || t2 == AUTH_RULE_IM_CERT) && - cert->equals(cert, require_ca)) + cert->equals(cert, ca)) + { + ca_match = TRUE; + } + } + e2->destroy(e2); + break; + } + case AUTH_RULE_CA_IDENTITY: + { + certificate_t *cert; + + require_ca = (identification_t*)value; + + e2 = create_enumerator(this); + while (e2->enumerate(e2, &t2, &cert)) + { + if ((t2 == AUTH_RULE_CA_CERT || t2 == AUTH_RULE_IM_CERT) && + cert->has_subject(cert, require_ca)) { ca_match = TRUE; } @@ -1138,8 +1164,7 @@ METHOD(auth_cfg_t, complies, bool, if (log_error) { DBG1(DBG_CFG, "constraint check failed: peer not " - "authenticated by CA '%Y'", - require_ca->get_subject(require_ca)); + "authenticated by CA '%Y'", require_ca); } return FALSE; } @@ -1205,6 +1230,7 @@ static void merge(private_auth_cfg_t *this, private_auth_cfg_t *other, bool copy break; } case AUTH_RULE_IDENTITY: + case AUTH_RULE_CA_IDENTITY: case AUTH_RULE_EAP_IDENTITY: case AUTH_RULE_AAA_IDENTITY: case AUTH_RULE_GROUP: @@ -1337,6 +1363,7 @@ METHOD(auth_cfg_t, clone_, auth_cfg_t*, switch (type) { case AUTH_RULE_IDENTITY: + case AUTH_RULE_CA_IDENTITY: case AUTH_RULE_EAP_IDENTITY: case AUTH_RULE_AAA_IDENTITY: case AUTH_RULE_GROUP: diff --git a/src/libstrongswan/credentials/auth_cfg.h b/src/libstrongswan/credentials/auth_cfg.h index 38c40c87d..4f36b19f2 100644 --- a/src/libstrongswan/credentials/auth_cfg.h +++ b/src/libstrongswan/credentials/auth_cfg.h @@ -84,6 +84,8 @@ enum auth_rule_t { AUTH_RULE_XAUTH_BACKEND, /** XAuth identity to use or require, identification_t* */ AUTH_RULE_XAUTH_IDENTITY, + /** subject of certificate authority, identification_t* */ + AUTH_RULE_CA_IDENTITY, /** certificate authority, certificate_t* */ AUTH_RULE_CA_CERT, /** intermediate certificate in trustchain, certificate_t* */ From 3c71a3201f5602c86fd7bb0adfb8f06f59b0830d Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 28 Nov 2019 10:09:30 +0100 Subject: [PATCH 2/6] vici: Introduce a ca_id option identity based CA certificate constraints --- src/libcharon/plugins/vici/vici_config.c | 13 +++++++++++++ src/libcharon/plugins/vici/vici_query.c | 3 +++ 2 files changed, 16 insertions(+) diff --git a/src/libcharon/plugins/vici/vici_config.c b/src/libcharon/plugins/vici/vici_config.c index 49ebea44b..1bbad139e 100644 --- a/src/libcharon/plugins/vici/vici_config.c +++ b/src/libcharon/plugins/vici/vici_config.c @@ -373,6 +373,9 @@ static void log_auth(auth_cfg_t *auth) case AUTH_RULE_IDENTITY: DBG2(DBG_CFG, " id = %Y", v.id); break; + case AUTH_RULE_CA_IDENTITY: + DBG2(DBG_CFG, " ca_id = %Y", v.id); + break; case AUTH_RULE_AAA_IDENTITY: DBG2(DBG_CFG, " aaa_id = %Y", v.id); break; @@ -1360,6 +1363,15 @@ CALLBACK(parse_ike_id, bool, return parse_id(cfg, AUTH_RULE_IDENTITY, v); } +/** + * Parse CA identity constraint + */ +CALLBACK(parse_ca_id, bool, + auth_cfg_t *cfg, chunk_t v) +{ + return parse_id(cfg, AUTH_RULE_CA_IDENTITY, v); +} + /** * Parse AAA identity */ @@ -1755,6 +1767,7 @@ CALLBACK(auth_kv, bool, parse_rule_t rules[] = { { "auth", parse_auth, auth->cfg }, { "id", parse_ike_id, auth->cfg }, + { "ca_id", parse_ca_id, auth->cfg }, { "aaa_id", parse_aaa_id, auth->cfg }, { "eap_id", parse_eap_id, auth->cfg }, { "xauth_id", parse_xauth_id, auth->cfg }, diff --git a/src/libcharon/plugins/vici/vici_query.c b/src/libcharon/plugins/vici/vici_query.c index 81d692c84..ad07ff12d 100644 --- a/src/libcharon/plugins/vici/vici_query.c +++ b/src/libcharon/plugins/vici/vici_query.c @@ -765,6 +765,9 @@ static void build_auth_cfgs(peer_cfg_t *peer_cfg, bool local, vici_builder_t *b) case AUTH_RULE_IDENTITY: b->add_kv(b, "id", "%Y", v.id); break; + case AUTH_RULE_CA_IDENTITY: + b->add_kv(b, "ca_id", "%Y", v.id); + break; case AUTH_RULE_AAA_IDENTITY: b->add_kv(b, "aaa_id", "%Y", v.id); break; From 55fc514ed20c204a0e0f34c1068beda5dba856e4 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 28 Nov 2019 10:20:50 +0100 Subject: [PATCH 3/6] swanctl: Document the remote ca_id option for identity based CA constraints --- src/swanctl/swanctl.opt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/swanctl/swanctl.opt b/src/swanctl/swanctl.opt index f7801b3c3..0ae9d4579 100644 --- a/src/swanctl/swanctl.opt +++ b/src/swanctl/swanctl.opt @@ -593,6 +593,16 @@ connections..remote.cacert.slot = connections..remote.cacert.module = Optional PKCS#11 module name. +connections..remote.ca_id = + Identity in CA certificate to accept for authentication. + + The specified identity must be contained in one (intermediate) CA + of the remote peer trustchain, either as subject or as subjectAltName. + This has the same effect as specifying _cacerts_ to force clients under + a CA to specific connections; it does not require the CA certificate to + be available locally, and can be received from the peer during the + IKE exchange. + connections..remote.pubkeys = Comma separated list of raw public keys to accept for authentication. From 026024bc02d82ee953a72eef40986d12d0cf0a87 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 28 Nov 2019 17:04:16 +0100 Subject: [PATCH 4/6] swanctl: Include ca_id property in list-conns command --- src/swanctl/commands/list_conns.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/swanctl/commands/list_conns.c b/src/swanctl/commands/list_conns.c index 5f7dd8189..f00fb54fd 100644 --- a/src/swanctl/commands/list_conns.c +++ b/src/swanctl/commands/list_conns.c @@ -183,6 +183,10 @@ CALLBACK(conn_sn, int, { printf(" id: %s\n", auth->get(auth, "id")); } + if (auth->get(auth, "ca_id")) + { + printf(" ca_id: %s\n", auth->get(auth, "ca_id")); + } if (auth->get(auth, "eap_id")) { printf(" eap_id: %s\n", auth->get(auth, "eap_id")); From f95d5122513ab34cd1b3cffc7bb49d69932cd51c Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 28 Nov 2019 10:25:20 +0100 Subject: [PATCH 5/6] testing: Use identity based CA restrictions in rw-hash-and-url-multi-level This is a prominent example where the identity based CA constraint is benefical. While the description of the test claims a strict binding of the client to the intermediate CA, this is not fully true if CA operators are not fully trusted: A rogue OU=Sales intermediate may issue certificates containing a OU=Research. By binding the connection to the CA, we can avoid this, and using the identity based constraint still allows moon to receive the intermediate over IKE or hash-and-url. --- .../hosts/moon/etc/swanctl/swanctl.conf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/tests/swanctl/rw-hash-and-url-multi-level/hosts/moon/etc/swanctl/swanctl.conf b/testing/tests/swanctl/rw-hash-and-url-multi-level/hosts/moon/etc/swanctl/swanctl.conf index 334ecb62f..a690cf2b1 100755 --- a/testing/tests/swanctl/rw-hash-and-url-multi-level/hosts/moon/etc/swanctl/swanctl.conf +++ b/testing/tests/swanctl/rw-hash-and-url-multi-level/hosts/moon/etc/swanctl/swanctl.conf @@ -10,7 +10,7 @@ connections { } remote { auth = pubkey - id = "C=CH, O=strongSwan Project, OU=Research, CN=*" + ca_id = "C=CH, O=strongSwan Project, OU=Research, CN=Research CA" } children { alice { @@ -32,7 +32,7 @@ connections { } remote { auth = pubkey - id = "C=CH, O=strongSwan Project, OU=Sales, CN=*" + ca_id = "C=CH, O=strongSwan Project, OU=Sales, CN=Sales CA" } children { venus { From ae9b748a77c3e5267f80bb320acdcd76940403bb Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 29 Nov 2019 12:16:43 +0100 Subject: [PATCH 6/6] vici: Log certificate constraints for loaded configs --- src/libcharon/plugins/vici/vici_config.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libcharon/plugins/vici/vici_config.c b/src/libcharon/plugins/vici/vici_config.c index 1bbad139e..eb679290d 100644 --- a/src/libcharon/plugins/vici/vici_config.c +++ b/src/libcharon/plugins/vici/vici_config.c @@ -347,6 +347,7 @@ static void log_auth(auth_cfg_t *auth) union { uintptr_t u; identification_t *id; + certificate_t *cert; char *str; } v; @@ -388,6 +389,12 @@ static void log_auth(auth_cfg_t *auth) case AUTH_RULE_GROUP: DBG2(DBG_CFG, " group = %Y", v.id); break; + case AUTH_RULE_SUBJECT_CERT: + DBG2(DBG_CFG, " cert = %Y", v.cert->get_subject(v.cert)); + break; + case AUTH_RULE_CA_CERT: + DBG2(DBG_CFG, " cacert = %Y", v.cert->get_subject(v.cert)); + break; default: break; }