From 67c55d5869d5473e5139614637f31ea37746181d Mon Sep 17 00:00:00 2001 From: Dimitri Staessens Date: Thu, 11 Jun 2026 10:03:14 +0000 Subject: irmd: Specify peer authentication contract OAP accepted requests and responses without a certificate even when the peer was expected to authenticate. An on-path attacker could strip the certificate and signature from a flow allocation response and substitute its own key exchange, silently downgrading the handshake to unauthenticated. Add an auth=required|optional policy to enc.conf, enforced per role: a client config requires the server to present a valid certificate, a server config requires the same from the client. Default is required for client side (https), optional server side. The client side default can be changed via OAP_CLIENT_AUTH_DEFAULT for testing. Replace the bare 'none' keyword with encryption=none, which disables encryption only: the digest and the authentication policy are kept, so authenticated but unencrypted flows can be configured. Configs using bare 'none' are now rejected. Signed-off-by: Dimitri Staessens Signed-off-by: Sander Vrijders --- src/irmd/config.h.in | 1 + src/irmd/oap/cli.c | 11 ++ src/irmd/oap/io.c | 2 - src/irmd/oap/srv.c | 8 ++ src/irmd/oap/tests/common.c | 4 + src/irmd/oap/tests/common.h | 1 + src/irmd/oap/tests/oap_test.c | 130 ++++++++++++++++++++ src/lib/crypt.c | 31 ++++- src/lib/tests/kex_test.c | 273 +++++++++++++++++++++++++++++++++++++++++- 9 files changed, 449 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/irmd/config.h.in b/src/irmd/config.h.in index df0cd718..0364e080 100644 --- a/src/irmd/config.h.in +++ b/src/irmd/config.h.in @@ -42,6 +42,7 @@ #define FLOW_DEALLOC_TIMEOUT @FLOW_DEALLOC_TIMEOUT@ #define OAP_REPLAY_TIMER @OAP_REPLAY_TIMER@ +#cmakedefine01 OAP_CLIENT_AUTH_DEFAULT #define BOOTSTRAP_TIMEOUT @BOOTSTRAP_TIMEOUT@ #define ENROLL_TIMEOUT @ENROLL_TIMEOUT@ diff --git a/src/irmd/oap/cli.c b/src/irmd/oap/cli.c index 7a202da7..d38f38dd 100644 --- a/src/irmd/oap/cli.c +++ b/src/irmd/oap/cli.c @@ -93,6 +93,11 @@ int load_cli_kex_config(const struct name_info * info, assert(info != NULL); assert(cfg != NULL); + memset(cfg, 0, sizeof(*cfg)); + + /* A client authenticates the server by default, like an https client */ + cfg->req_auth = OAP_CLIENT_AUTH_DEFAULT; + return load_kex_config(info->name, info->c.enc, cfg); } @@ -534,6 +539,12 @@ int oap_cli_complete(void * ctx, goto fail_oap; } + /* Required peer auth makes sig and name binding mandatory */ + if (s->kcfg.req_auth && peer_hdr.crt.len == 0) { + log_err_id(id, "Server did not provide a certificate."); + goto fail_oap; + } + /* Verify request hash in authenticated response */ if (peer_hdr.req_hash.len == 0) { log_err_id(id, "Response missing req_hash."); diff --git a/src/irmd/oap/io.c b/src/irmd/oap/io.c index c2c91b91..24d33e60 100644 --- a/src/irmd/oap/io.c +++ b/src/irmd/oap/io.c @@ -103,8 +103,6 @@ int load_kex_config(const char * name, assert(name != NULL); assert(cfg != NULL); - memset(cfg, 0, sizeof(*cfg)); - /* Load encryption config */ if (!file_exists(path)) log_dbg("No encryption %s for %s.", path, name); diff --git a/src/irmd/oap/srv.c b/src/irmd/oap/srv.c index 587a8f9f..08b4d9d2 100644 --- a/src/irmd/oap/srv.c +++ b/src/irmd/oap/srv.c @@ -73,6 +73,9 @@ int load_srv_kex_config(const struct name_info * info, assert(info != NULL); assert(cfg != NULL); + memset(cfg, 0, sizeof(*cfg)); + + /* Client auth stays opt-in (mTLS); enable with auth=required */ return load_kex_config(info->name, info->s.enc, cfg); } @@ -441,6 +444,11 @@ int oap_srv_process(const struct name_info * info, goto fail_auth; } + if (kcfg.req_auth && peer_hdr.crt.len == 0) { + log_err_id(id, "Client did not provide a certificate."); + goto fail_auth; + } + if (do_server_kex(info, &peer_hdr, &kcfg, &local_hdr.kex, sk) < 0) goto fail_kex; diff --git a/src/irmd/oap/tests/common.c b/src/irmd/oap/tests/common.c index 0a1af100..c5000e48 100644 --- a/src/irmd/oap/tests/common.c +++ b/src/irmd/oap/tests/common.c @@ -36,6 +36,8 @@ int load_srv_kex_config(const struct name_info * info, memset(cfg, 0, sizeof(*cfg)); + cfg->req_auth = test_cfg.srv.req_auth; + if (test_cfg.srv.kex == NID_undef) return 0; @@ -55,6 +57,8 @@ int load_cli_kex_config(const struct name_info * info, memset(cfg, 0, sizeof(*cfg)); + cfg->req_auth = test_cfg.cli.req_auth; + if (test_cfg.cli.kex == NID_undef) return 0; diff --git a/src/irmd/oap/tests/common.h b/src/irmd/oap/tests/common.h index d4b6733a..fa500ffe 100644 --- a/src/irmd/oap/tests/common.h +++ b/src/irmd/oap/tests/common.h @@ -38,6 +38,7 @@ struct test_sec_cfg { int md; /* Digest NID for signatures */ int kem_mode; /* KEM encapsulation mode (0 for ECDH) */ bool auth; /* Use authentication (certificates) */ + bool req_auth; /* Require peer authentication */ }; /* Test configuration - set by each test before running roundtrip */ diff --git a/src/irmd/oap/tests/oap_test.c b/src/irmd/oap/tests/oap_test.c index a525d988..fd2c5629 100644 --- a/src/irmd/oap/tests/oap_test.c +++ b/src/irmd/oap/tests/oap_test.c @@ -1183,6 +1183,129 @@ static int test_oap_server_name_mismatch(void) return TEST_RC_FAIL; } +/* Client requiring auth rejects a response without certificate */ +static int test_oap_cli_requires_srv_auth(void) +{ + struct oap_test_ctx ctx; + + test_default_cfg(); + test_cfg.srv.auth = NO_AUTH; + test_cfg.cli.req_auth = true; + + TEST_START(); + + if (oap_test_setup(&ctx, root_ca_crt_ec, im_ca_crt_ec) < 0) + goto fail; + + if (oap_cli_prepare_ctx(&ctx) < 0) { + printf("Client prepare failed.\n"); + goto fail_cleanup; + } + + if (oap_srv_process_ctx(&ctx) < 0) { + printf("Server process failed.\n"); + goto fail_cleanup; + } + + if (oap_cli_complete_ctx(&ctx) == 0) { + printf("Client should reject unauthenticated server.\n"); + goto fail_cleanup; + } + + oap_test_teardown(&ctx); + + TEST_SUCCESS(); + return TEST_RC_SUCCESS; + + fail_cleanup: + oap_test_teardown(&ctx); + fail: + TEST_FAIL(); + return TEST_RC_FAIL; +} + +/* Server requiring auth rejects a request without certificate */ +static int test_oap_srv_requires_cli_auth(void) +{ + struct oap_test_ctx ctx; + + test_default_cfg(); + test_cfg.srv.req_auth = true; + + TEST_START(); + + if (oap_test_setup(&ctx, root_ca_crt_ec, im_ca_crt_ec) < 0) + goto fail; + + if (oap_cli_prepare_ctx(&ctx) < 0) { + printf("Client prepare failed.\n"); + goto fail_cleanup; + } + + if (oap_srv_process_ctx(&ctx) == 0) { + printf("Server should reject unauthenticated client.\n"); + goto fail_cleanup; + } + + oap_test_teardown(&ctx); + + TEST_SUCCESS(); + return TEST_RC_SUCCESS; + + fail_cleanup: + oap_test_teardown(&ctx); + fail: + TEST_FAIL(); + return TEST_RC_FAIL; +} + +/* Roundtrip succeeds when both sides require and provide auth */ +static int test_oap_mutual_req_auth(void) +{ + struct oap_test_ctx ctx; + + test_default_cfg(); + test_cfg.srv.req_auth = true; + test_cfg.cli.auth = AUTH; + test_cfg.cli.req_auth = true; + + TEST_START(); + + if (oap_test_setup(&ctx, root_ca_crt_ec, im_ca_crt_ec) < 0) + goto fail; + + if (oap_cli_prepare_ctx(&ctx) < 0) { + printf("Client prepare failed.\n"); + goto fail_cleanup; + } + + if (oap_srv_process_ctx(&ctx) < 0) { + printf("Server process failed.\n"); + goto fail_cleanup; + } + + if (oap_cli_complete_ctx(&ctx) < 0) { + printf("Client complete failed.\n"); + goto fail_cleanup; + } + + if (memcmp(ctx.cli.key, ctx.srv.key, SYMMKEYSZ) != 0) { + printf("Client and server keys do not match!\n"); + goto fail_cleanup; + } + + oap_test_teardown(&ctx); + + TEST_SUCCESS(); + return TEST_RC_SUCCESS; + + fail_cleanup: + oap_test_teardown(&ctx); + fail: + TEST_FAIL(); + return TEST_RC_FAIL; +} + int oap_test(int argc, char **argv) { @@ -1220,6 +1343,10 @@ int oap_test(int argc, ret |= test_oap_replay_packet(); ret |= test_oap_missing_root_ca(); ret |= test_oap_server_name_mismatch(); + + ret |= test_oap_cli_requires_srv_auth(); + ret |= test_oap_srv_requires_cli_auth(); + ret |= test_oap_mutual_req_auth(); #else (void) test_oap_roundtrip_auth_only; (void) test_oap_roundtrip_kex_only; @@ -1245,6 +1372,9 @@ int oap_test(int argc, (void) test_oap_replay_packet; (void) test_oap_missing_root_ca; (void) test_oap_server_name_mismatch; + (void) test_oap_cli_requires_srv_auth; + (void) test_oap_srv_requires_cli_auth; + (void) test_oap_mutual_req_auth; ret = TEST_RC_SKIP; #endif diff --git a/src/lib/crypt.c b/src/lib/crypt.c index 71197f6e..66d07131 100644 --- a/src/lib/crypt.c +++ b/src/lib/crypt.c @@ -162,6 +162,7 @@ int parse_sec_config(struct sec_config * cfg, char * equals; char * key; char * value; + bool no_enc = false; assert(cfg != NULL); assert(fp != NULL); @@ -172,6 +173,7 @@ int parse_sec_config(struct sec_config * cfg, SET_KEX_KDF_NID(cfg, NID_sha256); SET_KEX_CIPHER_NID(cfg, NID_aes_256_gcm); SET_KEX_DIGEST_NID(cfg, NID_sha256); + /* req_auth is seeded per-role by the caller; only auth= overrides it */ while (fgets(line, sizeof(line), fp) != NULL) { char * trimmed; @@ -180,12 +182,10 @@ int parse_sec_config(struct sec_config * cfg, if (line[0] == '#' || line[0] == '\n') continue; - /* Check for 'none' keyword */ + /* Bare 'none' keyword replaced by encryption=none */ trimmed = trim_whitespace(line); - if (strcmp(trimmed, "none") == 0) { - memset(cfg, 0, sizeof(*cfg)); - return 0; - } + if (strcmp(trimmed, "none") == 0) + return -EINVAL; /* Find the = separator */ equals = strchr(line, '='); @@ -221,9 +221,30 @@ int parse_sec_config(struct sec_config * cfg, } else { return -EINVAL; } + } else if (strcmp(key, "auth") == 0) { + if (strcmp(value, "required") == 0) { + cfg->req_auth = true; + } else if (strcmp(value, "optional") == 0) { + cfg->req_auth = false; + } else { + return -EINVAL; + } + } else if (strcmp(key, "encryption") == 0) { + if (strcmp(value, "none") != 0) + return -EINVAL; + no_enc = true; + } else { + return -EINVAL; } } + if (no_enc) { + /* Digest stays: it belongs to the auth axis */ + CLEAR_KEX_ALGO(cfg); + CLEAR_KEX_KDF(cfg); + CLEAR_KEX_CIPHER(cfg); + } + return 0; } diff --git a/src/lib/tests/kex_test.c b/src/lib/tests/kex_test.c index 6a4f802e..300a0607 100644 --- a/src/lib/tests/kex_test.c +++ b/src/lib/tests/kex_test.c @@ -44,6 +44,9 @@ #define KEX_CONFIG_NONE \ "none\n" +#define KEX_CONFIG_NO_ENC \ + "encryption=none\n" + #define KEX_CONFIG_WHITESPACE \ "# Comment line\n" \ "kex = X448" \ @@ -58,6 +61,25 @@ "kex=X25519\n" \ "digest=sha384\n" +#define KEX_CONFIG_AUTH \ + "auth=required\n" + +#define KEX_CONFIG_AUTH_INVALID \ + "auth=mandatory\n" + +#define KEX_CONFIG_AUTH_OPTIONAL \ + "auth=optional\n" + +#define KEX_CONFIG_AUTH_THEN_NO_ENC \ + "auth=required\n" \ + "digest=sha512\n" \ + "encryption=none\n" + +#define KEX_CONFIG_NO_ENC_THEN_AUTH \ + "encryption=none\n" \ + "auth=required\n" \ + "digest=sha512\n" + /* Test key material for key loading tests */ #define X25519_PRIVKEY_PEM \ "-----BEGIN PRIVATE KEY-----\n" \ @@ -639,7 +661,8 @@ static int test_kex_parse_config_custom(void) return TEST_RC_FAIL; } -static int test_kex_parse_config_none(void) +/* The old bare 'none' keyword must be rejected loudly */ +static int test_kex_parse_config_none_rejected(void) { struct sec_config kex; FILE * fp; @@ -654,14 +677,51 @@ static int test_kex_parse_config_none(void) goto fail; } + if (parse_sec_config(&kex, fp) == 0) { + printf("Bare 'none' keyword should be rejected.\n"); + fclose(fp); + goto fail; + } + + fclose(fp); + + TEST_SUCCESS(); + + return TEST_RC_SUCCESS; + fail: + TEST_FAIL(); + return TEST_RC_FAIL; +} + +static int test_kex_parse_config_no_enc(void) +{ + struct sec_config kex; + FILE * fp; + + TEST_START(); + + memset(&kex, 0, sizeof(kex)); + + fp = FMEMOPEN_STR(KEX_CONFIG_NO_ENC); + if (fp == NULL) { + printf("Failed to open memory stream.\n"); + goto fail; + } + if (parse_sec_config(&kex, fp) < 0) { - printf("Failed to parse 'none' config.\n"); + printf("Failed to parse encryption=none config.\n"); fclose(fp); goto fail; } - if (kex.x.nid != NID_undef) { - printf("'none' keyword should disable encryption.\n"); + if (kex.x.nid != NID_undef || kex.c.nid != NID_undef) { + printf("encryption=none should disable encryption.\n"); + fclose(fp); + goto fail; + } + + if (kex.d.nid != NID_sha256) { + printf("encryption=none should keep the digest.\n"); fclose(fp); goto fail; } @@ -799,6 +859,202 @@ static int test_kex_parse_config_digest(void) return TEST_RC_FAIL; } +static int test_kex_parse_config_auth(void) +{ + struct sec_config kex; + FILE * fp; + + TEST_START(); + + memset(&kex, 0, sizeof(kex)); + + fp = FMEMOPEN_STR(KEX_CONFIG_AUTH); + if (fp == NULL) { + printf("Failed to open memory stream.\n"); + goto fail; + } + + if (parse_sec_config(&kex, fp) < 0) { + printf("Failed to parse auth config.\n"); + fclose(fp); + goto fail; + } + + if (!kex.req_auth) { + printf("auth=required not parsed correctly.\n"); + fclose(fp); + goto fail; + } + + fclose(fp); + + TEST_SUCCESS(); + + return TEST_RC_SUCCESS; + fail: + TEST_FAIL(); + return TEST_RC_FAIL; +} + +static int test_kex_parse_config_auth_invalid(void) +{ + struct sec_config kex; + FILE * fp; + + TEST_START(); + + memset(&kex, 0, sizeof(kex)); + + fp = FMEMOPEN_STR(KEX_CONFIG_AUTH_INVALID); + if (fp == NULL) { + printf("Failed to open memory stream.\n"); + goto fail; + } + + if (parse_sec_config(&kex, fp) == 0) { + printf("Invalid auth value should be rejected.\n"); + fclose(fp); + goto fail; + } + + fclose(fp); + + TEST_SUCCESS(); + + return TEST_RC_SUCCESS; + fail: + TEST_FAIL(); + return TEST_RC_FAIL; +} + +/* A caller-seeded req_auth survives parsing when no auth= line is set */ +static int test_kex_parse_config_auth_seed(void) +{ + struct sec_config kex; + FILE * fp; + + TEST_START(); + + memset(&kex, 0, sizeof(kex)); + kex.req_auth = true; + + fp = FMEMOPEN_STR(KEX_CONFIG_NO_ENC); + if (fp == NULL) { + printf("Failed to open memory stream.\n"); + goto fail; + } + + if (parse_sec_config(&kex, fp) < 0) { + printf("Failed to parse config.\n"); + fclose(fp); + goto fail; + } + + if (!kex.req_auth) { + printf("Seeded req_auth should survive parsing.\n"); + fclose(fp); + goto fail; + } + + fclose(fp); + + TEST_SUCCESS(); + + return TEST_RC_SUCCESS; + fail: + TEST_FAIL(); + return TEST_RC_FAIL; +} + +/* An explicit auth=optional clears a caller-seeded req_auth */ +static int test_kex_parse_config_auth_optional(void) +{ + struct sec_config kex; + FILE * fp; + + TEST_START(); + + memset(&kex, 0, sizeof(kex)); + kex.req_auth = true; + + fp = FMEMOPEN_STR(KEX_CONFIG_AUTH_OPTIONAL); + if (fp == NULL) { + printf("Failed to open memory stream.\n"); + goto fail; + } + + if (parse_sec_config(&kex, fp) < 0) { + printf("Failed to parse auth=optional config.\n"); + fclose(fp); + goto fail; + } + + if (kex.req_auth) { + printf("auth=optional should clear req_auth.\n"); + fclose(fp); + goto fail; + } + + fclose(fp); + + TEST_SUCCESS(); + + return TEST_RC_SUCCESS; + fail: + TEST_FAIL(); + return TEST_RC_FAIL; +} + +/* encryption=none must not drop auth=required or the digest */ +static int test_kex_parse_config_auth_no_enc(const char * config) +{ + struct sec_config kex; + FILE * fp; + + TEST_START(); + + memset(&kex, 0, sizeof(kex)); + + fp = FMEMOPEN_STR(config); + if (fp == NULL) { + printf("Failed to open memory stream.\n"); + goto fail; + } + + if (parse_sec_config(&kex, fp) < 0) { + printf("Failed to parse auth + encryption=none.\n"); + fclose(fp); + goto fail; + } + + if (!kex.req_auth) { + printf("encryption=none should not drop required auth.\n"); + fclose(fp); + goto fail; + } + + if (kex.x.nid != NID_undef) { + printf("encryption=none should disable encryption.\n"); + fclose(fp); + goto fail; + } + + if (kex.d.nid != NID_sha512) { + printf("encryption=none should keep the digest.\n"); + fclose(fp); + goto fail; + } + + fclose(fp); + + TEST_SUCCESS(); + + return TEST_RC_SUCCESS; + fail: + TEST_FAIL(); + return TEST_RC_FAIL; +} + int kex_test(int argc, char ** argv) { @@ -809,7 +1065,14 @@ int kex_test(int argc, ret |= test_kex_create_destroy(); ret |= test_kex_parse_config_empty(); - ret |= test_kex_parse_config_none(); + ret |= test_kex_parse_config_none_rejected(); + ret |= test_kex_parse_config_no_enc(); + ret |= test_kex_parse_config_auth(); + ret |= test_kex_parse_config_auth_invalid(); + ret |= test_kex_parse_config_auth_seed(); + ret |= test_kex_parse_config_auth_optional(); + ret |= test_kex_parse_config_auth_no_enc(KEX_CONFIG_AUTH_THEN_NO_ENC); + ret |= test_kex_parse_config_auth_no_enc(KEX_CONFIG_NO_ENC_THEN_AUTH); #ifdef HAVE_OPENSSL ret |= test_kex_parse_config_custom(); ret |= test_kex_parse_config_whitespace(); -- cgit v1.2.3