From 643f33fc603cb4de14cd786deb06087d9c621d2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 9 Sep 2026 11:46:23 +0200 Subject: [PATCH 01/18] Reject an EST listener configured without a TLS identity tls_setup() treated a missing tls_cert_pem or tls_key_pem as a successful plaintext configuration for every protocol, and the accept loop dispatched straight to the handler whenever no TLS context was present. An EST server started without TLS material therefore served /cacerts, /csrattrs, /simpleenroll and /simplereenroll over cleartext HTTP, putting the HTTP Basic credentials and the CSR on the wire in the clear. RFC 7030 has no plaintext mode, and the client side already refused an http:// EST URL, so the server now refuses the same configuration with WOLFCERT_ERR_TLS before it binds. SCEP authenticates at the pkiMessage layer and keeps its plaintext transport. The two integration tests that scripted byte-exact HTTP at a plain socket now drive the server through a small raw TLS client added to tls_test_util.h, pinning the identity the test mints. One write is one TLS record and so one read on the server, which preserves the segment boundaries the chunked framing cases depend on. wolfcert-server rejects --proto est without --tls-cert and --tls-key up front, and the quick start in README.md and CLAUDE.md is updated to match. Fixes F-8031. --- CLAUDE.md | 5 +- README.md | 7 +- cli/wolfcert_server.c | 12 ++- src/server.c | 11 ++- .../integration/test_est_chunked_robustness.c | 92 +++++++++---------- .../integration/test_est_csr_attrs_enforce.c | 57 +++++------- tests/integration/test_est_tls_roundtrip.c | 24 +++++ tests/integration/tls_test_util.h | 92 +++++++++++++++++++ wolfcert/server.h | 7 +- 9 files changed, 216 insertions(+), 91 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 30e08e8..34f7a0f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -96,9 +96,10 @@ Unit tests live in `tests/unit/`; end-to-end flows in After a build with `-DWOLFCERT_ENABLE_CLI=ON` (the default): ```sh -build/wolfcert-server --proto est --listen 127.0.0.1:8443 +build/wolfcert-server --proto est --listen 127.0.0.1:8443 \ + --tls-cert server.crt --tls-key server.key build/wolfcert-client enroll --proto est \ - --url http://127.0.0.1:8443/.well-known/est \ + --url https://127.0.0.1:8443/.well-known/est --trust server.crt \ --key-type ecc:256 --subject "CN=dev" \ --out-key dev.key --out-cert dev.crt ``` diff --git a/README.md b/README.md index 1c89f24..91e1c46 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,10 @@ on the include path. See [`docs/EMBEDDED.md`](docs/EMBEDDED.md#configuring-wolfc Start the bundled test server (issues from an auto-generated local CA): ```sh -./wolfcert-server --proto est --listen 127.0.0.1:8443 +# EST is TLS-only (RFC 7030), so it needs a server identity for the +# listen address; SCEP authenticates at the pkiMessage layer instead. +./wolfcert-server --proto est --listen 127.0.0.1:8443 \ + --tls-cert server.crt --tls-key server.key ./wolfcert-server --proto scep --listen 127.0.0.1:8088 ``` @@ -94,7 +97,7 @@ Enroll a certificate from the CLI: ```sh # EST ./wolfcert-client enroll --proto est \ - --url http://127.0.0.1:8443/.well-known/est \ + --url https://127.0.0.1:8443/.well-known/est --trust server.crt \ --key-type ecc:256 --subject "CN=device-1,O=Acme" \ --san-dns device-1.local --out-key dev.key --out-cert dev.crt diff --git a/cli/wolfcert_server.c b/cli/wolfcert_server.c index 413b82e..6d5bce0 100644 --- a/cli/wolfcert_server.c +++ b/cli/wolfcert_server.c @@ -19,7 +19,8 @@ /* * wolfcert-server - minimal EST/SCEP test server. Issues certificates - * against a local CA generated on startup. Plaintext HTTP only. + * against a local CA generated on startup. EST needs --tls-cert/--tls-key + * (RFC 7030); SCEP may be served over plaintext HTTP. */ #define _POSIX_C_SOURCE 200809L @@ -67,7 +68,8 @@ static void print_usage(FILE* out) " --listen HOST:PORT Bind address (default 0.0.0.0:8080)\n" " --basic USER:PASS Require HTTP Basic auth (EST enroll)\n" " --challenge PASS Require this SCEP challengePassword in the CSR\n" - " --tls-cert PEMFILE Terminate TLS with this server certificate (PEM)\n" + " --tls-cert PEMFILE Terminate TLS with this server certificate (PEM);\n" + " required for --proto est (RFC 7030)\n" " --tls-key PEMFILE Private key for --tls-cert (PEM)\n" " --tls-client-ca PEMFILE Require mutual TLS; verify clients against this CA\n" " --scep-require-approval Defer SCEP PKCSReq/RenewalReq (pkiStatus=PENDING); issue\n" @@ -293,6 +295,12 @@ int main(int argc, char** argv) return 1; } + if (sel == WOLFCERT_PROTO_EST && tls_cert == NULL) { + fprintf(stderr, "wolfcert-server: --proto est requires --tls-cert and " + "--tls-key (RFC 7030 has no plaintext mode)\n"); + return 1; + } + if (host == NULL) host = strdup("0.0.0.0"); diff --git a/src/server.c b/src/server.c index 96cf744..3ebc526 100644 --- a/src/server.c +++ b/src/server.c @@ -73,10 +73,8 @@ static int tls_setup(WolfCertServer* s, const WolfCertServerCfgSrv* cfg) { int rc = WOLFCERT_OK; - if (cfg->tls_cert_pem == NULL || cfg->tls_key_pem == NULL) { - /* plaintext: nothing to do */ + if (cfg->tls_cert_pem == NULL || cfg->tls_key_pem == NULL) return WOLFCERT_OK; - } if (cfg->tls_cert_pem_len == 0 || cfg->tls_key_pem_len == 0) { return WOLFCERT_ERR_BAD_ARG; @@ -177,6 +175,13 @@ int wolfcert_server_start(const WolfCertServerCfgSrv* cfg, WolfCertServer** out) if (ops == NULL) return WOLFCERT_ERR_UNSUPPORTED; + /* RFC 7030 has no plaintext mode. Reject here rather than in tls_setup(), + * which runs after the CA has been generated and written to the store. */ + if (cfg->protocol == WOLFCERT_PROTO_EST && + (cfg->tls_cert_pem == NULL || cfg->tls_key_pem == NULL)) + return WOLFCERT_ERR(WOLFCERT_ERR_TLS, "server", + "EST requires TLS: set tls_cert_pem and tls_key_pem (RFC 7030)"); + void* heap = cfg->heap ? cfg->heap : wolfcert_default_heap(); WolfCertServer* s = (WolfCertServer*)WOLFCERT_XMALLOC(sizeof(*s), heap); if (s == NULL) diff --git a/tests/integration/test_est_chunked_robustness.c b/tests/integration/test_est_chunked_robustness.c index 7c2702d..d9ef56b 100644 --- a/tests/integration/test_est_chunked_robustness.c +++ b/tests/integration/test_est_chunked_robustness.c @@ -34,8 +34,10 @@ * 5. Keep-alive correctness when the last-chunk trailer CRLF arrives * in its own segment, so a following request is not corrupted. * - * The target is `src/est/est_server.c`'s parse_request chunked path; - * no TLS is involved so we can script the byte-exact request here. + * The target is `src/est/est_server.c`'s parse_request chunked path. + * EST mandates TLS (RFC 7030), so the byte-exact requests are scripted + * through a raw TLS client that pins the server's minted identity; one + * test_tls_write() is one TLS record, hence one read on the server. */ #define _POSIX_C_SOURCE 200809L @@ -69,42 +71,35 @@ } \ } while (0) +/* The server's minted TLS identity, pinned by every client below. */ +static uint8_t* g_tls_cert = NULL; +static size_t g_tls_cert_len = 0; + static void* server_thread(void* arg) { wolfcert_server_run((WolfCertServer*)arg); return NULL; } -/* Dial 127.0.0.1:port, send `req` of `req_len` bytes, return the +/* Dial 127.0.0.1:port over TLS, send `req` of `req_len` bytes, return the * first line of the response (up to the CRLF or buffer cap). */ static int send_and_read_status(uint16_t port, const void* req, size_t req_len, char* status_line, size_t cap) { - int cs = socket(AF_INET, SOCK_STREAM, 0); - if (cs < 0) + TestTlsConn c; + size_t n = 0; + + if (test_tls_connect(&c, port, g_tls_cert, g_tls_cert_len) != 0) return -1; - struct sockaddr_in sa = { .sin_family = AF_INET, - .sin_port = htons(port), - .sin_addr.s_addr = htonl(INADDR_LOOPBACK) }; - if (connect(cs, (struct sockaddr*)&sa, sizeof(sa)) < 0) { - close(cs); + + if (test_tls_write(&c, req, req_len) != 0) { + test_tls_close(&c); return -1; } - const char* p = req; - size_t left = req_len; - while (left > 0) { - ssize_t w = send(cs, p, left, 0); - if (w <= 0) { - close(cs); - return -1; - } - p += w; - left -= (size_t)w; - } - size_t n = 0; + while (n + 1 < cap) { - ssize_t r = recv(cs, status_line + n, cap - 1 - n, 0); + int r = test_tls_read(&c, status_line + n, cap - 1 - n); if (r <= 0) break; n += (size_t)r; @@ -113,7 +108,7 @@ static int send_and_read_status(uint16_t port, if (memchr(status_line, '\n', n) != NULL) break; } - close(cs); + test_tls_close(&c); status_line[n < cap ? n : cap - 1] = '\0'; return (int)n; } @@ -218,24 +213,20 @@ static int accept_multisegment_chunked_body(uint16_t port) "\r\n"; const char* seg2 = "10\r\nAAAA"; /* size line + 4/16 bytes */ const char* seg3 = "AAAAAAAAAAAA\r\n0\r\n\r\n"; /* last 12 bytes + terminator */ - struct sockaddr_in sa = { .sin_family = AF_INET, - .sin_port = htons(port), - .sin_addr.s_addr = htonl(INADDR_LOOPBACK) }; + TestTlsConn c; char status[128] = { 0 }; size_t n = 0; - int cs = socket(AF_INET, SOCK_STREAM, 0); - REQUIRE(cs >= 0); - REQUIRE(connect(cs, (struct sockaddr*)&sa, sizeof(sa)) == 0); + REQUIRE(test_tls_connect(&c, port, g_tls_cert, g_tls_cert_len) == 0); - REQUIRE(send(cs, hdr, strlen(hdr), 0) == (ssize_t)strlen(hdr)); + REQUIRE(test_tls_write(&c, hdr, strlen(hdr)) == 0); nap_ms(80); - (void)send(cs, seg2, strlen(seg2), 0); + (void)test_tls_write(&c, seg2, strlen(seg2)); nap_ms(80); - (void)send(cs, seg3, strlen(seg3), 0); + (void)test_tls_write(&c, seg3, strlen(seg3)); while (n + 1 < sizeof(status)) { - ssize_t r = recv(cs, status + n, sizeof(status) - 1 - n, 0); + int r = test_tls_read(&c, status + n, sizeof(status) - 1 - n); if (r <= 0) break; n += (size_t)r; @@ -243,7 +234,7 @@ static int accept_multisegment_chunked_body(uint16_t port) if (memchr(status, '\n', n) != NULL) break; } - close(cs); + test_tls_close(&c); REQUIRE(strstr(status, "Bad CSR") != NULL); return 0; @@ -337,32 +328,26 @@ static int keepalive_after_split_trailer(uint16_t port) "Host: 127.0.0.1\r\n" "Connection: close\r\n" "\r\n"; - struct sockaddr_in sa = { .sin_family = AF_INET, - .sin_port = htons(port), - .sin_addr.s_addr = htonl(INADDR_LOOPBACK) }; + TestTlsConn c; char* req1_head = NULL; char* req1_tail = NULL; size_t req1_head_len = 0; char resp[4096] = { 0 }; size_t n = 0; - int cs; REQUIRE(build_split_enroll(&req1_head, &req1_head_len, &req1_tail) == 0); - cs = socket(AF_INET, SOCK_STREAM, 0); - REQUIRE(cs >= 0); - REQUIRE(connect(cs, (struct sockaddr*)&sa, sizeof(sa)) == 0); + REQUIRE(test_tls_connect(&c, port, g_tls_cert, g_tls_cert_len) == 0); /* Request #1: last-chunk line first, trailer CRLF withheld into its * own segment so a premature "0\r\n" completion leaves it unread. */ - REQUIRE(send(cs, req1_head, req1_head_len, 0) == (ssize_t)req1_head_len); + REQUIRE(test_tls_write(&c, req1_head, req1_head_len) == 0); nap_ms(80); - REQUIRE(send(cs, req1_tail, strlen(req1_tail), 0) - == (ssize_t)strlen(req1_tail)); + REQUIRE(test_tls_write(&c, req1_tail, strlen(req1_tail)) == 0); /* Wait for request #1's response head before sending request #2. */ while (n + 1 < sizeof(resp)) { - ssize_t r = recv(cs, resp + n, sizeof(resp) - 1 - n, 0); + int r = test_tls_read(&c, resp + n, sizeof(resp) - 1 - n); if (r <= 0) break; n += (size_t)r; @@ -372,18 +357,18 @@ static int keepalive_after_split_trailer(uint16_t port) } REQUIRE(strstr(resp, "200") != NULL); /* enrollment issued a cert */ - REQUIRE(send(cs, req2, strlen(req2), 0) == (ssize_t)strlen(req2)); + REQUIRE(test_tls_write(&c, req2, strlen(req2)) == 0); /* Drain until the server closes (request #2 asked for Connection: * close), appending onto the same buffer. */ while (n + 1 < sizeof(resp)) { - ssize_t r = recv(cs, resp + n, sizeof(resp) - 1 - n, 0); + int r = test_tls_read(&c, resp + n, sizeof(resp) - 1 - n); if (r <= 0) break; n += (size_t)r; resp[n] = '\0'; } - close(cs); + test_tls_close(&c); free(req1_head); free(req1_tail); @@ -402,9 +387,16 @@ int main(void) REQUIRE(wolfcert_init(NULL) == WOLFCERT_OK); + uint8_t* tls_key = NULL; + size_t tls_key_len = 0; + REQUIRE(gen_server_identity(&g_tls_cert, &g_tls_cert_len, + &tls_key, &tls_key_len) == 0); + WolfCertServerCfgSrv cfg = { .protocol = WOLFCERT_PROTO_EST, .bind_host = "127.0.0.1", .bind_port = 0, + .tls_cert_pem = g_tls_cert, .tls_cert_pem_len = g_tls_cert_len, + .tls_key_pem = tls_key, .tls_key_pem_len = tls_key_len, }; WolfCertServer* srv = NULL; REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_OK); @@ -426,6 +418,8 @@ int main(void) wolfcert_server_stop(srv); pthread_join(tid, NULL); wolfcert_server_free(srv); + free(g_tls_cert); + free(tls_key); if (rc != 0) return rc; diff --git a/tests/integration/test_est_csr_attrs_enforce.c b/tests/integration/test_est_csr_attrs_enforce.c index c61f188..d6659eb 100644 --- a/tests/integration/test_est_csr_attrs_enforce.c +++ b/tests/integration/test_est_csr_attrs_enforce.c @@ -188,51 +188,41 @@ static int enroll_without_challenge(WolfCertServer* s) return 0; } -/* Dial 127.0.0.1:port over plain TCP, send the request, and read the whole +/* Dial 127.0.0.1:port over TLS, send the request, and read the whole * response (headers + body) into `resp`. A receive timeout keeps a * misbehaving server from hanging the test. Returns bytes read, or -1. */ static int send_and_read_all(uint16_t port, const void* req, size_t req_len, char* resp, size_t cap) { struct timeval tv = { .tv_sec = 5, .tv_usec = 0 }; - struct sockaddr_in sa = { .sin_family = AF_INET, - .sin_port = htons(port), - .sin_addr.s_addr = htonl(INADDR_LOOPBACK) }; - const char* p = req; - size_t left = req_len; + TestTlsConn c; size_t n = 0; - int cs = socket(AF_INET, SOCK_STREAM, 0); - if (cs < 0) + + if (test_tls_connect(&c, port, g_ca, g_ca_len) != 0) return -1; - setsockopt(cs, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)); - if (connect(cs, (struct sockaddr*)&sa, sizeof(sa)) < 0) { - close(cs); + + setsockopt(c.fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)); + + if (test_tls_write(&c, req, req_len) != 0) { + test_tls_close(&c); return -1; } - while (left > 0) { - ssize_t w = send(cs, p, left, 0); - if (w <= 0) { - close(cs); - return -1; - } - p += (size_t)w; - left -= (size_t)w; - } + while (n + 1 < cap) { - ssize_t r = recv(cs, resp + n, cap - 1 - n, 0); + int r = test_tls_read(&c, resp + n, cap - 1 - n); if (r <= 0) break; n += (size_t)r; } resp[n] = '\0'; - close(cs); + test_tls_close(&c); return (int)n; } -/* Raw plain-HTTP probe of the enforcement 400 body: build a real CSR that - * omits challengePassword, POST it to /simpleenroll, and assert the response - * both fails with 400 and names the exact missing OID in dotted form. This is - * the end-to-end check that the value-result OID copy in csr_attrs_enforce +/* Raw-HTTP probe of the enforcement 400 body: build a real CSR that omits + * challengePassword, POST it to /simpleenroll, and assert the response both + * fails with 400 and names the exact missing OID in dotted form. This is the + * end-to-end check that the value-result OID copy in csr_attrs_enforce * renders the correct bytes; the client path above only observes rejection, * not the body. */ static int reject_body_names_missing_oid(uint16_t port) @@ -379,15 +369,14 @@ int main(void) pthread_join(tid2, NULL); wolfcert_server_free(srv2); wolfcert_buffer_free(&policy2); - free(tls_cert); - free(tls_key); if (rc != 0) return rc; - /* Plain-HTTP server (no TLS) with the same bare-OID policy, so the raw - * 400 body is readable: asserts it names the missing OID in dotted form. - * The client path above proves rejection; this proves the reported OID - * content (i.e. the value-result OID copy renders the right bytes). */ + /* Third server with the same bare-OID policy, driven by a raw HTTP + * request so the 400 body is readable: asserts it names the missing OID + * in dotted form. The client path above proves rejection; this proves the + * reported OID content (i.e. the value-result OID copy renders the right + * bytes). */ WolfCertBuffer policy_raw = { 0 }; REQUIRE(build_policy(&policy_raw) == WOLFCERT_OK); WolfCertServerCfgSrv cfg_raw = { @@ -396,6 +385,8 @@ int main(void) .csr_attributes_der = policy_raw.data, .csr_attributes_len = policy_raw.len, .est_require_csr_attributes = 1, + .tls_cert_pem = tls_cert, .tls_cert_pem_len = tls_cert_len, + .tls_key_pem = tls_key, .tls_key_pem_len = tls_key_len, }; WolfCertServer* srv_raw = NULL; REQUIRE(wolfcert_server_start(&cfg_raw, &srv_raw) == WOLFCERT_OK); @@ -408,6 +399,8 @@ int main(void) pthread_join(tid_raw, NULL); wolfcert_server_free(srv_raw); wolfcert_buffer_free(&policy_raw); + free(tls_cert); + free(tls_key); if (rc != 0) return rc; diff --git a/tests/integration/test_est_tls_roundtrip.c b/tests/integration/test_est_tls_roundtrip.c index 9841feb..b4bfa43 100644 --- a/tests/integration/test_est_tls_roundtrip.c +++ b/tests/integration/test_est_tls_roundtrip.c @@ -71,6 +71,30 @@ int main(void) size_t tls_key_len = 0; REQUIRE(gen_server_identity(&tls_cert, &tls_cert_len, &tls_key, &tls_key_len) == 0); + /* RFC 7030 has no plaintext mode, so an EST listener configured without a + * TLS identity must be refused at start rather than serve /simpleenroll + * over cleartext HTTP. */ + WolfCertStoreOps* plain_store = wolfcert_store_memory_open(NULL); + REQUIRE(plain_store != NULL); + WolfCertServerCfgSrv plain = { + .protocol = WOLFCERT_PROTO_EST, + .bind_host = "127.0.0.1", + .bind_port = 0, + .ca_store = plain_store, + }; + WolfCertServer* plain_srv = NULL; + REQUIRE(wolfcert_server_start(&plain, &plain_srv) == WOLFCERT_ERR_TLS); + REQUIRE(plain_srv == NULL); + + /* The rejection must land before the CA is minted, so the caller is not + * left with a CA it never asked for -- one the next start would adopt. */ + WolfCertBuffer leftover = { 0 }; + REQUIRE(plain_store->read(plain_store->ctx, "ca.cert.der", &leftover) + == WOLFCERT_ERR_NOT_FOUND); + REQUIRE(plain_store->read(plain_store->ctx, "ca.key.der", &leftover) + == WOLFCERT_ERR_NOT_FOUND); + wolfcert_store_memory_close(plain_store); + WolfCertServerCfgSrv cfg = { .protocol = WOLFCERT_PROTO_EST, .bind_host = "127.0.0.1", diff --git a/tests/integration/tls_test_util.h b/tests/integration/tls_test_util.h index d6c3a9a..b3181e7 100644 --- a/tests/integration/tls_test_util.h +++ b/tests/integration/tls_test_util.h @@ -35,15 +35,20 @@ #include /* pid_t, referenced by wolfssl/wolfcrypt/random.h */ #include +#include #include #include #include #include #include +#include +#include #include #include #include +#include +#include /* A key algorithm + parameter the current build supports, for client * enrollments where the algorithm is incidental to what the test verifies. */ @@ -210,4 +215,91 @@ static inline int gen_server_identity(uint8_t** cert_pem, size_t* cert_pem_len, key_pem, key_pem_len); } +/* A raw TLS client against the in-tree test server, for the tests that need + * to script byte-exact HTTP rather than go through wolfcert_est_*. Each + * test_tls_write() becomes one TLS record and so one wolfSSL_read() on the + * server, which is what the segmentation-sensitive framing tests rely on. */ +typedef struct { + WOLFSSL_CTX* ctx; + WOLFSSL* ssl; + int fd; +} TestTlsConn; + +static inline void test_tls_close(TestTlsConn* c) +{ + if (c->ssl != NULL) { + wolfSSL_shutdown(c->ssl); + wolfSSL_free(c->ssl); + c->ssl = NULL; + } + if (c->ctx != NULL) { + wolfSSL_CTX_free(c->ctx); + c->ctx = NULL; + } + if (c->fd >= 0) { + close(c->fd); + c->fd = -1; + } +} + +/* Connect to 127.0.0.1:port and handshake, pinning `ca_pem` as the sole trust + * anchor. Returns 0 on success; the caller closes with test_tls_close(). */ +static inline int test_tls_connect(TestTlsConn* c, uint16_t port, + const uint8_t* ca_pem, size_t ca_pem_len) +{ + struct sockaddr_in sa = { 0 }; + + memset(c, 0, sizeof(*c)); + c->fd = -1; + + c->ctx = wolfSSL_CTX_new(wolfTLS_client_method()); + if (c->ctx == NULL) + return -1; + + if (wolfSSL_CTX_load_verify_buffer(c->ctx, ca_pem, (long)ca_pem_len, + WOLFSSL_FILETYPE_PEM) + != WOLFSSL_SUCCESS) + goto fail; + + wolfSSL_CTX_set_verify(c->ctx, WOLFSSL_VERIFY_PEER, NULL); + + c->fd = socket(AF_INET, SOCK_STREAM, 0); + if (c->fd < 0) + goto fail; + + sa.sin_family = AF_INET; + sa.sin_port = htons(port); + if (inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr) != 1) + goto fail; + if (connect(c->fd, (struct sockaddr*)&sa, sizeof(sa)) < 0) + goto fail; + + c->ssl = wolfSSL_new(c->ctx); + if (c->ssl == NULL) + goto fail; + + if (wolfSSL_set_fd(c->ssl, c->fd) != WOLFSSL_SUCCESS) + goto fail; + + if (wolfSSL_connect(c->ssl) != WOLFSSL_SUCCESS) + goto fail; + + return 0; +fail: + test_tls_close(c); + return -1; +} + +/* Write `len` bytes as a single TLS record. Returns 0 on success. */ +static inline int test_tls_write(TestTlsConn* c, const void* buf, size_t len) +{ + return wolfSSL_write(c->ssl, buf, (int)len) == (int)len ? 0 : -1; +} + +/* One wolfSSL_read(). Returns the byte count, or <= 0 at close/error. */ +static inline int test_tls_read(TestTlsConn* c, void* buf, size_t len) +{ + return wolfSSL_read(c->ssl, buf, (int)len); +} + #endif /* WOLFCERT_TLS_TEST_UTIL_H */ diff --git a/wolfcert/server.h b/wolfcert/server.h index 706053a..69de315 100644 --- a/wolfcert/server.h +++ b/wolfcert/server.h @@ -49,11 +49,16 @@ typedef struct { WolfCertKeyType ca_key_type; /* WOLFCERT_KEY_RSA default */ int ca_key_param; /* 2048 default for RSA, 256 for ECC */ - /* Optional TLS. If tls_cert_pem + tls_key_pem are set, the server + /* TLS identity. If tls_cert_pem + tls_key_pem are set, the server * terminates TLS on every accepted connection before dispatching to * the protocol handler. tls_client_ca_pem, when set, enables mutual * TLS (WOLFSSL_VERIFY_PEER) against the supplied client-CA bundle. * + * Mandatory for WOLFCERT_PROTO_EST (RFC 7030 has no plaintext mode): + * wolfcert_server_start() returns WOLFCERT_ERR_TLS without them. + * Optional for WOLFCERT_PROTO_SCEP, which authenticates at the + * pkiMessage layer and may be served over cleartext HTTP. + * * All three point at caller-owned PEM bytes; wolfCert copies what it * needs during wolfcert_server_start() and does not retain the * pointers. */ From 204c7012ff980979b9f4488d45be2ddcb9e80766 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 9 Sep 2026 11:54:14 +0200 Subject: [PATCH 02/18] http: keep IPv6 literals bracketed in origins and Host headers wolfcert_http_url_parse stores an IP-literal host with its brackets stripped, but wolfcert_http_url_origin re-emitted it bare, producing an origin such as https://::1:8443. The EST and SCEP session opens feed that string back through wolfcert_http_url_parse to build the session base URL, where the non-bracket host scan stops at the leading colon and yields a bogus host and port, so every session-based operation against an IPv6 literal server URL broke before the connect. The two request builders had the same defect, emitting a Host header of ::1:8443 rather than the bracketed form RFC 7230 section 5.4 requires. Re-add the brackets whenever the stored host carries a colon, which can only happen on the bracket-stripping parse branch, and cover the parse to origin to parse roundtrip in the URL unit test. Fixes F-8009. --- src/http.c | 41 ++++++++++++---- tests/unit/test_http.c | 106 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 10 deletions(-) diff --git a/src/http.c b/src/http.c index 7a0b857..7e88334 100644 --- a/src/http.c +++ b/src/http.c @@ -101,6 +101,14 @@ WOLFCERT_TEST_VIS void wolfcert_http_url_free(WolfCertUrl* u) u->scheme = u->host = u->path = NULL; } +/* wolfcert_http_url_parse stores an IPv6 literal with its brackets stripped, so + * a host carrying a colon is one: anything re-emitted into a URL or a Host + * header has to bracket it again (RFC 3986 section 3.2.2). */ +static int host_is_ip_literal(const char* host) +{ + return strchr(host, ':') != NULL; +} + /* Build the "scheme://host[:port]" origin for a parsed URL into a freshly * allocated buffer (owned by the caller, free with WOLFCERT_XFREE). The default * port (443 for TLS, 80 otherwise) is omitted. Shared by the EST and SCEP @@ -108,23 +116,30 @@ WOLFCERT_TEST_VIS void wolfcert_http_url_free(WolfCertUrl* u) WOLFCERT_TEST_VIS int wolfcert_http_url_origin(const WolfCertUrl* u, void* heap, char** out_origin) { - size_t origin_len; - char* origin; + size_t origin_len; + char* origin; + const char* open_br; + const char* close_br; if (u == NULL || u->scheme == NULL || u->host == NULL || out_origin == NULL) return WOLFCERT_ERR_BAD_ARG; - /* scheme + "://" (3) + host + the optional ":65535" and NUL; 16 leaves the - * port suffix room to spare rather than sizing it to the digit. */ + open_br = host_is_ip_literal(u->host) ? "[" : ""; + close_br = host_is_ip_literal(u->host) ? "]" : ""; + + /* scheme + "://" (3) + host + the optional brackets, ":65535" and NUL; 16 + * leaves that suffix room to spare rather than sizing it to the digit. */ origin_len = strlen(u->scheme) + 3 + strlen(u->host) + 16; origin = (char*)WOLFCERT_XMALLOC(origin_len, heap); if (origin == NULL) return WOLFCERT_ERR_MEMORY; if ((u->tls && u->port == 443) || (!u->tls && u->port == 80)) - snprintf(origin, origin_len, "%s://%s", u->scheme, u->host); + snprintf(origin, origin_len, "%s://%s%s%s", u->scheme, + open_br, u->host, close_br); else - snprintf(origin, origin_len, "%s://%s:%d", u->scheme, u->host, u->port); + snprintf(origin, origin_len, "%s://%s%s%s:%d", u->scheme, + open_br, u->host, close_br, u->port); *out_origin = origin; return WOLFCERT_OK; @@ -1015,6 +1030,9 @@ static int http_write_request(WolfCertConn* c, const WolfCertUrl* u, snprintf(port_frag, sizeof(port_frag), ":%d", u->port); } + const char* open_br = host_is_ip_literal(u->host) ? "[" : ""; + const char* close_br = host_is_ip_literal(u->host) ? "]" : ""; + size_t head_cap = 1024 + (req->content_type ? strlen(req->content_type) : 0) + (req->content_transfer_encoding ? strlen(req->content_transfer_encoding) : 0) @@ -1028,7 +1046,7 @@ static int http_write_request(WolfCertConn* c, const WolfCertUrl* u, int hn = snprintf(head, head_cap, "%s %s HTTP/1.1\r\n" - "Host: %s%s\r\n" + "Host: %s%s%s%s\r\n" "User-Agent: wolfCert/%s\r\n" "Connection: %s\r\n" "%s%s%s" @@ -1038,7 +1056,7 @@ static int http_write_request(WolfCertConn* c, const WolfCertUrl* u, "%s" "\r\n", req->method, u->path, - u->host, port_frag, + open_br, u->host, close_br, port_frag, WOLFCERT_VERSION_STRING, keep_alive ? "keep-alive" : "close", req->accept ? "Accept: " : "", @@ -1554,6 +1572,9 @@ static int build_head(WolfCertHttpSession* s, const WolfCertHttpRequest* req, if ((u->tls && u->port != 443) || (!u->tls && u->port != 80)) snprintf(port_frag, sizeof(port_frag), ":%d", u->port); + const char* open_br = host_is_ip_literal(u->host) ? "[" : ""; + const char* close_br = host_is_ip_literal(u->host) ? "]" : ""; + size_t head_cap = 1024 + (req->content_type ? strlen(req->content_type) : 0) + (req->content_transfer_encoding ? strlen(req->content_transfer_encoding) : 0) @@ -1567,7 +1588,7 @@ static int build_head(WolfCertHttpSession* s, const WolfCertHttpRequest* req, int hn = snprintf(head, head_cap, "%s %s HTTP/1.1\r\n" - "Host: %s%s\r\n" + "Host: %s%s%s%s\r\n" "User-Agent: wolfCert/%s\r\n" "Connection: keep-alive\r\n" "%s%s%s" @@ -1577,7 +1598,7 @@ static int build_head(WolfCertHttpSession* s, const WolfCertHttpRequest* req, "%s" "\r\n", req->method, u->path, - u->host, port_frag, + open_br, u->host, close_br, port_frag, WOLFCERT_VERSION_STRING, req->accept ? "Accept: " : "", req->accept ? req->accept : "", diff --git a/tests/unit/test_http.c b/tests/unit/test_http.c index e1fbed2..01c2471 100644 --- a/tests/unit/test_http.c +++ b/tests/unit/test_http.c @@ -113,6 +113,29 @@ static int test_url_origin(void) WOLFCERT_XFREE(origin, NULL); origin = NULL; wolfcert_http_url_free(&u); + /* An IPv6 literal is re-bracketed, so parse -> origin -> parse round-trips + * instead of collapsing into an unparsable "https://::1:8443". */ + REQUIRE(wolfcert_http_url_parse("https://[::1]:8443/p", &u, NULL) == WOLFCERT_OK); + REQUIRE(wolfcert_http_url_origin(&u, NULL, &origin) == WOLFCERT_OK); + REQUIRE(strcmp(origin, "https://[::1]:8443") == 0); + wolfcert_http_url_free(&u); + REQUIRE(wolfcert_http_url_parse(origin, &u, NULL) == WOLFCERT_OK); + REQUIRE(strcmp(u.host, "::1") == 0); + REQUIRE(u.port == 8443); + WOLFCERT_XFREE(origin, NULL); origin = NULL; + wolfcert_http_url_free(&u); + + /* Same for the default port, where no ":port" suffix follows the host. */ + REQUIRE(wolfcert_http_url_parse("https://[2001:db8::1]/p", &u, NULL) == WOLFCERT_OK); + REQUIRE(wolfcert_http_url_origin(&u, NULL, &origin) == WOLFCERT_OK); + REQUIRE(strcmp(origin, "https://[2001:db8::1]") == 0); + wolfcert_http_url_free(&u); + REQUIRE(wolfcert_http_url_parse(origin, &u, NULL) == WOLFCERT_OK); + REQUIRE(strcmp(u.host, "2001:db8::1") == 0); + REQUIRE(u.port == 443); + WOLFCERT_XFREE(origin, NULL); origin = NULL; + wolfcert_http_url_free(&u); + /* NULL url and NULL out are rejected. */ REQUIRE(wolfcert_http_url_origin(NULL, NULL, &origin) == WOLFCERT_ERR_BAD_ARG); REQUIRE(wolfcert_http_url_parse("https://h/x", &u, NULL) == WOLFCERT_OK); @@ -153,6 +176,33 @@ static int listen_loopback(int* port) return ls; } +/* Same on ::1. Returns -1 when the host has no IPv6 loopback, which the + * callers treat as "skip" rather than "fail". */ +static int listen_loopback6(int* port) +{ + struct sockaddr_in6 sa; + socklen_t slen = sizeof(sa); + int yes = 1; + int ls = socket(AF_INET6, SOCK_STREAM, 0); + + if (ls < 0) + return -1; + + setsockopt(ls, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(yes)); + memset(&sa, 0, sizeof(sa)); + sa.sin6_family = AF_INET6; + sa.sin6_port = htons(0); + sa.sin6_addr = in6addr_loopback; + if (bind(ls, (struct sockaddr*)&sa, sizeof(sa)) < 0 || listen(ls, 1) < 0 || + getsockname(ls, (struct sockaddr*)&sa, &slen) < 0) { + close(ls); + return -1; + } + + *port = ntohs(sa.sin6_port); + return ls; +} + static void* srv_thread(void* arg) { struct srv_ctx* sc = (struct srv_ctx*)arg; @@ -463,6 +513,60 @@ static int test_request_transfer_encoding(void) return 0; } +/* RFC 3986 section 3.2.2: an IPv6 literal stays bracketed in the Host header, + * or a virtual-host match against "::1:8443" fails. Both request builders + * carry their own copy of the bracketing, so drive each one. */ +static int ipv6_host_header(int use_session) +{ + struct capture_ctx cc = { 0 }; + pthread_t tid; + char base[128]; + char url[160]; + char expect[64]; + int port = 0; + + cc.listen_fd = listen_loopback6(&port); + if (cc.listen_fd < 0) { + printf("no IPv6 loopback, skipping Host-header check\n"); + return 0; + } + REQUIRE(pthread_create(&tid, NULL, srv_thread_capture, &cc) == 0); + + snprintf(base, sizeof(base), "http://[::1]:%d", port); + snprintf(url, sizeof(url), "http://[::1]:%d/p", port); + snprintf(expect, sizeof(expect), "Host: [::1]:%d\r\n", port); + + WolfCertHttpRequest req = { .method = "GET", .url = url }; + WolfCertHttpResponse resp = { 0 }; + + if (use_session) { + WolfCertHttpSessionCfg cfg = { .base_url = base }; + WolfCertHttpSession* s = NULL; + + REQUIRE(wolfcert_http_session_open(&cfg, &s) == WOLFCERT_OK); + REQUIRE(wolfcert_http_session_request(s, &req, &resp) == WOLFCERT_OK); + wolfcert_http_session_close(s); + } + else { + REQUIRE(wolfcert_http_request(&req, &resp) == WOLFCERT_OK); + } + + REQUIRE(resp.status_code == 200); + wolfcert_http_response_free(&resp); + pthread_join(tid, NULL); + + REQUIRE(strstr(cc.request, expect) != NULL); + return 0; +} + +static int test_request_host_header_ipv6(void) +{ + if (ipv6_host_header(0)) + return 1; + + return ipv6_host_header(1); +} + int main(void) { REQUIRE(wolfcert_init(NULL) == WOLFCERT_OK); @@ -478,6 +582,8 @@ int main(void) return 1; if (test_request_transfer_encoding()) return 1; + if (test_request_host_header_ipv6()) + return 1; wolfcert_cleanup(); printf("OK\n"); return 0; From e75492bc74deb8cf2c6aa1539f624932d23de59e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 9 Sep 2026 12:01:51 +0200 Subject: [PATCH 03/18] Carry every CSR subject RDN into the issued certificate wolfcert_ca_issue rebuilt the subject of the issued certificate field by field and only copied CN, O, OU, C, ST and L. Every other name component the CSR builder accepts was discarded without an error, so a CSR carrying for example UID and postalCode produced a certificate holding neither. Copy the remaining components wolfSSL exposes on the decoded CSR. The givenName copy is inert for now because wolfSSL's decoder only stores subject ids up to ASN_USER_ID and never fills subjectGN, but the field is in place for when that changes. The EST round-trip test now enrolls a CSR carrying the full set and requires each component to reappear in the issued certificate. Fixes F-8025. --- src/ca_issue.c | 23 ++++++--- tests/integration/test_est_roundtrip.c | 69 ++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/src/ca_issue.c b/src/ca_issue.c index 72c30dd..afadbf0 100644 --- a/src/ca_issue.c +++ b/src/ca_issue.c @@ -650,12 +650,23 @@ int wolfcert_ca_issue(WolfCertCa* ca, if (rc == 0) { wc_InitCert_ex(nc, heap, WOLFCERT_DEVID_SOFTWARE); - COPY_SUBJ(subjectCN, nc->subject.commonName); - COPY_SUBJ(subjectO, nc->subject.org); - COPY_SUBJ(subjectOU, nc->subject.unit); - COPY_SUBJ(subjectC, nc->subject.country); - COPY_SUBJ(subjectST, nc->subject.state); - COPY_SUBJ(subjectL, nc->subject.locality); + COPY_SUBJ(subjectCN, nc->subject.commonName); + COPY_SUBJ(subjectO, nc->subject.org); + COPY_SUBJ(subjectOU, nc->subject.unit); + COPY_SUBJ(subjectC, nc->subject.country); + COPY_SUBJ(subjectST, nc->subject.state); + COPY_SUBJ(subjectL, nc->subject.locality); + COPY_SUBJ(subjectSN, nc->subject.sur); + /* wolfSSL only stores subject ids up to ASN_USER_ID, so givenName + * never arrives; copy it anyway for when that gap closes. */ + COPY_SUBJ(subjectGN, nc->subject.givenName); + COPY_SUBJ(subjectEmail, nc->subject.email); + COPY_SUBJ(subjectSND, nc->subject.serialDev); + COPY_SUBJ(subjectUID, nc->subject.userId); + COPY_SUBJ(subjectPC, nc->subject.postalCode); +#ifdef WOLFSSL_CERT_EXT + COPY_SUBJ(subjectBC, nc->subject.busCat); +#endif if (wc_SetIssuerBuffer(nc, ca->cert_der, (int)ca->cert_der_len) != 0) rc = WOLFCERT_ERR_CRYPTO; diff --git a/tests/integration/test_est_roundtrip.c b/tests/integration/test_est_roundtrip.c index 441ef15..eefad7c 100644 --- a/tests/integration/test_est_roundtrip.c +++ b/tests/integration/test_est_roundtrip.c @@ -208,6 +208,71 @@ static int enroll_check_san(const WolfCertServerCfg* client_cfg) return 0; } +static int rdn_is(const char* p, int len, const char* want) +{ + return p != NULL && len == (int)strlen(want) && + memcmp(p, want, (size_t)len) == 0; +} + +/* Regression: every subject RDN the CSR builder accepts must survive into the + * issued certificate. The CA rebuilds the subject field by field, so a name + * component it has no copy for is dropped without any error. givenName is + * absent below because wolfSSL's decoder never reports it back. */ +static int enroll_check_subject_rdns(const WolfCertServerCfg* client_cfg) +{ + WolfCertKeyCfg kcfg = { .type = TEST_ENROLL_KEY_TYPE, .param = TEST_ENROLL_KEY_PARAM, + .dev_id = WOLFCERT_DEVID_SOFTWARE }; + WolfCertKey* dk = NULL; + REQUIRE(wolfcert_key_generate(&kcfg, &dk) == WOLFCERT_OK); + + WolfCertCertMeta meta = { + .subject_dn = "CN=device-rdn,O=Acme,OU=Devices,C=US,ST=Washington," + "L=Seattle,SN=Doe," + "emailAddress=jane@example.com,serialNumber=SRL-42," + "UID=factory-1,postalCode=98109" +#ifdef WOLFSSL_CERT_EXT + ",businessCategory=Manufacturing" +#endif + }; + WolfCertBuffer csr = { 0 }; + REQUIRE(wolfcert_csr_build(dk, &meta, &csr) == WOLFCERT_OK); + + WolfCertBuffer issued = { 0 }; + REQUIRE(wolfcert_est_simple_enroll(client_cfg, csr.data, csr.len, &issued) + == WOLFCERT_OK); + + DerBuffer* der = NULL; + REQUIRE(wc_PemToDer(issued.data, (long)issued.len, CERT_TYPE, &der, + NULL, NULL, NULL) == 0); + + DecodedCert dc; + wc_InitDecodedCert(&dc, der->buffer, der->length, NULL); + REQUIRE(wc_ParseCert(&dc, CERT_TYPE, NO_VERIFY, NULL) == 0); + + REQUIRE(rdn_is(dc.subjectCN, dc.subjectCNLen, "device-rdn")); + REQUIRE(rdn_is(dc.subjectO, dc.subjectOLen, "Acme")); + REQUIRE(rdn_is(dc.subjectOU, dc.subjectOULen, "Devices")); + REQUIRE(rdn_is(dc.subjectC, dc.subjectCLen, "US")); + REQUIRE(rdn_is(dc.subjectST, dc.subjectSTLen, "Washington")); + REQUIRE(rdn_is(dc.subjectL, dc.subjectLLen, "Seattle")); + /* The guard: everything below was dropped by the issuer. */ + REQUIRE(rdn_is(dc.subjectSN, dc.subjectSNLen, "Doe")); + REQUIRE(rdn_is(dc.subjectEmail, dc.subjectEmailLen, "jane@example.com")); + REQUIRE(rdn_is(dc.subjectSND, dc.subjectSNDLen, "SRL-42")); + REQUIRE(rdn_is(dc.subjectUID, dc.subjectUIDLen, "factory-1")); + REQUIRE(rdn_is(dc.subjectPC, dc.subjectPCLen, "98109")); +#ifdef WOLFSSL_CERT_EXT + REQUIRE(rdn_is(dc.subjectBC, dc.subjectBCLen, "Manufacturing")); +#endif + + wc_FreeDecodedCert(&dc); + wc_FreeDer(&der); + wolfcert_buffer_free(&csr); + wolfcert_buffer_free(&issued); + wolfcert_key_free(dk); + return 0; +} + /* HTTP Basic (RFC 7030 section 3.2.3) must authenticate a keep-alive session * too, not just the one-shot calls: the credentials have to ride every request * on the connection. Enrolls with the good credentials from `client_cfg`, then @@ -341,6 +406,10 @@ int main(void) if (enroll_check_san(&client_cfg)) return 1; + /* Full subject-RDN round-trip guard. */ + if (enroll_check_subject_rdns(&client_cfg)) + return 1; + /* Keep-alive session against the same Basic-auth-protected server. */ if (session_basic_auth(&client_cfg)) return 1; From 766eb8a1d36076e5f7b3b59ac264e9999f89a8f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 9 Sep 2026 13:41:20 +0200 Subject: [PATCH 04/18] Fail server startup when the CA store cannot be read or written wolfcert_server_start treated every wolfcert_ca_load failure as an empty store, so a transient read error or a corrupt ca.key.der made the server mint a fresh CA and overwrite the stored one, re-rooting every previously issued certificate. The wolfcert_ca_save return was discarded as well, so a store that could not be written still reported a successful start with a CA that only lived in RAM. Generate a new CA only when the load reports WOLFCERT_ERR_NOT_FOUND, propagate any other load error, and fail the start when saving a freshly generated CA does not succeed. A save that fails halfway is the same trap in miniature: wolfcert_ca_save writes the certificate before the key, so a failing key write left a certificate-only store, which the load path now rejects for good and no later start can repair. Remove the certificate again when the key does not follow, so the next start bootstraps into an empty store instead of a poisoned one. Fixes F-8026. --- Makefile.am | 5 +- src/ca_issue.c | 36 +++- src/server.c | 26 ++- tests/CMakeLists.txt | 4 + tests/unit/test_server_ca_store.c | 299 ++++++++++++++++++++++++++++++ wolfcert/server.h | 5 +- 6 files changed, 359 insertions(+), 16 deletions(-) create mode 100644 tests/unit/test_server_ca_store.c diff --git a/Makefile.am b/Makefile.am index ccd7d24..06ee49d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -188,7 +188,8 @@ test_scep_msg_SOURCES = tests/unit/test_scep_msg.c test_scep_msg_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/src test_scep_msg_LDADD = libwolfcert.la $(WOLFSSL_LIBS) if WOLFCERT_HAVE_SERVER -check_PROGRAMS += test_scep_roundtrip test_scep_poll_roundtrip test_scep_async_roundtrip +check_PROGRAMS += test_scep_roundtrip test_scep_poll_roundtrip test_scep_async_roundtrip \ + test_server_ca_store test_scep_roundtrip_SOURCES = tests/integration/test_scep_roundtrip.c test_scep_roundtrip_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/src test_scep_roundtrip_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread @@ -196,6 +197,8 @@ test_scep_poll_roundtrip_SOURCES = tests/integration/test_scep_poll_roundtrip.c test_scep_poll_roundtrip_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread test_scep_async_roundtrip_SOURCES = tests/integration/test_scep_async_roundtrip.c test_scep_async_roundtrip_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread +test_server_ca_store_SOURCES = tests/unit/test_server_ca_store.c +test_server_ca_store_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread endif endif diff --git a/src/ca_issue.c b/src/ca_issue.c index afadbf0..2540385 100644 --- a/src/ca_issue.c +++ b/src/ca_issue.c @@ -224,14 +224,26 @@ int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, void* heap) WolfCertBuffer cert_buf = { .heap = heap }; WolfCertBuffer key_buf = { .heap = heap }; - int rc = store->read(store->ctx, "ca.cert.der", &cert_buf); - if (rc != WOLFCERT_OK) - return rc; + int cert_rc = store->read(store->ctx, "ca.cert.der", &cert_buf); + int key_rc = store->read(store->ctx, "ca.key.der", &key_buf); + int rc; - rc = store->read(store->ctx, "ca.key.der", &key_buf); - if (rc != WOLFCERT_OK) { + if (cert_rc != WOLFCERT_OK || key_rc != WOLFCERT_OK) { wolfcert_buffer_free(&cert_buf); - return rc; + wolfcert_buffer_free(&key_buf); + + if (cert_rc == WOLFCERT_ERR_NOT_FOUND && key_rc == WOLFCERT_ERR_NOT_FOUND) + return WOLFCERT_ERR_NOT_FOUND; + + /* Half a pair is a damaged store, not an empty one. Reporting + * NOT_FOUND here would let the caller mint a CA over the survivor. */ + if (cert_rc == WOLFCERT_ERR_NOT_FOUND || key_rc == WOLFCERT_ERR_NOT_FOUND) + return WOLFCERT_ERR(WOLFCERT_ERR_PARSE, "ca", + "CA store is incomplete: %s is missing", + cert_rc == WOLFCERT_ERR_NOT_FOUND ? "ca.cert.der" : "ca.key.der"); + + rc = (cert_rc != WOLFCERT_OK) ? cert_rc : key_rc; + return WOLFCERT_ERR(rc, "ca", "CA store read failed"); } /* Iterate every registered algorithm and see which private-key decoder @@ -266,7 +278,8 @@ int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, void* heap) wolfcert_buffer_free(&cert_buf); wolfcert_buffer_free(&key_buf); - return WOLFCERT_ERR_PARSE; + return WOLFCERT_ERR(WOLFCERT_ERR_PARSE, "ca", + "stored CA key does not decode as any supported algorithm"); } int wolfcert_ca_save(const WolfCertCa* ca, WolfCertStoreOps* store) @@ -278,7 +291,14 @@ int wolfcert_ca_save(const WolfCertCa* ca, WolfCertStoreOps* store) if (rc != WOLFCERT_OK) return rc; - return store->write(store->ctx, "ca.key.der", ca->key_der, ca->key_der_len, 1); + rc = store->write(store->ctx, "ca.key.der", ca->key_der, ca->key_der_len, 1); + + /* A certificate without its key is a damaged store that every later load + * rejects, so drop the half that landed. */ + if (rc != WOLFCERT_OK && store->remove != NULL) + (void)store->remove(store->ctx, "ca.cert.der"); + + return rc; } void wolfcert_ca_free(WolfCertCa* ca) diff --git a/src/server.c b/src/server.c index 3ebc526..ea36866 100644 --- a/src/server.c +++ b/src/server.c @@ -221,11 +221,20 @@ int wolfcert_server_start(const WolfCertServerCfgSrv* cfg, WolfCertServer** out) } int rc; - if (cfg->ca_store != NULL && - wolfcert_ca_load(&s->ca, cfg->ca_store, heap) == WOLFCERT_OK) { - /* loaded existing CA */ + int have_ca = 0; + + if (cfg->ca_store != NULL) { + rc = wolfcert_ca_load(&s->ca, cfg->ca_store, heap); + if (rc == WOLFCERT_OK) + have_ca = 1; + /* Only an empty store means "no CA yet"; an I/O, memory or parse + * failure must not silently replace a CA the caller still has. + * wolfcert_ca_load() already recorded which one it was. */ + else if (rc != WOLFCERT_ERR_NOT_FOUND) + goto fail; } - else { + + if (!have_ca) { WolfCertKeyType kt = cfg->ca_key_type ? cfg->ca_key_type : WOLFCERT_DEFAULT_KEY_TYPE; int kp = cfg->ca_key_param; @@ -234,8 +243,13 @@ int wolfcert_server_start(const WolfCertServerCfgSrv* cfg, WolfCertServer** out) if (rc != WOLFCERT_OK) goto fail; - if (cfg->ca_store != NULL) - wolfcert_ca_save(&s->ca, cfg->ca_store); + if (cfg->ca_store != NULL) { + rc = wolfcert_ca_save(&s->ca, cfg->ca_store); + if (rc != WOLFCERT_OK) { + WOLFCERT_ERR(rc, "server", "ca_store save failed"); + goto fail; + } + } } rc = ops->start(cfg, s); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index d40688b..ff7e4d7 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -123,6 +123,10 @@ if(WOLFCERT_ENABLE_SCEP AND WOLFCERT_ENABLE_SERVER) add_executable(test_scep_async_roundtrip integration/test_scep_async_roundtrip.c) target_link_libraries(test_scep_async_roundtrip PRIVATE wolfcert Threads::Threads) add_test(NAME scep_async_roundtrip COMMAND test_scep_async_roundtrip) + + add_executable(test_server_ca_store unit/test_server_ca_store.c) + target_link_libraries(test_server_ca_store PRIVATE wolfcert Threads::Threads) + add_test(NAME server_ca_store COMMAND test_server_ca_store) endif() # The CLI's protocol scoping and keyword validation. Most cases need no diff --git a/tests/unit/test_server_ca_store.c b/tests/unit/test_server_ca_store.c new file mode 100644 index 0000000..ad61a5d --- /dev/null +++ b/tests/unit/test_server_ca_store.c @@ -0,0 +1,299 @@ +/* + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfCert. + * + * wolfCert is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfCert is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with wolfCert. If not, see . + */ + +#include +#include +#include "../test_static_mem.h" + +#include +#include + +#define REQUIRE(cond) \ + do { \ + if (!(cond)) { \ + fprintf(stderr, "FAIL %s:%d %s\n", __FILE__, __LINE__, #cond); \ + return 1; \ + } \ + } while (0) + +#ifdef WOLFCERT_HAVE_ECC + #define CA_KEY_TYPE WOLFCERT_KEY_ECC + #define CA_KEY_PARAM 256 +#else + #define CA_KEY_TYPE 0 + #define CA_KEY_PARAM 0 +#endif + +static void ca_store_cfg(WolfCertServerCfgSrv* cfg, WolfCertStoreOps* store) +{ + memset(cfg, 0, sizeof(*cfg)); + cfg->protocol = WOLFCERT_PROTO_SCEP; + cfg->bind_host = "127.0.0.1"; + cfg->ca_store = store; + cfg->ca_key_type = CA_KEY_TYPE; + cfg->ca_key_param = CA_KEY_PARAM; +} + +/* Stub backend: every read reports the configured error, every write the + * configured error, so a start can be driven down one failure path at a time. */ +typedef struct { + int read_rc; + int write_rc; +} StubCtx; + +static int stub_read(void* ctx_, const char* key, WolfCertBuffer* out) +{ + (void)key; + (void)out; + return ((StubCtx*)ctx_)->read_rc; +} + +static int stub_write(void* ctx_, const char* key, const uint8_t* data, + size_t len, int sensitive) +{ + (void)key; + (void)data; + (void)len; + (void)sensitive; + return ((StubCtx*)ctx_)->write_rc; +} + +static int stub_remove(void* ctx_, const char* key) +{ + (void)ctx_; + (void)key; + return WOLFCERT_OK; +} + +static void stub_store(WolfCertStoreOps* ops, StubCtx* ctx) +{ + memset(ops, 0, sizeof(*ops)); + ops->read = stub_read; + ops->write = stub_write; + ops->remove = stub_remove; + ops->ctx = ctx; +} + +static int test_corrupt_ca_rejected(void) +{ + WolfCertStoreOps* store = wolfcert_store_memory_open(NULL); + REQUIRE(store != NULL); + + const uint8_t junk[] = { 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA }; + REQUIRE(store->write(store->ctx, "ca.cert.der", junk, sizeof(junk), 0) == WOLFCERT_OK); + REQUIRE(store->write(store->ctx, "ca.key.der", junk, sizeof(junk), 1) == WOLFCERT_OK); + + WolfCertServerCfgSrv cfg; + WolfCertServer* srv = NULL; + + ca_store_cfg(&cfg, store); + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_ERR_PARSE); + REQUIRE(srv == NULL); + + wolfcert_store_memory_close(store); + return 0; +} + +static int test_load_io_error_rejected(void) +{ + StubCtx ctx = { WOLFCERT_ERR_IO, WOLFCERT_OK }; + WolfCertStoreOps store; + WolfCertServerCfgSrv cfg; + WolfCertServer* srv = NULL; + + stub_store(&store, &ctx); + ca_store_cfg(&cfg, &store); + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_ERR_IO); + REQUIRE(srv == NULL); + return 0; +} + +static int test_save_failure_rejected(void) +{ + StubCtx ctx = { WOLFCERT_ERR_NOT_FOUND, WOLFCERT_ERR_IO }; + WolfCertStoreOps store; + WolfCertServerCfgSrv cfg; + WolfCertServer* srv = NULL; + + stub_store(&store, &ctx); + ca_store_cfg(&cfg, &store); + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_ERR_IO); + REQUIRE(srv == NULL); + return 0; +} + +/* Backend that forwards to a real store but fails the nth write, so a + * bootstrap can be interrupted between the certificate and the key. */ +typedef struct { + WolfCertStoreOps* inner; + int writes; + int fail_at; +} FlakyCtx; + +static int flaky_read(void* ctx_, const char* key, WolfCertBuffer* out) +{ + WolfCertStoreOps* in = ((FlakyCtx*)ctx_)->inner; + + return in->read(in->ctx, key, out); +} + +static int flaky_write(void* ctx_, const char* key, const uint8_t* data, + size_t len, int sensitive) +{ + FlakyCtx* ctx = (FlakyCtx*)ctx_; + + if (++ctx->writes == ctx->fail_at) + return WOLFCERT_ERR_IO; + + return ctx->inner->write(ctx->inner->ctx, key, data, len, sensitive); +} + +static int flaky_remove(void* ctx_, const char* key) +{ + WolfCertStoreOps* in = ((FlakyCtx*)ctx_)->inner; + + return in->remove(in->ctx, key); +} + +/* A key write that fails once the certificate has landed must take the + * certificate with it: a cert-only store is rejected by every later load. */ +static int test_save_rollback(void) +{ + WolfCertStoreOps* mem = wolfcert_store_memory_open(NULL); + FlakyCtx fctx; + WolfCertStoreOps store; + WolfCertServerCfgSrv cfg; + WolfCertServer* srv = NULL; + WolfCertBuffer left = { 0 }; + + REQUIRE(mem != NULL); + + memset(&fctx, 0, sizeof(fctx)); + fctx.inner = mem; + fctx.fail_at = 2; + + memset(&store, 0, sizeof(store)); + store.read = flaky_read; + store.write = flaky_write; + store.remove = flaky_remove; + store.ctx = &fctx; + + ca_store_cfg(&cfg, &store); + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_ERR_IO); + REQUIRE(srv == NULL); + REQUIRE(mem->read(mem->ctx, "ca.cert.der", &left) == WOLFCERT_ERR_NOT_FOUND); + + /* The store is still empty, so the next start bootstraps normally. */ + fctx.fail_at = 0; + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_OK); + wolfcert_server_free(srv); + REQUIRE(mem->read(mem->ctx, "ca.cert.der", &left) == WOLFCERT_OK); + + wolfcert_buffer_free(&left); + wolfcert_store_memory_close(mem); + return 0; +} + +/* A store holding one half of the pair is damaged, not empty: starting + * against it must fail rather than mint a CA over the surviving half. */ +static int partial_store_rejected(const char* present) +{ + WolfCertStoreOps* store = wolfcert_store_memory_open(NULL); + const uint8_t stored[] = { 0x30, 0x03, 0x02, 0x01, 0x01 }; + WolfCertBuffer left = { 0 }; + WolfCertServerCfgSrv cfg; + WolfCertServer* srv = NULL; + + REQUIRE(store != NULL); + REQUIRE(store->write(store->ctx, present, stored, sizeof(stored), 0) + == WOLFCERT_OK); + + ca_store_cfg(&cfg, store); + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_ERR_PARSE); + REQUIRE(srv == NULL); + + /* The survivor is still exactly what was stored. */ + REQUIRE(store->read(store->ctx, present, &left) == WOLFCERT_OK); + REQUIRE(left.len == sizeof(stored)); + REQUIRE(memcmp(left.data, stored, left.len) == 0); + + wolfcert_buffer_free(&left); + wolfcert_store_memory_close(store); + return 0; +} + +static int test_partial_store_rejected(void) +{ + if (partial_store_rejected("ca.cert.der")) + return 1; + + return partial_store_rejected("ca.key.der"); +} + +static int test_ca_persists_across_starts(void) +{ + WolfCertStoreOps* store = wolfcert_store_memory_open(NULL); + REQUIRE(store != NULL); + + WolfCertServerCfgSrv cfg; + WolfCertServer* srv = NULL; + WolfCertBuffer first = { 0 }; + WolfCertBuffer second = { 0 }; + + ca_store_cfg(&cfg, store); + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_OK); + wolfcert_server_free(srv); + srv = NULL; + REQUIRE(store->read(store->ctx, "ca.cert.der", &first) == WOLFCERT_OK); + + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_OK); + wolfcert_server_free(srv); + REQUIRE(store->read(store->ctx, "ca.cert.der", &second) == WOLFCERT_OK); + + REQUIRE(first.len == second.len); + REQUIRE(memcmp(first.data, second.data, first.len) == 0); + + wolfcert_buffer_free(&first); + wolfcert_buffer_free(&second); + wolfcert_store_memory_close(store); + return 0; +} + +int main(void) +{ + REQUIRE(test_static_mem_init() == 0); + REQUIRE(wolfcert_init(NULL) == WOLFCERT_OK); + + if (test_corrupt_ca_rejected()) + return 1; + if (test_load_io_error_rejected()) + return 1; + if (test_save_failure_rejected()) + return 1; + if (test_save_rollback()) + return 1; + if (test_partial_store_rejected()) + return 1; + if (test_ca_persists_across_starts()) + return 1; + + wolfcert_cleanup(); + printf("OK\n"); + return 0; +} diff --git a/wolfcert/server.h b/wolfcert/server.h index 69de315..c4494f8 100644 --- a/wolfcert/server.h +++ b/wolfcert/server.h @@ -40,7 +40,10 @@ typedef struct { uint16_t bind_port; WolfCertStoreOps* ca_store; /* optional: persist the local CA across runs; NULL = regen on - each start */ + each start. A store that fails + to read or write fails the + start rather than falling back + to an ephemeral CA. */ const char* challenge_password; /* SCEP challengePassword to accept; NULL disables */ const char* http_basic_user; /* EST HTTP Basic credentials to accept; NULL disables */ const char* http_basic_pass; From 65e2289b2d06bba0659b2ad439605e47b9ca027f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 9 Sep 2026 16:19:12 +0200 Subject: [PATCH 05/18] Interrupt an idle accepted connection on server stop wolfcert_server_stop only set the stopping flag, which the accept loop checks while parked at poll() on the listener. Once a connection was accepted the flag was not looked at again until the keep-alive loop condition, so a peer that connected and then sent nothing left the serving thread blocked forever in wolfSSL_accept or in the protocol handler's first read, and a caller joining that thread after stop hung. Put a receive timeout on accepted sockets and treat its expiry as resumable: the handshake retries on WANT_READ and wolfcert_io_recv retries on WANT_READ or EAGAIN, both only while shutdown has not been requested. Shutdown latency on an established connection is now bounded by the same cadence as the listener poll, with no cross-thread fd manipulation and so no risk of acting on a recycled descriptor. Those timeouts are the whole of the bound, so a connection whose SO_RCVTIMEO or SO_SNDTIMEO cannot be installed is closed rather than served: without both the handler blocks unbounded again, and with only the send timeout missing a receive expiry would be read as an error and disconnect a healthy idle client. Each idle case in the test waits for the server to reach the blocking site before it stops -- the server's handshake flight, a completed handshake, a served GetCACaps -- so no case can pass with the server still parked at the listener poll. Fixes F-8027. --- .github/workflows/nightly.yml | 4 +- .github/workflows/pr.yml | 5 +- .github/workflows/sanitizers.yml | 4 +- Makefile.am | 5 +- src/internal.h | 6 + src/server.c | 102 ++++++- tests/CMakeLists.txt | 6 + tests/integration/test_server_stop_idle.c | 322 ++++++++++++++++++++++ tests/integration/tls_test_util.h | 59 +++- 9 files changed, 488 insertions(+), 25 deletions(-) create mode 100644 tests/integration/test_server_stop_idle.c diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index a544905..41ed458 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -50,7 +50,9 @@ jobs: - { name: cmake-no-builtin-transport, os: ubuntu-latest, build: cmake, wolfssl: full, cmake_extra: "-DWOLFCERT_ENABLE_BUILTIN_TRANSPORT=OFF -DWOLFCERT_ENABLE_POSIX_STORE=OFF -DWOLFCERT_ENABLE_CLI=OFF -DWOLFCERT_ENABLE_SERVER=OFF" } # ---- Constrained builds (unit tests only: single-thread / no-malloc) ---- - { name: cmake-static-mem, os: ubuntu-latest, build: cmake, wolfssl: static-mem, cmake_extra: -DWOLFCERT_ENABLE_SERVER=OFF, ctest_exclude: "http|tls|roundtrip" } - - { name: cmake-no-malloc, os: ubuntu-latest, build: cmake, wolfssl: no-malloc, ctest_exclude: "http|tls|roundtrip|est" } + # stop_idle is named explicitly: its TLS cases carry no "tls" in the + # test name, and TLS over a static pool is out of scope here. + - { name: cmake-no-malloc, os: ubuntu-latest, build: cmake, wolfssl: no-malloc, ctest_exclude: "http|tls|roundtrip|est|stop_idle" } # ---- macOS reduced subset ---- - { name: cmake-full-macos, os: macos-latest, build: cmake, wolfssl: full } - { name: cmake-nonrsa-macos, os: macos-latest, build: cmake, wolfssl: est-only-nonrsa, cmake_extra: -DWOLFCERT_ENABLE_SCEP=OFF } diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index ebfbf00..ee613a2 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -129,8 +129,9 @@ jobs: wolfssl: no-malloc # Non-TLS crypto/cert/PKCS7 units only: TLS + threads over a static # pool is a separate wolfSSL concern, out of scope for the - # allocation-free validation. - ctest_exclude: "http|tls|roundtrip|est" + # allocation-free validation. stop_idle is named because its TLS + # cases carry no "tls" in the test name. + ctest_exclude: "http|tls|roundtrip|est|stop_idle" # ---- macOS ---- - name: cmake-full-macos os: macos-latest diff --git a/.github/workflows/sanitizers.yml b/.github/workflows/sanitizers.yml index 87de8ac..1ba5319 100644 --- a/.github/workflows/sanitizers.yml +++ b/.github/workflows/sanitizers.yml @@ -88,8 +88,10 @@ jobs: cmake --build build -j "$(nproc)" # The roundtrip tests spin the server on a background pthread while the # client drives it on the main thread -> the real concurrency surface. + # server_stop_idle belongs here too: it stops the server from a second + # thread, which is the one shutdown path nothing else exercises. ctest --test-dir build -j "$(nproc)" --output-on-failure \ - -R 'roundtrip|tls_http' + -R 'roundtrip|tls_http|stop_idle' valgrind: name: valgrind (representative subset) diff --git a/Makefile.am b/Makefile.am index 06ee49d..37c086e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -156,7 +156,8 @@ check_PROGRAMS += test_est_roundtrip test_est_tls_roundtrip test_est_mtls_roundt test_est_pha_roundtrip test_est_async_roundtrip \ test_est_csr_attrs_roundtrip test_est_csr_attrs_apply_roundtrip \ test_est_csr_attrs_enforce test_est_chunked_robustness \ - test_est_pending_roundtrip test_est_mldsa_roundtrip + test_est_pending_roundtrip test_est_mldsa_roundtrip \ + test_server_stop_idle test_est_roundtrip_SOURCES = tests/integration/test_est_roundtrip.c test_est_roundtrip_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread test_est_tls_roundtrip_SOURCES = tests/integration/test_est_tls_roundtrip.c @@ -179,6 +180,8 @@ test_est_chunked_robustness_SOURCES = tests/integration/test_est_chunked_robustn test_est_chunked_robustness_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread test_est_pending_roundtrip_SOURCES = tests/integration/test_est_pending_roundtrip.c test_est_pending_roundtrip_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread +test_server_stop_idle_SOURCES = tests/integration/test_server_stop_idle.c +test_server_stop_idle_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread endif endif diff --git a/src/internal.h b/src/internal.h index 02821df..9d0c269 100644 --- a/src/internal.h +++ b/src/internal.h @@ -197,6 +197,12 @@ struct WolfCertServer { * this after serve_fd and breaks out of the keep-alive loop when * it's zero. */ int keep_alive; + /* Set while the accept loop is serving a connection it armed with + * SO_RCVTIMEO/SO_SNDTIMEO. The would-block retries in + * wolfcert_io_{recv,send} are bounded only on such a connection: an fd + * handed in through wolfcert_server_serve_fd() may be non-blocking and + * has no shutdown flag driving it, so retrying there would spin. */ + int poll_timeouts_armed; void* heap; }; diff --git a/src/server.c b/src/server.c index ea36866..0e3e15d 100644 --- a/src/server.c +++ b/src/server.c @@ -37,36 +37,80 @@ #include #include #include +#include #include #include #include -/* accept() poll cadence: how often wolfcert_server_run() wakes to re-check the - * stopping flag while idle. Bounds shutdown latency; not performance-critical. - */ +/* Shutdown cadence: how often wolfcert_server_run() wakes to re-check the + * stopping flag while idle at the listener, and the send/receive timeouts put + * on an accepted connection so a stalled peer cannot hold the handler. Bounds + * shutdown latency; not performance-critical. */ #ifndef WOLFCERT_SERVER_POLL_MS #define WOLFCERT_SERVER_POLL_MS 200 #endif ssize_t wolfcert_io_recv(WolfCertServer* srv, int fd, void* buf, size_t len) { + ssize_t r; + + /* A connection the accept loop armed carries a receive timeout, so its + * expiry is a retry rather than an error: wolfSSL reports it as WANT_READ, + * a raw socket as EAGAIN. Retrying stops once shutdown is requested. */ if (srv != NULL && srv->tls_current != NULL) { - int r = wolfSSL_read(srv->tls_current, buf, (int)len); - return r <= 0 ? -1 : (ssize_t)r; + int tr; + + do { + tr = wolfSSL_read(srv->tls_current, buf, (int)len); + } + while (tr <= 0 && + wolfSSL_get_error(srv->tls_current, tr) == WOLFSSL_ERROR_WANT_READ && + !WOLFSSL_ATOMIC_LOAD(srv->stopping)); + + return tr <= 0 ? -1 : (ssize_t)tr; } - return recv(fd, buf, len, 0); + do { + r = recv(fd, buf, len, 0); + } + while (r < 0 && srv != NULL && !WOLFSSL_ATOMIC_LOAD(srv->stopping) && + (errno == EINTR || + (srv->poll_timeouts_armed && + (errno == EAGAIN || errno == EWOULDBLOCK)))); + + return r; } ssize_t wolfcert_io_send(WolfCertServer* srv, int fd, const void* buf, size_t len) { + ssize_t r; + + /* Mirrors wolfcert_io_recv: the send timeout bounds a peer that stops + * reading, and its expiry is a retry rather than an error. Callers write + * through send_all(), so a short write is already handled. */ if (srv != NULL && srv->tls_current != NULL) { - int r = wolfSSL_write(srv->tls_current, buf, (int)len); - return r <= 0 ? -1 : (ssize_t)r; + int tr; + + do { + tr = wolfSSL_write(srv->tls_current, buf, (int)len); + } + while (tr <= 0 && + wolfSSL_get_error(srv->tls_current, tr) == WOLFSSL_ERROR_WANT_WRITE && + !WOLFSSL_ATOMIC_LOAD(srv->stopping)); + + return tr <= 0 ? -1 : (ssize_t)tr; } - return send(fd, buf, len, 0); + do { + r = send(fd, buf, len, 0); + } + while (r < 0 && srv != NULL && !WOLFSSL_ATOMIC_LOAD(srv->stopping) && + (errno == EINTR || + (srv->poll_timeouts_armed && + (errno == EAGAIN || errno == EWOULDBLOCK)))); + + return r; } static int tls_setup(WolfCertServer* s, const WolfCertServerCfgSrv* cfg) @@ -293,6 +337,7 @@ int wolfcert_server_start(const WolfCertServerCfgSrv* cfg, WolfCertServer** out) int wolfcert_server_run(WolfCertServer* srv) { struct pollfd pfd; + struct timeval poll_to; int ret; int pr; int cs; @@ -345,6 +390,21 @@ int wolfcert_server_run(WolfCertServer* srv) return WOLFCERT_ERR_IO; } + /* Bound how long a read or write on this connection can block, so a + * peer that goes silent or stops reading cannot hold the handler past + * wolfcert_server_stop(). */ + poll_to.tv_sec = WOLFCERT_SERVER_POLL_MS / 1000; + poll_to.tv_usec = (WOLFCERT_SERVER_POLL_MS % 1000) * 1000; + if (setsockopt(cs, SOL_SOCKET, SO_RCVTIMEO, &poll_to, + sizeof(poll_to)) != 0 || + setsockopt(cs, SOL_SOCKET, SO_SNDTIMEO, &poll_to, + sizeof(poll_to)) != 0) { + close(cs); + continue; + } + + srv->poll_timeouts_armed = 1; + if (srv->tls_ctx != NULL) { /* Terminate TLS on this accepted fd. The protocol handler sees * plaintext HTTP through wolfcert_io_{recv,send}. */ @@ -352,7 +412,17 @@ int wolfcert_server_run(WolfCertServer* srv) if (ssl != NULL) { wolfSSL_set_fd(ssl, cs); - if ((ret = wolfSSL_accept(ssl)) == WOLFSSL_SUCCESS) { + /* A timed-out handshake read surfaces as WANT_READ, which + * is resumable: keep going until it completes, genuinely + * fails, or shutdown is requested. */ + do { + ret = wolfSSL_accept(ssl); + } + while (ret != WOLFSSL_SUCCESS && + wolfSSL_get_error(ssl, ret) == WOLFSSL_ERROR_WANT_READ && + !WOLFSSL_ATOMIC_LOAD(srv->stopping)); + + if (ret == WOLFSSL_SUCCESS) { srv->tls_current = ssl; /* Keep-alive loop: protocol handlers read one @@ -389,6 +459,7 @@ int wolfcert_server_run(WolfCertServer* srv) while (srv->keep_alive && !WOLFSSL_ATOMIC_LOAD(srv->stopping)); } + srv->poll_timeouts_armed = 0; close(cs); } @@ -408,11 +479,12 @@ int wolfcert_server_stop(WolfCertServer* srv) if (srv == NULL) return WOLFCERT_ERR_BAD_ARG; - /* Signal the accept loop to exit. It polls the listener on a short timeout - * (WOLFCERT_SERVER_POLL_MS) and re-checks this flag, so no fd surgery is - * needed here -- wolfcert_server_free() closes listen_fd after the serving - * thread is joined. Setting the flag from another thread (test harness) or - * a signal handler (wolfcert-server CLI) is safe: the store is atomic. */ + /* Signal the accept loop to exit. Both the listener poll and the reads on + * an accepted connection use a WOLFCERT_SERVER_POLL_MS timeout and + * re-check this flag, so no fd surgery is needed here -- + * wolfcert_server_free() closes listen_fd after the serving thread is + * joined. Setting the flag from another thread (test harness) or a signal + * handler (wolfcert-server CLI) is safe: the store is atomic. */ WOLFSSL_ATOMIC_STORE(srv->stopping, 1); return WOLFCERT_OK; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index ff7e4d7..b57c505 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -100,6 +100,12 @@ if(WOLFCERT_ENABLE_EST) add_executable(test_est_mldsa_roundtrip integration/test_est_mldsa_roundtrip.c) target_link_libraries(test_est_mldsa_roundtrip PRIVATE wolfcert Threads::Threads) add_test(NAME est_mldsa_roundtrip COMMAND test_est_mldsa_roundtrip) + + add_executable(test_server_stop_idle integration/test_server_stop_idle.c) + target_link_libraries(test_server_stop_idle PRIVATE wolfcert Threads::Threads) + add_test(NAME server_stop_idle COMMAND test_server_stop_idle) + # A stop that cannot interrupt an idle peer hangs the serving thread. + set_tests_properties(server_stop_idle PROPERTIES TIMEOUT 60) endif() endif() diff --git a/tests/integration/test_server_stop_idle.c b/tests/integration/test_server_stop_idle.c new file mode 100644 index 0000000..812095e --- /dev/null +++ b/tests/integration/test_server_stop_idle.c @@ -0,0 +1,322 @@ +/* + * Copyright (C) 2026 wolfSSL Inc. + * + * This file is part of wolfCert. + * + * wolfCert is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * wolfCert is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with wolfCert. If not, see . + */ + +/* + * Shutdown coverage for the accept loop in src/server.c: a peer that + * connects and then sends nothing must not pin the serving thread past + * wolfcert_server_stop(). + * + * Three idle points are exercised, each entered only once the peer has seen + * the server reach it, so no case can pass without the server parked: + * 1. A ClientHello answered by the server's flight, so the server is parked + * in wolfSSL_accept() waiting for the rest of the handshake. + * 2. A completed TLS handshake with no request bytes, so the server is + * parked in the protocol handler's read. + * 3. A served GetCACaps on a plaintext SCEP listener, so the keep-alive + * loop is parked in recv() on the next request. + * + * In each case the test calls wolfcert_server_stop() and requires + * wolfcert_server_run() to return inside a bounded wait; a thread still + * running at the deadline cannot be joined, so the test reports the + * failure and exits immediately rather than hanging. + * + * A fourth case covers the other entry point: wolfcert_server_serve_fd() + * runs on a caller-supplied fd that the accept loop never armed, so a + * would-block read there must fail instead of retrying forever. + */ + +#define _POSIX_C_SOURCE 200809L +#define _DEFAULT_SOURCE +#define _DARWIN_C_SOURCE +#define _GNU_SOURCE + +#include +#include + +#include "tls_test_util.h" + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define REQUIRE(cond) \ + do { \ + if (!(cond)) { \ + fprintf(stderr, "FAIL %s:%d %s\n", __FILE__, __LINE__, #cond); \ + return 1; \ + } \ + } while (0) + +/* How long wolfcert_server_run() gets to return after stop(), and the + * granularity the test polls at. */ +#define STOP_DEADLINE_MS 5000 +#define POLL_STEP_MS 10 + +typedef struct { + WolfCertServer* srv; + /* Atomic, not volatile: the poll below needs a happens-before edge + * against the serving thread, the same way srv->stopping does. */ + wolfSSL_Atomic_Int returned; +} ServerCtx; + +static void* server_thread(void* arg) +{ + ServerCtx* ctx = (ServerCtx*)arg; + + wolfcert_server_run(ctx->srv); + WOLFSSL_ATOMIC_STORE(ctx->returned, 1); + + return NULL; +} + +static void sleep_ms(int ms) +{ + struct timespec ts; + + ts.tv_sec = ms / 1000; + ts.tv_nsec = (long)(ms % 1000) * 1000000L; + nanosleep(&ts, NULL); +} + +#ifdef WOLFCERT_HAVE_SCEP +/* Connect to 127.0.0.1:port. Returns the fd, or -1. */ +static int connect_loopback(uint16_t port) +{ + struct sockaddr_in sa; + int fd; + + fd = socket(AF_INET, SOCK_STREAM, 0); + if (fd < 0) + return -1; + + memset(&sa, 0, sizeof(sa)); + sa.sin_family = AF_INET; + sa.sin_port = htons(port); + if (inet_pton(AF_INET, "127.0.0.1", &sa.sin_addr) != 1 || + connect(fd, (struct sockaddr*)&sa, sizeof(sa)) < 0) { + close(fd); + return -1; + } + + return fd; +} + +/* Drive one plaintext GetCACaps to completion, so the reply proves the handler + * ran and the keep-alive loop is now parked reading the next request. */ +static int connect_after_getcacaps(uint16_t port) +{ + static const char req[] = + "GET /?operation=GetCACaps HTTP/1.1\r\n" + "Host: 127.0.0.1\r\n" + "\r\n"; + struct timeval to; + char buf[64]; + int fd; + + fd = connect_loopback(port); + if (fd < 0) + return -1; + + to.tv_sec = STOP_DEADLINE_MS / 1000; + to.tv_usec = 0; + if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &to, sizeof(to)) != 0 || + send(fd, req, sizeof(req) - 1, 0) != (ssize_t)(sizeof(req) - 1) || + recv(fd, buf, sizeof(buf), 0) <= 0) { + close(fd); + return -1; + } + + return fd; +} +#endif /* WOLFCERT_HAVE_SCEP */ + +/* Stop the server and wait for its thread to leave wolfcert_server_run(). + * Returns 0 when it did, -1 on the deadline. */ +static int stop_and_wait(ServerCtx* ctx) +{ + int waited; + + if (wolfcert_server_stop(ctx->srv) != WOLFCERT_OK) + return -1; + + for (waited = 0; waited < STOP_DEADLINE_MS; waited += POLL_STEP_MS) { + if (WOLFSSL_ATOMIC_LOAD(ctx->returned)) + return 0; + + sleep_ms(POLL_STEP_MS); + } + + return -1; +} + +static int fail_stuck(const char* which) +{ + fprintf(stderr, "FAIL %s: did not return within %d ms\n", + which, STOP_DEADLINE_MS); + fflush(stderr); + + /* The serving thread is still blocked, so it cannot be joined. */ + _exit(1); +} + +#ifdef WOLFCERT_HAVE_SCEP +typedef struct { + WolfCertServer* srv; + int fd; + wolfSSL_Atomic_Int returned; +} ServeFdCtx; + +static void* serve_fd_thread(void* arg) +{ + ServeFdCtx* ctx = (ServeFdCtx*)arg; + + (void)wolfcert_server_serve_fd(ctx->srv, ctx->fd); + WOLFSSL_ATOMIC_STORE(ctx->returned, 1); + + return NULL; +} +#endif + +int main(void) +{ + uint8_t* tls_cert = NULL; + size_t tls_cert_len = 0; + uint8_t* tls_key = NULL; + size_t tls_key_len = 0; + WolfCertServerCfgSrv cfg; + ServerCtx ctx; + pthread_t tid; + TestTlsConn conn; +#ifdef WOLFCERT_HAVE_SCEP + ServeFdCtx serve_ctx; + int fd; + int sp[2]; + int waited; +#endif + + REQUIRE(wolfcert_init(NULL) == WOLFCERT_OK); + REQUIRE(gen_server_identity(&tls_cert, &tls_cert_len, + &tls_key, &tls_key_len) == 0); + + memset(&cfg, 0, sizeof(cfg)); + cfg.protocol = WOLFCERT_PROTO_EST; + cfg.bind_host = "127.0.0.1"; + cfg.bind_port = 0; + cfg.tls_cert_pem = tls_cert; + cfg.tls_cert_pem_len = tls_cert_len; + cfg.tls_key_pem = tls_key; + cfg.tls_key_pem_len = tls_key_len; + + /* 1. Parked in wolfSSL_accept(): TCP is up, no ClientHello follows. */ + memset(&ctx, 0, sizeof(ctx)); + REQUIRE(wolfcert_server_start(&cfg, &ctx.srv) == WOLFCERT_OK); + REQUIRE(pthread_create(&tid, NULL, server_thread, &ctx) == 0); + + REQUIRE(test_tls_connect_partial(&conn, wolfcert_server_port(ctx.srv), + tls_cert, tls_cert_len, + STOP_DEADLINE_MS) == 0); + + if (stop_and_wait(&ctx) != 0) + return fail_stuck("idle before handshake"); + + REQUIRE(pthread_join(tid, NULL) == 0); + test_tls_close(&conn); + wolfcert_server_free(ctx.srv); + + /* 2. Parked in the handler's read: handshake done, no request bytes. */ + memset(&ctx, 0, sizeof(ctx)); + REQUIRE(wolfcert_server_start(&cfg, &ctx.srv) == WOLFCERT_OK); + REQUIRE(pthread_create(&tid, NULL, server_thread, &ctx) == 0); + + REQUIRE(test_tls_connect(&conn, wolfcert_server_port(ctx.srv), + tls_cert, tls_cert_len) == 0); + + if (stop_and_wait(&ctx) != 0) + return fail_stuck("idle after handshake"); + + REQUIRE(pthread_join(tid, NULL) == 0); + test_tls_close(&conn); + wolfcert_server_free(ctx.srv); + +#ifdef WOLFCERT_HAVE_SCEP + /* 3. Parked in recv() on a plaintext listener. */ + memset(&cfg, 0, sizeof(cfg)); + cfg.protocol = WOLFCERT_PROTO_SCEP; + cfg.bind_host = "127.0.0.1"; + cfg.bind_port = 0; + + memset(&ctx, 0, sizeof(ctx)); + REQUIRE(wolfcert_server_start(&cfg, &ctx.srv) == WOLFCERT_OK); + REQUIRE(pthread_create(&tid, NULL, server_thread, &ctx) == 0); + + fd = connect_after_getcacaps(wolfcert_server_port(ctx.srv)); + REQUIRE(fd >= 0); + + if (stop_and_wait(&ctx) != 0) + return fail_stuck("idle on plaintext listener"); + + REQUIRE(pthread_join(tid, NULL) == 0); + close(fd); + wolfcert_server_free(ctx.srv); + + /* 4. serve_fd() on a non-blocking fd the accept loop never armed: the + * read must surface the error instead of spinning on EAGAIN. */ + memset(&ctx, 0, sizeof(ctx)); + REQUIRE(wolfcert_server_start(&cfg, &ctx.srv) == WOLFCERT_OK); + + REQUIRE(socketpair(AF_UNIX, SOCK_STREAM, 0, sp) == 0); + REQUIRE(fcntl(sp[0], F_SETFL, O_NONBLOCK) == 0); + + memset(&serve_ctx, 0, sizeof(serve_ctx)); + serve_ctx.srv = ctx.srv; + serve_ctx.fd = sp[0]; + REQUIRE(pthread_create(&tid, NULL, serve_fd_thread, &serve_ctx) == 0); + + for (waited = 0; waited < STOP_DEADLINE_MS && + !WOLFSSL_ATOMIC_LOAD(serve_ctx.returned); + waited += POLL_STEP_MS) { + sleep_ms(POLL_STEP_MS); + } + + if (!WOLFSSL_ATOMIC_LOAD(serve_ctx.returned)) + return fail_stuck("serve_fd on a non-blocking fd"); + + REQUIRE(pthread_join(tid, NULL) == 0); + close(sp[0]); + close(sp[1]); + wolfcert_server_free(ctx.srv); +#endif + + free(tls_cert); + free(tls_key); + wolfcert_cleanup(); + + printf("server stop idle: OK\n"); + return 0; +} diff --git a/tests/integration/tls_test_util.h b/tests/integration/tls_test_util.h index b3181e7..fd96c7a 100644 --- a/tests/integration/tls_test_util.h +++ b/tests/integration/tls_test_util.h @@ -43,7 +43,9 @@ #include #include +#include #include +#include #include #include #include @@ -242,10 +244,10 @@ static inline void test_tls_close(TestTlsConn* c) } } -/* Connect to 127.0.0.1:port and handshake, pinning `ca_pem` as the sole trust - * anchor. Returns 0 on success; the caller closes with test_tls_close(). */ -static inline int test_tls_connect(TestTlsConn* c, uint16_t port, - const uint8_t* ca_pem, size_t ca_pem_len) +/* Everything up to the handshake: TCP is connected and the WOLFSSL is bound to + * the socket, with `ca_pem` pinned as the sole trust anchor. */ +static inline int test_tls_setup(TestTlsConn* c, uint16_t port, + const uint8_t* ca_pem, size_t ca_pem_len) { struct sockaddr_in sa = { 0 }; @@ -281,7 +283,54 @@ static inline int test_tls_connect(TestTlsConn* c, uint16_t port, if (wolfSSL_set_fd(c->ssl, c->fd) != WOLFSSL_SUCCESS) goto fail; - if (wolfSSL_connect(c->ssl) != WOLFSSL_SUCCESS) + return 0; +fail: + test_tls_close(c); + return -1; +} + +/* Connect to 127.0.0.1:port and handshake, pinning `ca_pem` as the sole trust + * anchor. Returns 0 on success; the caller closes with test_tls_close(). */ +static inline int test_tls_connect(TestTlsConn* c, uint16_t port, + const uint8_t* ca_pem, size_t ca_pem_len) +{ + if (test_tls_setup(c, port, ca_pem, ca_pem_len) != 0) + return -1; + + if (wolfSSL_connect(c->ssl) != WOLFSSL_SUCCESS) { + test_tls_close(c); + return -1; + } + + return 0; +} + +/* Send the ClientHello and stop there, returning once the server's flight has + * arrived -- proof that the server is inside wolfSSL_accept() awaiting the + * rest of the handshake. Returns 0 on success, -1 on error or `timeout_ms`. */ +static inline int test_tls_connect_partial(TestTlsConn* c, uint16_t port, + const uint8_t* ca_pem, + size_t ca_pem_len, int timeout_ms) +{ + struct pollfd pfd; + int flags; + int ret; + + if (test_tls_setup(c, port, ca_pem, ca_pem_len) != 0) + return -1; + + flags = fcntl(c->fd, F_GETFL, 0); + if (flags < 0 || fcntl(c->fd, F_SETFL, flags | O_NONBLOCK) < 0) + goto fail; + + ret = wolfSSL_connect(c->ssl); + if (ret == WOLFSSL_SUCCESS || + wolfSSL_get_error(c->ssl, ret) != WOLFSSL_ERROR_WANT_READ) + goto fail; + + pfd.fd = c->fd; + pfd.events = POLLIN; + if (poll(&pfd, 1, timeout_ms) != 1 || (pfd.revents & POLLIN) == 0) goto fail; return 0; From 308cc04725ecb474aba44f8f3642b68df0ce958a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 9 Sep 2026 16:41:32 +0200 Subject: [PATCH 06/18] Verify the stored CA certificate matches the stored CA key wolfcert_ca_load only checked that the stored private key could be decoded by one of the registered algorithms. It never parsed the stored certificate and never confirmed the certificate carried the public half of that key, so a store holding an unrelated pair started a server whose issued certificates and PKCS#7 replies were signed by a key the advertised CA certificate did not match, and whose SCEP requests could not be decrypted. The certificate is now parsed before either buffer is adopted, its key algorithm has to agree with the decoded private key, and a new pub_check entry in the key algorithm table compares the two public keys. The comparison lives behind the vtable because DecodedCert.publicKey is framed differently per algorithm: a bare RSAPublicKey for RSA, a full SubjectPublicKeyInfo for ECC, and raw public key bytes for Ed25519, Ed448 and ML-DSA. Every failure exit wipes the stored key DER before releasing it, as wolfcert_ca_free does for the copy it adopts, and the test's key-type list enumerates every enabled ML-DSA parameter set rather than only the first. The five per-algorithm pub_check hooks are not a reimplementation of wolfSSL_X509_check_private_key() by oversight. That call is exported and covers all five algorithms through wc_CheckPrivateKey(), but reaching it means building an X509 and an EVP_PKEY through the compatibility layer, and it is a pure check: the Ed25519, Ed448 and ML-DSA hooks here import the verified public half into the key, which a reloaded PKCS#8 v1 private key has no other way to obtain. Fixes F-9771. --- Makefile.am | 1 + src/ca_issue.c | 64 +++++- src/internal.h | 10 +- src/key_algs.c | 193 ++++++++++++++++++ src/key_algs.h | 5 + tests/CMakeLists.txt | 1 + tests/unit/test_server_ca_store.c | 313 ++++++++++++++++++++++++++++++ 7 files changed, 581 insertions(+), 6 deletions(-) diff --git a/Makefile.am b/Makefile.am index 37c086e..54305b3 100644 --- a/Makefile.am +++ b/Makefile.am @@ -201,6 +201,7 @@ test_scep_poll_roundtrip_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread test_scep_async_roundtrip_SOURCES = tests/integration/test_scep_async_roundtrip.c test_scep_async_roundtrip_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread test_server_ca_store_SOURCES = tests/unit/test_server_ca_store.c +test_server_ca_store_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/src test_server_ca_store_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread endif endif diff --git a/src/ca_issue.c b/src/ca_issue.c index 2540385..31c9a21 100644 --- a/src/ca_issue.c +++ b/src/ca_issue.c @@ -213,6 +213,58 @@ int wolfcert_ca_generate(WolfCertCa* ca, WolfCertKeyType type, int param, void* return WOLFCERT_OK; } +/* Confirm the stored certificate is a CA and carries the public half of the + * stored private key. A mismatched pair would otherwise start a server whose + * signatures and PKCS#7 decryption do not match the CA it advertises. */ +static int ca_check_stored_pair(const WolfCertKeyAlg* alg, WolfCertKey* key, + const uint8_t* cert_der, size_t cert_len, + void* heap) +{ + DecodedCert* dc = (DecodedCert*)WOLFCERT_XMALLOC(sizeof(*dc), heap); + if (dc == NULL) + return WOLFCERT_ERR_MEMORY; + + wc_InitDecodedCert(dc, cert_der, (word32)cert_len, heap); + + int rc = wc_ParseCert(dc, CERT_TYPE, NO_VERIFY, NULL); + if (rc != 0) { + rc = WOLFCERT_ERR(WOLFCERT_ERR_PARSE, "ca", + "stored CA certificate does not parse"); + } + else if (!dc->isCA || + (dc->extKeyUsageSet && (dc->extKeyUsage & KEYUSE_KEY_CERT_SIGN) == 0)) { + /* Signing with a leaf produces a chain no relying party accepts, and + * /cacerts would advertise it as the trust anchor. */ + rc = WOLFCERT_ERR(WOLFCERT_ERR_PARSE, "ca", + "stored CA certificate is not a CA " + "(basicConstraints/keyUsage)"); + } + else if (dc->keyOID != (word32)alg->key_oid || dc->publicKey == NULL) { + rc = WOLFCERT_ERR(WOLFCERT_ERR_PARSE, "ca", + "stored CA certificate and key use different algorithms"); + } + else { + rc = alg->pub_check(key, dc->publicKey, dc->pubKeySize); + if (rc != WOLFCERT_OK) + rc = WOLFCERT_ERR(rc, "ca", + "stored CA certificate does not match the stored key"); + } + + wc_FreeDecodedCert(dc); + WOLFCERT_XFREE(dc, heap); + return rc; +} + +/* The stored bytes are the CA private key, so every exit wipes them before + * releasing the buffer, as wolfcert_ca_free() does for the loaded copy. */ +static void ca_key_buf_free(WolfCertBuffer* key_buf) +{ + if (key_buf->data != NULL && key_buf->len > 0) + wc_ForceZero(key_buf->data, (word32)key_buf->len); + + wolfcert_buffer_free(key_buf); +} + int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, void* heap) { if (ca == NULL || store == NULL) @@ -230,7 +282,7 @@ int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, void* heap) if (cert_rc != WOLFCERT_OK || key_rc != WOLFCERT_OK) { wolfcert_buffer_free(&cert_buf); - wolfcert_buffer_free(&key_buf); + ca_key_buf_free(&key_buf); if (cert_rc == WOLFCERT_ERR_NOT_FOUND && key_rc == WOLFCERT_ERR_NOT_FOUND) return WOLFCERT_ERR_NOT_FOUND; @@ -263,6 +315,14 @@ int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, void* heap) continue; if (a->priv_decode(&shim, key_buf.data, (word32)key_buf.len) == WOLFCERT_OK) { + rc = ca_check_stored_pair(a, &shim, cert_buf.data, cert_buf.len, heap); + if (rc != WOLFCERT_OK) { + a->free_(&shim); + wolfcert_buffer_free(&cert_buf); + ca_key_buf_free(&key_buf); + return rc; + } + ca->type = a->type; ca->impl = shim.impl; ca->cert_der = cert_buf.data; @@ -277,7 +337,7 @@ int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, void* heap) } wolfcert_buffer_free(&cert_buf); - wolfcert_buffer_free(&key_buf); + ca_key_buf_free(&key_buf); return WOLFCERT_ERR(WOLFCERT_ERR_PARSE, "ca", "stored CA key does not decode as any supported algorithm"); } diff --git a/src/internal.h b/src/internal.h index 9d0c269..57866aa 100644 --- a/src/internal.h +++ b/src/internal.h @@ -148,11 +148,13 @@ typedef struct { } WolfCertCa; int wolfcert_ca_generate(WolfCertCa* ca, WolfCertKeyType type, int param, void* heap); -int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, void* heap); +WOLFCERT_TEST_VIS int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, + void* heap); int wolfcert_ca_save(const WolfCertCa* ca, WolfCertStoreOps* store); -void wolfcert_ca_free(WolfCertCa* ca); -int wolfcert_ca_issue (WolfCertCa* ca, const uint8_t* csr_der, size_t csr_len, - uint8_t** out_cert, size_t* out_len); +WOLFCERT_TEST_VIS void wolfcert_ca_free(WolfCertCa* ca); +WOLFCERT_TEST_VIS int wolfcert_ca_issue(WolfCertCa* ca, const uint8_t* csr_der, + size_t csr_len, uint8_t** out_cert, + size_t* out_len); /* ---- server vtable ------------------------------------------------------ */ diff --git a/src/key_algs.c b/src/key_algs.c index bff25e7..66753f8 100644 --- a/src/key_algs.c +++ b/src/key_algs.c @@ -86,6 +86,69 @@ static int rsa_priv_to_der(const struct WolfCertKey* k, uint8_t* buf, word32 cap return wc_RsaKeyToDer((RsaKey*)k->impl, buf, cap); } +static int rsa_pub_check(struct WolfCertKey* k, const uint8_t* pub, + word32 pub_len) +{ + RsaKey* cert_key; + uint8_t* buf = NULL; + word32 idx = 0; + int cap, mine, theirs; + int rc; + + cert_key = (RsaKey*)WOLFCERT_XMALLOC(sizeof(*cert_key), k->heap); + if (cert_key == NULL) + return WOLFCERT_ERR_MEMORY; + + rc = wc_InitRsaKey_ex(cert_key, k->heap, k->dev_id); + if (rc != 0) { + WOLFCERT_XFREE(cert_key, k->heap); + return WOLFCERT_ERR_WC(rc, "keygen", "InitRsaKey_ex"); + } + + rc = wc_RsaPublicKeyDecode(pub, &idx, cert_key, pub_len); + if (rc != 0) { + rc = WOLFCERT_ERR_PARSE; + goto out; + } + + /* Size the scratch from the keys themselves: a fixed cap would report an + * oversized stored CA as a crypto failure. */ + mine = wc_RsaPublicKeyDerSize((RsaKey*)k->impl, 0); + theirs = wc_RsaPublicKeyDerSize(cert_key, 0); + if (mine <= 0 || theirs <= 0) { + rc = WOLFCERT_ERR_CRYPTO; + goto out; + } + if (mine != theirs) { + rc = WOLFCERT_ERR_PARSE; + goto out; + } + + cap = mine; + buf = (uint8_t*)WOLFCERT_XMALLOC(2 * (size_t)cap, k->heap); + if (buf == NULL) { + rc = WOLFCERT_ERR_MEMORY; + goto out; + } + + /* Both sides go through the same encoder so the comparison does not + * depend on how the certificate framed its public key. */ + mine = wc_RsaKeyToPublicDer_ex((RsaKey*)k->impl, buf, (word32)cap, 0); + theirs = wc_RsaKeyToPublicDer_ex(cert_key, buf + cap, (word32)cap, 0); + if (mine <= 0 || theirs <= 0) + rc = WOLFCERT_ERR_CRYPTO; + else if (mine != theirs || memcmp(buf, buf + cap, (size_t)mine) != 0) + rc = WOLFCERT_ERR_PARSE; + else + rc = WOLFCERT_OK; + +out: + wc_FreeRsaKey(cert_key); + WOLFCERT_XFREE(buf, k->heap); + WOLFCERT_XFREE(cert_key, k->heap); + return rc; +} + static void rsa_free(struct WolfCertKey* k) { if (k->impl == NULL) @@ -146,6 +209,57 @@ static int ecc_priv_to_der(const struct WolfCertKey* k, uint8_t* buf, word32 cap return wc_EccKeyToDer((ecc_key*)k->impl, buf, cap); } +/* Uncompressed X9.63 point: the 0x04 marker plus two coordinates. */ +#define ECC_X963_CAP (1 + 2 * MAX_ECC_BYTES) + +static int ecc_pub_check(struct WolfCertKey* k, const uint8_t* pub, + word32 pub_len) +{ + ecc_key* cert_key; + byte mine[ECC_X963_CAP]; + byte theirs[ECC_X963_CAP]; + word32 mine_len = sizeof(mine); + word32 theirs_len = sizeof(theirs); + word32 idx = 0; + int rc; + + cert_key = (ecc_key*)WOLFCERT_XMALLOC(sizeof(*cert_key), k->heap); + if (cert_key == NULL) + return WOLFCERT_ERR_MEMORY; + + rc = wc_ecc_init_ex(cert_key, k->heap, k->dev_id); + if (rc != 0) { + WOLFCERT_XFREE(cert_key, k->heap); + return WOLFCERT_ERR_WC(rc, "keygen", "ecc_init_ex"); + } + + /* A SEC1 private key need not carry its public point, so derive it when + * the decoder did not supply one. */ + if (((ecc_key*)k->impl)->type == ECC_PRIVATEKEY_ONLY) + (void)wc_ecc_make_pub((ecc_key*)k->impl, NULL); + + rc = wc_EccPublicKeyDecode(pub, &idx, cert_key, pub_len); + if (rc != 0) + rc = wc_ecc_import_x963(pub, pub_len, cert_key); + if (rc != 0) { + rc = WOLFCERT_ERR_PARSE; + goto out; + } + + if (wc_ecc_export_x963((ecc_key*)k->impl, mine, &mine_len) != 0 || + wc_ecc_export_x963(cert_key, theirs, &theirs_len) != 0) + rc = WOLFCERT_ERR_CRYPTO; + else if (mine_len != theirs_len || memcmp(mine, theirs, mine_len) != 0) + rc = WOLFCERT_ERR_PARSE; + else + rc = WOLFCERT_OK; + +out: + wc_ecc_free(cert_key); + WOLFCERT_XFREE(cert_key, k->heap); + return rc; +} + static void ecc_free(struct WolfCertKey* k) { if (k->impl == NULL) @@ -199,6 +313,31 @@ static int ed25519_priv_to_der(const struct WolfCertKey* k, uint8_t* buf, word32 return wc_Ed25519PrivateKeyToDer((ed25519_key*)k->impl, buf, cap); } +static int ed25519_pub_check(struct WolfCertKey* k, const uint8_t* pub, + word32 pub_len) +{ + byte mine[ED25519_PUB_KEY_SIZE]; + int rc; + + if (pub_len != sizeof(mine)) + return WOLFCERT_ERR_PARSE; + + rc = wc_ed25519_make_public((ed25519_key*)k->impl, mine, sizeof(mine)); + if (rc != 0) + return WOLFCERT_ERR_WC(rc, "keygen", "ed25519_make_public"); + + if (memcmp(mine, pub, sizeof(mine)) != 0) + return WOLFCERT_ERR_PARSE; + + /* On wolfSSL 5.9.2 make_public only sets pubKeySet, leaving key->p empty + * while wc_ed25519_sign_msg() hashes it -- so import the half we just + * verified. Newer wolfSSL stores it itself and this is a no-op. */ + rc = wc_ed25519_import_public(mine, sizeof(mine), (ed25519_key*)k->impl); + + return rc == 0 ? WOLFCERT_OK + : WOLFCERT_ERR_WC(rc, "keygen", "ed25519_import_public"); +} + static void ed25519_free(struct WolfCertKey* k) { if (k->impl == NULL) @@ -249,6 +388,32 @@ static int ed448_priv_to_der(const struct WolfCertKey* k, uint8_t* buf, word32 c return wc_Ed448PrivateKeyToDer((ed448_key*)k->impl, buf, cap); } +static int ed448_pub_check(struct WolfCertKey* k, const uint8_t* pub, + word32 pub_len) +{ + byte mine[ED448_PUB_KEY_SIZE]; + int rc; + + if (pub_len != sizeof(mine)) + return WOLFCERT_ERR_PARSE; + + rc = wc_ed448_make_public((ed448_key*)k->impl, mine, sizeof(mine)); + if (rc != 0) + return WOLFCERT_ERR_WC(rc, "keygen", "ed448_make_public"); + + if (memcmp(mine, pub, sizeof(mine)) != 0) + return WOLFCERT_ERR_PARSE; + + /* wc_ed448_make_public() sets pubKeySet but, unlike its Ed25519 + * counterpart, leaves key->p untouched -- and wc_ed448_sign_msg() gates + * on the flag while hashing key->p. Import the half we just verified, or + * every certificate this CA issues is signed over an all-zero key. */ + rc = wc_ed448_import_public(mine, sizeof(mine), (ed448_key*)k->impl); + + return rc == 0 ? WOLFCERT_OK + : WOLFCERT_ERR_WC(rc, "keygen", "ed448_import_public"); +} + static void ed448_free(struct WolfCertKey* k) { if (k->impl == NULL) @@ -327,6 +492,27 @@ static int mldsa_priv_to_der(const struct WolfCertKey* k, uint8_t* buf, word32 c return wc_MlDsaKey_PrivateKeyToDer((MlDsaKey*)k->impl, buf, cap); } +/* Reloading an ML-DSA CA from a store needs wc_MlDsaKey_CheckKey(); say so + * here rather than letting the link fail. */ +#ifndef WOLFSSL_MLDSA_CHECK_KEY +#error "wolfCert's ML-DSA support needs wc_MlDsaKey_CheckKey(): rebuild wolfSSL " \ + "without WOLFSSL_DILITHIUM_NO_CHECK_KEY / WOLFSSL_MLDSA_VERIFY_ONLY." +#endif + +/* Unlike its siblings this hook mutates `key`: the certificate's public half + * is adopted into it, since none can be derived from a PKCS#8 v1 private key. + * A key that fails the check therefore carries an unverified public half and + * must be discarded -- wolfcert_ca_load() frees the shim on any failure. */ +static int mldsa_pub_check(struct WolfCertKey* k, const uint8_t* pub, + word32 pub_len) +{ + if (wc_MlDsaKey_ImportPubRaw((MlDsaKey*)k->impl, pub, pub_len) != 0) + return WOLFCERT_ERR_PARSE; + + return wc_MlDsaKey_CheckKey((MlDsaKey*)k->impl) == 0 ? WOLFCERT_OK + : WOLFCERT_ERR_PARSE; +} + static void mldsa_free(struct WolfCertKey* k) { if (k->impl == NULL) @@ -352,6 +538,7 @@ static const WolfCertKeyAlg ALG_RSA = { .make = rsa_make, .priv_decode = rsa_priv_decode, .priv_to_der = rsa_priv_to_der, + .pub_check = rsa_pub_check, .free_ = rsa_free, }; #endif @@ -368,6 +555,7 @@ static const WolfCertKeyAlg ALG_ECC = { .make = ecc_make, .priv_decode = ecc_priv_decode, .priv_to_der = ecc_priv_to_der, + .pub_check = ecc_pub_check, .free_ = ecc_free, }; #endif @@ -384,6 +572,7 @@ static const WolfCertKeyAlg ALG_ED25519 = { .make = ed25519_make, .priv_decode = ed25519_priv_decode, .priv_to_der = ed25519_priv_to_der, + .pub_check = ed25519_pub_check, .free_ = ed25519_free, }; #endif @@ -400,6 +589,7 @@ static const WolfCertKeyAlg ALG_ED448 = { .make = ed448_make, .priv_decode = ed448_priv_decode, .priv_to_der = ed448_priv_to_der, + .pub_check = ed448_pub_check, .free_ = ed448_free, }; #endif @@ -421,6 +611,7 @@ static const WolfCertKeyAlg ALG_MLDSA44 = { .make = mldsa_make, .priv_decode = mldsa_priv_decode, .priv_to_der = mldsa_priv_to_der, + .pub_check = mldsa_pub_check, .free_ = mldsa_free, }; #endif @@ -437,6 +628,7 @@ static const WolfCertKeyAlg ALG_MLDSA65 = { .make = mldsa_make, .priv_decode = mldsa_priv_decode, .priv_to_der = mldsa_priv_to_der, + .pub_check = mldsa_pub_check, .free_ = mldsa_free, }; #endif @@ -453,6 +645,7 @@ static const WolfCertKeyAlg ALG_MLDSA87 = { .make = mldsa_make, .priv_decode = mldsa_priv_decode, .priv_to_der = mldsa_priv_to_der, + .pub_check = mldsa_pub_check, .free_ = mldsa_free, }; #endif diff --git a/src/key_algs.h b/src/key_algs.h index ca20e98..cf387c4 100644 --- a/src/key_algs.h +++ b/src/key_algs.h @@ -59,6 +59,11 @@ typedef struct WolfCertKeyAlg { * Returns written length (>0) on success, negative on error. */ int (*priv_to_der)(const struct WolfCertKey* key, uint8_t* buf, word32 cap); + /* Confirm the private key belongs to the given public key. An + * implementation may adopt `pub` into `key` when the private encoding + * carries no public half (ML-DSA), so a key that fails must be freed. */ + int (*pub_check) (struct WolfCertKey* key, + const uint8_t* pub, word32 pub_len); /* wc_*_free + free(key->impl). */ void (*free_) (struct WolfCertKey* key); } WolfCertKeyAlg; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b57c505..2c85ca4 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -131,6 +131,7 @@ if(WOLFCERT_ENABLE_SCEP AND WOLFCERT_ENABLE_SERVER) add_test(NAME scep_async_roundtrip COMMAND test_scep_async_roundtrip) add_executable(test_server_ca_store unit/test_server_ca_store.c) + target_include_directories(test_server_ca_store PRIVATE ${CMAKE_SOURCE_DIR}/src) target_link_libraries(test_server_ca_store PRIVATE wolfcert Threads::Threads) add_test(NAME server_ca_store COMMAND test_server_ca_store) endif() diff --git a/tests/unit/test_server_ca_store.c b/tests/unit/test_server_ca_store.c index ad61a5d..b6c3215 100644 --- a/tests/unit/test_server_ca_store.c +++ b/tests/unit/test_server_ca_store.c @@ -19,6 +19,7 @@ #include #include +#include "internal.h" #include "../test_static_mem.h" #include @@ -275,6 +276,308 @@ static int test_ca_persists_across_starts(void) return 0; } +/* Fill `store` with a freshly generated CA of `type` by letting a server start + * against it, then hand back copies of the stored pair. */ +/* Every compiled key type, so each algorithm's certificate-to-key check is + * exercised on both a legitimate reload and a mismatched pair. */ +static const WolfCertKeyType CA_KEY_TYPES[] = { +#ifdef WOLFCERT_HAVE_RSA + WOLFCERT_KEY_RSA, +#endif +#ifdef WOLFCERT_HAVE_ECC + WOLFCERT_KEY_ECC, +#endif +#ifdef WOLFCERT_HAVE_ED25519 + WOLFCERT_KEY_ED25519, +#endif +#ifdef WOLFCERT_HAVE_ED448 + WOLFCERT_KEY_ED448, +#endif +#ifdef WOLFCERT_HAVE_MLDSA +#ifndef WOLFSSL_NO_ML_DSA_44 + WOLFCERT_KEY_MLDSA44, +#endif +#ifndef WOLFSSL_NO_ML_DSA_65 + WOLFCERT_KEY_MLDSA65, +#endif +#ifndef WOLFSSL_NO_ML_DSA_87 + WOLFCERT_KEY_MLDSA87, +#endif +#endif +}; + +static int ca_key_param(WolfCertKeyType t) +{ + if (t == WOLFCERT_KEY_RSA) + return 2048; + if (t == WOLFCERT_KEY_ECC) + return 256; + return 0; +} + +/* Reload the stored CA and have it issue one certificate, then verify that + * certificate against the CA. A pub_check that leaves the key's public half + * unset passes the pair check and still signs garbage, so asserting the + * reload alone would miss it. */ +static int reloaded_ca_signs(WolfCertStoreOps* store, WolfCertKeyType ca_type) +{ + /* Any compiled algorithm serves as the leaf; the first entry always is. */ + WolfCertKeyCfg kcfg = { .type = CA_KEY_TYPES[0], + .param = ca_key_param(CA_KEY_TYPES[0]), + .dev_id = WOLFCERT_DEVID_SOFTWARE }; + WolfCertCertMeta meta = { .subject_dn = "CN=leaf" }; + WolfCertKey* leaf_key = NULL; + WolfCertBuffer csr = { 0 }; + WolfCertCa ca; + uint8_t* issued = NULL; + size_t issued_len = 0; + WOLFSSL_CERT_MANAGER* cm = NULL; + int rc; + + REQUIRE(wolfcert_key_generate(&kcfg, &leaf_key) == WOLFCERT_OK); + REQUIRE(wolfcert_csr_build(leaf_key, &meta, &csr) == WOLFCERT_OK); + wolfcert_key_free(leaf_key); + + REQUIRE(wolfcert_ca_load(&ca, store, NULL) == WOLFCERT_OK); + rc = wolfcert_ca_issue(&ca, csr.data, csr.len, &issued, &issued_len); + wolfcert_buffer_free(&csr); + REQUIRE(rc == WOLFCERT_OK); + + (void)ca_type; +#ifdef WOLFSSL_NO_MALLOC + /* A WOLFSSL_NO_MALLOC wolfSSL never copies an RSA public key onto the CA + * Signer, so its chain verify fails BAD_FUNC_ARG. Assert the issue half + * only, until that lands upstream and the declared wolfSSL floor clears + * it. */ + if (ca_type == WOLFCERT_KEY_RSA) { + WOLFCERT_XFREE(issued, ca.heap); + wolfcert_ca_free(&ca); + return 0; + } +#endif + + cm = wolfSSL_CertManagerNew(); + REQUIRE(cm != NULL); + REQUIRE(wolfSSL_CertManagerLoadCABuffer(cm, ca.cert_der, + (long)ca.cert_der_len, WOLFSSL_FILETYPE_ASN1) == WOLFSSL_SUCCESS); + rc = wolfSSL_CertManagerVerifyBuffer(cm, issued, (long)issued_len, + WOLFSSL_FILETYPE_ASN1); + wolfSSL_CertManagerFree(cm); + + WOLFCERT_XFREE(issued, ca.heap); + wolfcert_ca_free(&ca); + + REQUIRE(rc == WOLFSSL_SUCCESS); + return 0; +} + +static int test_every_alg_reloads(void) +{ + size_t i; + + for (i = 0; i < sizeof(CA_KEY_TYPES) / sizeof(CA_KEY_TYPES[0]); ++i) { + WolfCertStoreOps* store = wolfcert_store_memory_open(NULL); + WolfCertServerCfgSrv cfg; + WolfCertServer* srv = NULL; + + REQUIRE(store != NULL); + ca_store_cfg(&cfg, store); + cfg.ca_key_type = CA_KEY_TYPES[i]; + cfg.ca_key_param = ca_key_param(CA_KEY_TYPES[i]); + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_OK); + wolfcert_server_free(srv); + srv = NULL; + + /* Second start reloads the saved pair through the pair check. */ + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_OK); + wolfcert_server_free(srv); + + if (reloaded_ca_signs(store, CA_KEY_TYPES[i])) + return 1; + + wolfcert_store_memory_close(store); + } + return 0; +} + +static int generate_ca_into(WolfCertStoreOps* store, WolfCertKeyType type, + WolfCertBuffer* cert, WolfCertBuffer* key) +{ + WolfCertServerCfgSrv cfg; + WolfCertServer* srv = NULL; + + ca_store_cfg(&cfg, store); + cfg.ca_key_type = type; + cfg.ca_key_param = ca_key_param(type); + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_OK); + wolfcert_server_free(srv); + + REQUIRE(store->read(store->ctx, "ca.cert.der", cert) == WOLFCERT_OK); + REQUIRE(store->read(store->ctx, "ca.key.der", key) == WOLFCERT_OK); + return 0; +} + +static int mismatched_ca_rejected(WolfCertKeyType type) +{ + WolfCertStoreOps* src_a = wolfcert_store_memory_open(NULL); + WolfCertStoreOps* src_b = wolfcert_store_memory_open(NULL); + WolfCertStoreOps* mixed = wolfcert_store_memory_open(NULL); + WolfCertBuffer cert_a = { 0 }; + WolfCertBuffer key_a = { 0 }; + WolfCertBuffer cert_b = { 0 }; + WolfCertBuffer key_b = { 0 }; + WolfCertServerCfgSrv cfg; + WolfCertServer* srv = NULL; + + REQUIRE(src_a != NULL); + REQUIRE(src_b != NULL); + REQUIRE(mixed != NULL); + + if (generate_ca_into(src_a, type, &cert_a, &key_a)) + return 1; + if (generate_ca_into(src_b, type, &cert_b, &key_b)) + return 1; + REQUIRE(key_a.len != key_b.len || memcmp(key_a.data, key_b.data, key_a.len) != 0); + + REQUIRE(mixed->write(mixed->ctx, "ca.cert.der", cert_a.data, cert_a.len, 0) + == WOLFCERT_OK); + REQUIRE(mixed->write(mixed->ctx, "ca.key.der", key_b.data, key_b.len, 1) + == WOLFCERT_OK); + + ca_store_cfg(&cfg, mixed); + cfg.ca_key_type = type; + cfg.ca_key_param = ca_key_param(type); + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_ERR_PARSE); + REQUIRE(srv == NULL); + + wolfcert_buffer_free(&cert_a); + wolfcert_buffer_free(&key_a); + wolfcert_buffer_free(&cert_b); + wolfcert_buffer_free(&key_b); + wolfcert_store_memory_close(src_a); + wolfcert_store_memory_close(src_b); + wolfcert_store_memory_close(mixed); + return 0; +} + +static int test_mismatched_ca_rejected(void) +{ + size_t i; + + for (i = 0; i < sizeof(CA_KEY_TYPES) / sizeof(CA_KEY_TYPES[0]); ++i) { + if (mismatched_ca_rejected(CA_KEY_TYPES[i])) + return 1; + } + return 0; +} + +static int test_corrupt_ca_cert_rejected(void) +{ + WolfCertStoreOps* src = wolfcert_store_memory_open(NULL); + WolfCertStoreOps* mixed = wolfcert_store_memory_open(NULL); + WolfCertBuffer cert = { 0 }; + WolfCertBuffer key = { 0 }; + const uint8_t junk[] = { 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA }; + WolfCertServerCfgSrv cfg; + WolfCertServer* srv = NULL; + + REQUIRE(src != NULL); + REQUIRE(mixed != NULL); + + if (generate_ca_into(src, CA_KEY_TYPE, &cert, &key)) + return 1; + + REQUIRE(mixed->write(mixed->ctx, "ca.cert.der", junk, sizeof(junk), 0) + == WOLFCERT_OK); + REQUIRE(mixed->write(mixed->ctx, "ca.key.der", key.data, key.len, 1) + == WOLFCERT_OK); + + ca_store_cfg(&cfg, mixed); + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_ERR_PARSE); + REQUIRE(srv == NULL); + + wolfcert_buffer_free(&cert); + wolfcert_buffer_free(&key); + wolfcert_store_memory_close(src); + wolfcert_store_memory_close(mixed); + return 0; +} + +#ifdef WOLFCERT_HAVE_ECC +/* A self-signed certificate with no CA:TRUE, plus the key that signed it: a + * self-consistent pair that is still unusable as a CA. */ +static int make_leaf_pair(WolfCertBuffer* cert_out, WolfCertBuffer* key_out) +{ + WolfCertKeyCfg kcfg = { .type = WOLFCERT_KEY_ECC, .param = 256, + .dev_id = WOLFCERT_DEVID_SOFTWARE }; + WolfCertKey* dk = NULL; + ecc_key wck; + word32 idx = 0; + Cert cert; + WC_RNG rng; + uint8_t der[4096]; + int body, sz; + + REQUIRE(wolfcert_key_generate(&kcfg, &dk) == WOLFCERT_OK); + REQUIRE(wolfcert_key_to_der(dk, key_out) == WOLFCERT_OK); + wolfcert_key_free(dk); + + REQUIRE(wc_ecc_init(&wck) == 0); + REQUIRE(wc_EccPrivateKeyDecode(key_out->data, &idx, &wck, + (word32)key_out->len) == 0); + + wc_InitCert(&cert); + snprintf(cert.subject.commonName, sizeof(cert.subject.commonName), + "%s", "wolfCert Leaf"); + cert.isCA = 0; + cert.selfSigned = 1; + cert.daysValid = 30; + cert.sigType = CTC_SHA256wECDSA; + + REQUIRE(wc_InitRng(&rng) == 0); + body = wc_MakeCert(&cert, der, sizeof(der), NULL, &wck, &rng); + REQUIRE(body > 0); + sz = wc_SignCert(body, cert.sigType, der, sizeof(der), NULL, &wck, &rng); + REQUIRE(sz > 0); + wc_FreeRng(&rng); + wc_ecc_free(&wck); + + cert_out->data = (uint8_t*)WOLFCERT_XMALLOC((size_t)sz, NULL); + REQUIRE(cert_out->data != NULL); + memcpy(cert_out->data, der, (size_t)sz); + cert_out->len = (size_t)sz; + cert_out->heap = NULL; + return 0; +} + +static int test_leaf_ca_rejected(void) +{ + WolfCertStoreOps* store = wolfcert_store_memory_open(NULL); + WolfCertBuffer cert = { 0 }; + WolfCertBuffer key = { 0 }; + WolfCertServerCfgSrv cfg; + WolfCertServer* srv = NULL; + + REQUIRE(store != NULL); + if (make_leaf_pair(&cert, &key)) + return 1; + + REQUIRE(store->write(store->ctx, "ca.cert.der", cert.data, cert.len, 0) + == WOLFCERT_OK); + REQUIRE(store->write(store->ctx, "ca.key.der", key.data, key.len, 1) + == WOLFCERT_OK); + + ca_store_cfg(&cfg, store); + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_ERR_PARSE); + REQUIRE(srv == NULL); + + wolfcert_buffer_free(&cert); + wolfcert_buffer_free(&key); + wolfcert_store_memory_close(store); + return 0; +} +#endif /* WOLFCERT_HAVE_ECC */ + int main(void) { REQUIRE(test_static_mem_init() == 0); @@ -292,6 +595,16 @@ int main(void) return 1; if (test_ca_persists_across_starts()) return 1; + if (test_every_alg_reloads()) + return 1; + if (test_mismatched_ca_rejected()) + return 1; + if (test_corrupt_ca_cert_rejected()) + return 1; +#ifdef WOLFCERT_HAVE_ECC + if (test_leaf_ca_rejected()) + return 1; +#endif wolfcert_cleanup(); printf("OK\n"); From 41a1a9bc02567e7ee9079c402af7b2df041d696d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 9 Sep 2026 17:34:52 +0200 Subject: [PATCH 07/18] Bound the non-blocking read to the remaining response allowance nb_read_some always asked nb_rx_reserve for a full WOLFCERT_HTTP_READ_CHUNK, and nb_rx_reserve refuses any request that would take the accumulator past the caller's body cap plus the header budget. A response that fits inside that allowance but leaves less than one read quantum of headroom was therefore rejected with WOLFCERT_ERR_PROTOCOL before its final bytes were ever read. The blocking reader, which appends only the bytes it actually received, accepted the same response, so the two paths disagreed on which replies were legal. Compute the room the allowance still permits, clamp both the reserve and the read length to it, and raise the protocol error only once no room is left at all. Both the TLS and the transport read paths are clamped. A new unit test drives a non-blocking session against a reply sized to land exactly on the allowance and requires the body to arrive intact. The header budget itself was a bare 8192 repeated in both readers and now in the test, which only pins the boundary while the three agree, so it becomes WOLFCERT_HTTP_HEADER_BUDGET alongside the other overridable HTTP sizes in internal.h and is documented as a RAM knob. It stays separate from WOLFCERT_HTTP_READ_CHUNK: one is a header allowance, the other a read granularity, and tuning the read size must not move the largest response header block the client accepts. A full accumulator is not a protocol error by itself: an EOF-delimited body ending exactly on the allowance is complete, and the blocking reader accepts it. Fall back to a one-byte probe there, which takes the close as the end of the body and rejects only a peer that keeps sending. Fixes F-11049. --- docs/EMBEDDED.md | 6 +- src/http.c | 53 +++++++++--- src/internal.h | 7 ++ tests/unit/test_http.c | 182 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 236 insertions(+), 12 deletions(-) diff --git a/docs/EMBEDDED.md b/docs/EMBEDDED.md index d1ed3b6..1b5348e 100644 --- a/docs/EMBEDDED.md +++ b/docs/EMBEDDED.md @@ -134,12 +134,16 @@ the request completes) so it never counts against the stack budget. | `WOLFCERT_HTTP_QUERY_SZ` | `8192` | sized to hold a base64 GET `PKIOperation` message; on the SCEP server it extends the heap read buffer (`REQ_BUF_SZ + QUERY_SZ`) that `query` points into | | `WOLFCERT_HTTP_AUTH_BUF_SZ` | `512` | client Basic-auth header line (`http.c`) | | `WOLFCERT_HTTP_MAX_PATH_LEN` | `8192` | client-side ceiling on a request URL's path+query (`http.c`) | +| `WOLFCERT_HTTP_HEADER_BUDGET` | `8192` | client response allowance added to the caller's body cap, bounding the status line plus header block. Both the blocking and the non-blocking reader grow their accumulator to `max_response_bytes + this` (`http.c`) | | `WOLFCERT_SCEP_MAX_GET_URL` | `8192` | client cap on a GET `PKIOperation` URL; a larger message is refused with `WOLFCERT_ERR_UNSUPPORTED` so the caller POSTs (`internal.h`) | Shrinking `WOLFCERT_HTTP_REQ_BUF_SZ` lowers the largest request header block the server accepts; `WOLFCERT_HTTP_PATH_SZ` / `WOLFCERT_HTTP_QUERY_SZ` lower the longest request path / query; `WOLFCERT_HTTP_AUTH_BUF_SZ` lowers the -longest Basic-auth credential the client can send. A POST-only SCEP deployment +longest Basic-auth credential the client can send. `WOLFCERT_HTTP_HEADER_BUDGET` +trims the client's response accumulator, and with it the largest response +header block it will accept, so keep it above the headers your CA actually +sends. A POST-only SCEP deployment can trim `WOLFCERT_HTTP_QUERY_SZ` (and, on the client, `WOLFCERT_SCEP_MAX_GET_URL` and `WOLFCERT_HTTP_MAX_PATH_LEN`) back down. Example: diff --git a/src/http.c b/src/http.c index 7e88334..08264f0 100644 --- a/src/http.c +++ b/src/http.c @@ -1104,7 +1104,8 @@ static int http_read_response(WolfCertConn* c, WolfCertHttpResponse* resp, void* heap) { - DynBuf rx = { .heap = heap, .max = max_body + 8192 }; + DynBuf rx = { .heap = heap, + .max = max_body + WOLFCERT_HTTP_HEADER_BUDGET }; int hdr_end = read_headers(c, &rx); if (hdr_end < 0) { WOLFCERT_XFREE(rx.buf, heap); @@ -1469,6 +1470,12 @@ static int nb_write(WolfCertConn* c, const uint8_t* buf, size_t len, size_t* off return WOLFCERT_OK; } +/* Total accumulator allowance: the body cap plus the header budget. */ +static size_t nb_rx_max(const WolfCertHttpSession* s) +{ + return s->max_body + WOLFCERT_HTTP_HEADER_BUDGET; +} + /* Ensure the rx buffer has room for `need` more bytes. */ static int nb_rx_reserve(WolfCertHttpSession* s, size_t need) { @@ -1476,7 +1483,7 @@ static int nb_rx_reserve(WolfCertHttpSession* s, size_t need) if (want <= s->sm_rx_cap) return WOLFCERT_OK; - size_t max = s->max_body + 8192; + size_t max = nb_rx_max(s); if (want > max) return WOLFCERT_ERR_PROTOCOL; @@ -1503,14 +1510,36 @@ static int nb_rx_reserve(WolfCertHttpSession* s, size_t need) static int nb_read_some(WolfCertHttpSession* s, int* ended) { *ended = 0; - int rc = nb_rx_reserve(s, WOLFCERT_HTTP_READ_CHUNK); - if (rc != WOLFCERT_OK) - return rc; + + /* Read at most what the allowance still permits, so a response that + * ends inside the final quantum is not rejected before it is read. */ + size_t room = nb_rx_max(s) - s->sm_rx_len; + uint8_t probe; + uint8_t* dst; + + if (room == 0) { + /* An EOF-delimited body ending exactly on the allowance is legal, so + * a full accumulator still has to look for the close. */ + dst = &probe; + room = 1; + } + else { + if (room > WOLFCERT_HTTP_READ_CHUNK) + room = WOLFCERT_HTTP_READ_CHUNK; + + int rc = nb_rx_reserve(s, room); + if (rc != WOLFCERT_OK) + return rc; + + dst = s->sm_rx + s->sm_rx_len; + } if (s->conn.ssl) { - int r = wolfSSL_read(s->conn.ssl, s->sm_rx + s->sm_rx_len, - WOLFCERT_HTTP_READ_CHUNK); + int r = wolfSSL_read(s->conn.ssl, dst, (int)room); if (r > 0) { + if (dst == &probe) + return WOLFCERT_ERR_PROTOCOL; + s->sm_rx_len += (size_t)r; return WOLFCERT_OK; } @@ -1533,12 +1562,14 @@ static int nb_read_some(WolfCertHttpSession* s, int* ended) return WOLFCERT_ERR_IO; } - int r = s->conn.t.read(s->conn.t.ctx, s->conn.handle, - s->sm_rx + s->sm_rx_len, - WOLFCERT_HTTP_READ_CHUNK, s->conn.io_timeout_ms); + int r = s->conn.t.read(s->conn.t.ctx, s->conn.handle, dst, + room, s->conn.io_timeout_ms); if (r > 0) { - if ((size_t)r > WOLFCERT_HTTP_READ_CHUNK) + if ((size_t)r > room) return WOLFCERT_ERR_IO; + if (dst == &probe) + return WOLFCERT_ERR_PROTOCOL; + s->sm_rx_len += (size_t)r; return WOLFCERT_OK; } diff --git a/src/internal.h b/src/internal.h index 57866aa..28981d8 100644 --- a/src/internal.h +++ b/src/internal.h @@ -75,6 +75,13 @@ #define WOLFCERT_HTTP_AUTH_BUF_SZ 512 /* client Basic-auth header line */ #endif +/* Response allowance the client readers add on top of the caller's body cap, + * bounding the status line plus header block. Distinct from the read + * granularity: a read is clamped to whatever of this allowance is left. */ +#ifndef WOLFCERT_HTTP_HEADER_BUDGET +#define WOLFCERT_HTTP_HEADER_BUDGET 8192 /* client response header allowance */ +#endif + /* Heap headroom added on top of (envelope + signer cert) when encoding a SCEP * SignedData pkiMessage. wolfSSL's PKCS#7 encoder mutates internal state per * call, so it is given a single right-sized one-shot buffer rather than diff --git a/tests/unit/test_http.c b/tests/unit/test_http.c index 01c2471..dcd450d 100644 --- a/tests/unit/test_http.c +++ b/tests/unit/test_http.c @@ -444,6 +444,184 @@ static int test_chunked_size_overflow(void) return 0; } +/* Size the reply to land exactly on the reader's accumulator allowance: a legal + * response the blocking reader accepts, with less than one read quantum of + * headroom left at the end. */ +#define NEAR_CAP_MAX_BODY 1024 +#define NEAR_CAP_TOTAL (NEAR_CAP_MAX_BODY + WOLFCERT_HTTP_HEADER_BUDGET) +/* The reply is built as head + pad + tail; too small a budget underflows the + * unsigned pad and memsets past the buffer. */ +#if NEAR_CAP_TOTAL < 256 +#error "test_session_near_cap_response needs WOLFCERT_HTTP_HEADER_BUDGET >= 256" +#endif + +static void* srv_thread_near_cap(void* arg) +{ + struct srv_ctx* sc = (struct srv_ctx*)arg; + int cs = accept(sc->listen_fd, NULL, NULL); + close(sc->listen_fd); + if (cs < 0) + return NULL; + + char buf[4096]; + size_t n = 0; + while (n < sizeof(buf) - 1) { + ssize_t r = recv(cs, buf + n, sizeof(buf) - 1 - n, 0); + if (r <= 0) + break; + n += (size_t)r; + buf[n] = '\0'; + if (strstr(buf, "\r\n\r\n") != NULL) + break; + } + + const char* head = + "HTTP/1.1 200 OK\r\n" + "Content-Type: text/plain\r\n" + "Content-Length: 5\r\n" + "X-Pad: "; + const char* tail = "\r\n\r\nready"; + size_t pad = NEAR_CAP_TOTAL - strlen(head) - strlen(tail); + + char response[NEAR_CAP_TOTAL]; + memcpy(response, head, strlen(head)); + memset(response + strlen(head), 'A', pad); + memcpy(response + strlen(head) + pad, tail, strlen(tail)); + + size_t off = 0; + while (off < sizeof(response)) { + ssize_t w = send(cs, response + off, sizeof(response) - off, 0); + if (w <= 0) + break; + off += (size_t)w; + } + + shutdown(cs, SHUT_WR); + close(cs); + return NULL; +} + +/* A response whose total size stays within the configured allowance must be + * accepted even when the reader is left with less than one read quantum of + * headroom. */ +static int test_session_near_cap_response(void) +{ + struct srv_ctx sc = { 0 }; + pthread_t tid; + int port = 0; + sc.listen_fd = listen_loopback(&port); + REQUIRE(sc.listen_fd >= 0); + REQUIRE(pthread_create(&tid, NULL, srv_thread_near_cap, &sc) == 0); + + char base[128]; + char url[160]; + snprintf(base, sizeof(base), "http://127.0.0.1:%d", port); + snprintf(url, sizeof(url), "http://127.0.0.1:%d/test", port); + + WolfCertHttpSessionCfg cfg = { + .base_url = base, + .nonblocking = 1, + .max_response_bytes = NEAR_CAP_MAX_BODY, + }; + WolfCertHttpSession* s = NULL; + REQUIRE(wolfcert_http_session_open(&cfg, &s) == WOLFCERT_OK); + + WolfCertHttpRequest req = { .method = "GET", .url = url }; + WolfCertHttpResponse resp = { 0 }; + REQUIRE(drive_nb(s, &req, &resp) == WOLFCERT_OK); + REQUIRE(resp.status_code == 200); + REQUIRE(resp.body_len == 5); + REQUIRE(memcmp(resp.body, "ready", 5) == 0); + wolfcert_http_response_free(&resp); + + wolfcert_http_session_close(s); + pthread_join(tid, NULL); + return 0; +} + +static void* srv_thread_eof_cap(void* arg) +{ + struct srv_ctx* sc = (struct srv_ctx*)arg; + int cs = accept(sc->listen_fd, NULL, NULL); + close(sc->listen_fd); + if (cs < 0) + return NULL; + + char buf[4096]; + size_t n = 0; + while (n < sizeof(buf) - 1) { + ssize_t r = recv(cs, buf + n, sizeof(buf) - 1 - n, 0); + if (r <= 0) + break; + n += (size_t)r; + buf[n] = '\0'; + if (strstr(buf, "\r\n\r\n") != NULL) + break; + } + + /* No Content-Length and no chunking: the body runs to the close. */ + const char* head = + "HTTP/1.1 200 OK\r\n" + "Content-Type: text/plain\r\n" + "Connection: close\r\n" + "X-Pad: "; + const char* tail = "\r\n\r\nready"; + size_t pad = NEAR_CAP_TOTAL - strlen(head) - strlen(tail); + + char response[NEAR_CAP_TOTAL]; + memcpy(response, head, strlen(head)); + memset(response + strlen(head), 'A', pad); + memcpy(response + strlen(head) + pad, tail, strlen(tail)); + + size_t off = 0; + while (off < sizeof(response)) { + ssize_t w = send(cs, response + off, sizeof(response) - off, 0); + if (w <= 0) + break; + off += (size_t)w; + } + + close(cs); + return NULL; +} + +/* An EOF-delimited response that fills the allowance exactly is complete: the + * reader must look for the close rather than reject the full accumulator. */ +static int test_session_eof_cap_response(void) +{ + struct srv_ctx sc = { 0 }; + pthread_t tid; + int port = 0; + sc.listen_fd = listen_loopback(&port); + REQUIRE(sc.listen_fd >= 0); + REQUIRE(pthread_create(&tid, NULL, srv_thread_eof_cap, &sc) == 0); + + char base[128]; + char url[160]; + snprintf(base, sizeof(base), "http://127.0.0.1:%d", port); + snprintf(url, sizeof(url), "http://127.0.0.1:%d/test", port); + + WolfCertHttpSessionCfg cfg = { + .base_url = base, + .nonblocking = 1, + .max_response_bytes = NEAR_CAP_MAX_BODY, + }; + WolfCertHttpSession* s = NULL; + REQUIRE(wolfcert_http_session_open(&cfg, &s) == WOLFCERT_OK); + + WolfCertHttpRequest req = { .method = "GET", .url = url }; + WolfCertHttpResponse resp = { 0 }; + REQUIRE(drive_nb(s, &req, &resp) == WOLFCERT_OK); + REQUIRE(resp.status_code == 200); + REQUIRE(resp.body_len >= 5); + REQUIRE(memcmp(resp.body + resp.body_len - 5, "ready", 5) == 0); + wolfcert_http_response_free(&resp); + + wolfcert_http_session_close(s); + pthread_join(tid, NULL); + return 0; +} + /* Capture the request headers a client sends so the test can inspect * which headers were emitted on the wire. */ struct capture_ctx { @@ -580,6 +758,10 @@ int main(void) return 1; if (test_chunked_size_overflow()) return 1; + if (test_session_near_cap_response()) + return 1; + if (test_session_eof_cap_response()) + return 1; if (test_request_transfer_encoding()) return 1; if (test_request_host_header_ipv6()) From b8f6c5b3c61c2faf057827303500f7cda9b4dc96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 9 Sep 2026 17:40:26 +0200 Subject: [PATCH 08/18] Stop the URL authority scan at a query or fragment A server URL with no path but a query, such as http://ca.example?operation=GetCACaps, put the whole query into the host because the authority scan only stopped at a colon or a slash. The SCEP client builds exactly that shape whenever the configured server URL has no path, so GetCACaps and PKIOperation failed to resolve against valid servers. Terminate the authority at a question mark or a hash as well, and give the request target a leading slash when the remainder does not already start with one. That also covers an explicit port or a bracketed IPv6 literal followed directly by a query, where the port parse stopped at the query and left the target without its slash. Fixes F-12856. --- src/http.c | 23 +++++++++++++++++++---- tests/unit/test_http.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/src/http.c b/src/http.c index 08264f0..d0fa037 100644 --- a/src/http.c +++ b/src/http.c @@ -224,7 +224,8 @@ WOLFCERT_TEST_VIS int wolfcert_http_url_parse(const char* url, WolfCertUrl* out, } else { host_end = host_start; - while (*host_end && *host_end != ':' && *host_end != '/') { + while (*host_end && *host_end != ':' && *host_end != '/' + && *host_end != '?' && *host_end != '#') { ++host_end; } @@ -253,19 +254,33 @@ WOLFCERT_TEST_VIS int wolfcert_http_url_parse(const char* url, WolfCertUrl* out, host_end = end; } - size_t plen = *host_end ? strlen(host_end) : 1; + /* A pathless authority may still be followed by a query or a fragment, so + * the request target gets a synthesized leading slash. RFC 7230 section + * 5.3: the fragment is client-side only and never goes on the wire. */ + const char* frag = strchr(host_end, '#'); + size_t tlen = frag ? (size_t)(frag - host_end) : strlen(host_end); + size_t plen = (*host_end == '/') ? tlen : tlen + 1; if (plen > WOLFCERT_HTTP_MAX_PATH_LEN) { wolfcert_http_url_free(out); return WOLFCERT_ERR_PARSE; } - out->path = (*host_end == '\0') ? wolfcert_strdup("/", heap) - : wolfcert_strdup(host_end, heap); + out->path = (char*)WOLFCERT_XMALLOC(plen + 1, heap); if (out->path == NULL) { wolfcert_http_url_free(out); return WOLFCERT_ERR_MEMORY; } + if (*host_end == '/') { + memcpy(out->path, host_end, tlen); + out->path[tlen] = '\0'; + } + else { + out->path[0] = '/'; + memcpy(out->path + 1, host_end, tlen); + out->path[tlen + 1] = '\0'; + } + return WOLFCERT_OK; } diff --git a/tests/unit/test_http.c b/tests/unit/test_http.c index dcd450d..d2fc3dc 100644 --- a/tests/unit/test_http.c +++ b/tests/unit/test_http.c @@ -76,6 +76,43 @@ static int test_url_parser(void) REQUIRE(wolfcert_http_url_parse("ftp://nope/", &u, NULL) == WOLFCERT_ERR_UNSUPPORTED); + /* A pathless URL carrying a query: SCEP builds exactly this shape. The + * query must not be absorbed into the host, and the request target has to + * keep a leading slash. */ + REQUIRE(wolfcert_http_url_parse("http://ca.example?operation=GetCACaps", &u, NULL) == WOLFCERT_OK); + REQUIRE(strcmp(u.host, "ca.example") == 0); + REQUIRE(u.port == 80); + REQUIRE(strcmp(u.path, "/?operation=GetCACaps") == 0); + wolfcert_http_url_free(&u); + + REQUIRE(wolfcert_http_url_parse("http://ca.example:8080?operation=PKIOperation", &u, NULL) == WOLFCERT_OK); + REQUIRE(strcmp(u.host, "ca.example") == 0); + REQUIRE(u.port == 8080); + REQUIRE(strcmp(u.path, "/?operation=PKIOperation") == 0); + wolfcert_http_url_free(&u); + + REQUIRE(wolfcert_http_url_parse("https://[::1]?operation=GetCACaps", &u, NULL) == WOLFCERT_OK); + REQUIRE(strcmp(u.host, "::1") == 0); + REQUIRE(u.port == 443); + REQUIRE(strcmp(u.path, "/?operation=GetCACaps") == 0); + wolfcert_http_url_free(&u); + + /* RFC 7230 section 5.3: the fragment is client-side only, so it must not + * reach the request target. */ + REQUIRE(wolfcert_http_url_parse("http://ca.example#frag", &u, NULL) == WOLFCERT_OK); + REQUIRE(strcmp(u.host, "ca.example") == 0); + REQUIRE(strcmp(u.path, "/") == 0); + wolfcert_http_url_free(&u); + + REQUIRE(wolfcert_http_url_parse("http://ca.example/p#frag", &u, NULL) == WOLFCERT_OK); + REQUIRE(strcmp(u.host, "ca.example") == 0); + REQUIRE(strcmp(u.path, "/p") == 0); + wolfcert_http_url_free(&u); + + REQUIRE(wolfcert_http_url_parse("http://ca.example/p?q=1#frag", &u, NULL) == WOLFCERT_OK); + REQUIRE(strcmp(u.path, "/p?q=1") == 0); + wolfcert_http_url_free(&u); + /* A URL with no explicit scheme defaults to TLS (https). */ REQUIRE(wolfcert_http_url_parse("ca.example.com:8443/p", &u, NULL) == WOLFCERT_OK); REQUIRE(strcmp(u.scheme, "https") == 0); From cc97254985ec3cd03104bdc908c00c042a37f15d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Wed, 9 Sep 2026 20:13:36 +0200 Subject: [PATCH 09/18] Assert keyUsage on the generated self-signed CA certificate The CA bootstrap path emitted a certificate with basicConstraints CA:TRUE and no keyUsage extension, so a relying party had nothing to check the CA against. The same key signs issued certificates and SCEP CertReps, and on RSA it also decrypts the pkcsPKIEnvelope, so assert keyCertSign, cRLSign and digitalSignature for every key type and keyEncipherment only for RSA. An ECC, Ed25519 or ML-DSA key cannot encipher a key, and RFC 8894 limits the envelope to RSA in any case. The test asserts the full set for every key type, including that keyEncipherment is absent from a non-RSA CA. Fixes F-8032. --- src/ca_issue.c | 11 ++++++ tests/unit/test_server_ca_store.c | 57 +++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/src/ca_issue.c b/src/ca_issue.c index 31c9a21..436f427 100644 --- a/src/ca_issue.c +++ b/src/ca_issue.c @@ -95,6 +95,17 @@ static int gen_self_signed_cert(WolfCertCa* ca) cert->daysValid = 3650; cert->sigType = alg->ctc_sig_default; + /* RFC 5280 section 4.2.1.3: this key signs certificates and SCEP CertReps, + * and on RSA it also decrypts the pkcsPKIEnvelope. */ + const char* usage = ca->type == WOLFCERT_KEY_RSA + ? "keyCertSign,cRLSign,digitalSignature,keyEncipherment" + : "keyCertSign,cRLSign,digitalSignature"; + int ku = wc_SetKeyUsage(cert, usage); + if (ku != 0) { + wc_CertFree(cert); + return WOLFCERT_ERR_WC(ku, "ca", "SetKeyUsage"); + } + WC_RNG rng; if (wc_InitRng_ex(&rng, ca->heap, WOLFCERT_DEVID_SOFTWARE) != 0) { wc_CertFree(cert); diff --git a/tests/unit/test_server_ca_store.c b/tests/unit/test_server_ca_store.c index b6c3215..86654af 100644 --- a/tests/unit/test_server_ca_store.c +++ b/tests/unit/test_server_ca_store.c @@ -22,6 +22,9 @@ #include "internal.h" #include "../test_static_mem.h" +#include +#include + #include #include @@ -471,6 +474,58 @@ static int test_mismatched_ca_rejected(void) return 0; } +/* RFC 5280 section 4.2.1.3: the CA signs certificates and, for SCEP, both + * signs CertReps and decrypts the pkcsPKIEnvelope, so its certificate must + * assert those usages rather than omitting the extension. */ +static int ca_key_usage_set(WolfCertKeyType type) +{ + WolfCertStoreOps* store = wolfcert_store_memory_open(NULL); + WolfCertBuffer cert = { 0 }; + WolfCertBuffer key = { 0 }; + DecodedCert dc; + int rc = 0; + + REQUIRE(store != NULL); + + if (generate_ca_into(store, type, &cert, &key)) + return 1; + + wc_InitDecodedCert(&dc, cert.data, (word32)cert.len, NULL); + REQUIRE(wc_ParseCert(&dc, CERT_TYPE, NO_VERIFY, NULL) == 0); + + if (dc.extKeyUsageSet == 0 || + (dc.extKeyUsage & KEYUSE_KEY_CERT_SIGN) == 0 || + (dc.extKeyUsage & KEYUSE_CRL_SIGN) == 0 || + (dc.extKeyUsage & KEYUSE_DIGITAL_SIG) == 0) { + rc = 1; + } + /* keyEncipherment belongs to the RSA CA alone: it decrypts the SCEP + * pkcsPKIEnvelope, which no other key type is used for. */ + else if (((dc.extKeyUsage & KEYUSE_KEY_ENCIPHER) != 0) != + (type == WOLFCERT_KEY_RSA)) { + rc = 1; + } + + wc_FreeDecodedCert(&dc); + wolfcert_buffer_free(&cert); + wolfcert_buffer_free(&key); + wolfcert_store_memory_close(store); + + REQUIRE(rc == 0); + return 0; +} + +static int test_ca_key_usage(void) +{ + size_t i; + + for (i = 0; i < sizeof(CA_KEY_TYPES) / sizeof(CA_KEY_TYPES[0]); ++i) { + if (ca_key_usage_set(CA_KEY_TYPES[i])) + return 1; + } + return 0; +} + static int test_corrupt_ca_cert_rejected(void) { WolfCertStoreOps* src = wolfcert_store_memory_open(NULL); @@ -605,6 +660,8 @@ int main(void) if (test_leaf_ca_rejected()) return 1; #endif + if (test_ca_key_usage()) + return 1; wolfcert_cleanup(); printf("OK\n"); From be7e9ce7d2fb22e525de488496d5c71c2b9334d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Fri, 11 Sep 2026 11:21:32 +0200 Subject: [PATCH 10/18] Report CA-store read and rollback failures as themselves wolfcert_ca_load() classified any result pair containing NOT_FOUND as an incomplete store, so a cert read that reported NOT_FOUND beside a key read that failed with I/O or memory error surfaced as WOLFCERT_ERR_PARSE. That hides an actionable store failure behind a parse error, and it reads the same either way round. A read that failed for a reason other than absence is now returned as itself; the store is called incomplete only once the other half actually read back. wolfcert_ca_save() writes the certificate before the key, and rolled the certificate back on a key-write failure only when the caller-supplied vtable happened to provide remove. An absent or failing remove left ca.cert.der committed without its key, which every later start now rejects as incomplete - and the caller was told nothing beyond the write error. The vtable has no rename or commit primitive, so removal is the only rollback available; when it cannot be performed the error now says the store is left incomplete and must be cleared before restart. wolfcert_server_start() re-wrapped that result with a generic "ca_store save failed", overwriting the diagnostic it had just recorded. The load path above it already avoids that for the same reason. test_mixed_read_failure drives both orders of the mixed pair plus the memory case; test_rollback_unavailable drives both a NULL remove and a failing one, and asserts the certificate really is still there and that the next start refuses it. --- src/ca_issue.c | 27 ++++--- src/server.c | 5 +- tests/unit/test_server_ca_store.c | 115 ++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 13 deletions(-) diff --git a/src/ca_issue.c b/src/ca_issue.c index 436f427..ebe3f6e 100644 --- a/src/ca_issue.c +++ b/src/ca_issue.c @@ -298,15 +298,17 @@ int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, void* heap) if (cert_rc == WOLFCERT_ERR_NOT_FOUND && key_rc == WOLFCERT_ERR_NOT_FOUND) return WOLFCERT_ERR_NOT_FOUND; + /* An absent half is only damage once the other half read back. */ + if (cert_rc != WOLFCERT_OK && cert_rc != WOLFCERT_ERR_NOT_FOUND) + return WOLFCERT_ERR(cert_rc, "ca", "CA store read failed"); + if (key_rc != WOLFCERT_OK && key_rc != WOLFCERT_ERR_NOT_FOUND) + return WOLFCERT_ERR(key_rc, "ca", "CA store read failed"); + /* Half a pair is a damaged store, not an empty one. Reporting * NOT_FOUND here would let the caller mint a CA over the survivor. */ - if (cert_rc == WOLFCERT_ERR_NOT_FOUND || key_rc == WOLFCERT_ERR_NOT_FOUND) - return WOLFCERT_ERR(WOLFCERT_ERR_PARSE, "ca", - "CA store is incomplete: %s is missing", - cert_rc == WOLFCERT_ERR_NOT_FOUND ? "ca.cert.der" : "ca.key.der"); - - rc = (cert_rc != WOLFCERT_OK) ? cert_rc : key_rc; - return WOLFCERT_ERR(rc, "ca", "CA store read failed"); + return WOLFCERT_ERR(WOLFCERT_ERR_PARSE, "ca", + "CA store is incomplete: %s is missing", + cert_rc == WOLFCERT_ERR_NOT_FOUND ? "ca.cert.der" : "ca.key.der"); } /* Iterate every registered algorithm and see which private-key decoder @@ -363,11 +365,16 @@ int wolfcert_ca_save(const WolfCertCa* ca, WolfCertStoreOps* store) return rc; rc = store->write(store->ctx, "ca.key.der", ca->key_der, ca->key_der_len, 1); + if (rc == WOLFCERT_OK) + return rc; /* A certificate without its key is a damaged store that every later load - * rejects, so drop the half that landed. */ - if (rc != WOLFCERT_OK && store->remove != NULL) - (void)store->remove(store->ctx, "ca.cert.der"); + * rejects, and the vtable has no primitive but remove to undo it. */ + if (store->remove == NULL || + store->remove(store->ctx, "ca.cert.der") != WOLFCERT_OK) + return WOLFCERT_ERR(rc, "ca", + "CA key write failed and ca.cert.der could not be rolled back: " + "the store is left incomplete and must be cleared before restart"); return rc; } diff --git a/src/server.c b/src/server.c index 0e3e15d..ecd8b4c 100644 --- a/src/server.c +++ b/src/server.c @@ -288,11 +288,10 @@ int wolfcert_server_start(const WolfCertServerCfgSrv* cfg, WolfCertServer** out) goto fail; if (cfg->ca_store != NULL) { + /* Not re-wrapped: it would lose ca_save's rollback diagnostic. */ rc = wolfcert_ca_save(&s->ca, cfg->ca_store); - if (rc != WOLFCERT_OK) { - WOLFCERT_ERR(rc, "server", "ca_store save failed"); + if (rc != WOLFCERT_OK) goto fail; - } } } diff --git a/tests/unit/test_server_ca_store.c b/tests/unit/test_server_ca_store.c index 86654af..a756f2d 100644 --- a/tests/unit/test_server_ca_store.c +++ b/tests/unit/test_server_ca_store.c @@ -142,6 +142,61 @@ static int test_save_failure_rejected(void) return 0; } +/* Stub backend whose two CA reads fail differently, so the precedence between + * an absent half and a genuinely failing read can be driven either way. */ +typedef struct { + int cert_rc; + int key_rc; +} SplitCtx; + +static int split_read(void* ctx_, const char* key, WolfCertBuffer* out) +{ + SplitCtx* ctx = (SplitCtx*)ctx_; + + (void)out; + return strcmp(key, "ca.cert.der") == 0 ? ctx->cert_rc : ctx->key_rc; +} + +/* A read that failed for a reason other than absence must be reported as + * itself: calling the store "incomplete" hides an actionable I/O or memory + * failure behind a parse error. */ +static int mixed_read_failure(int cert_rc, int key_rc, int want) +{ + SplitCtx ctx = { cert_rc, key_rc }; + WolfCertStoreOps store; + WolfCertServerCfgSrv cfg; + WolfCertServer* srv = NULL; + + memset(&store, 0, sizeof(store)); + store.read = split_read; + store.write = stub_write; + store.remove = stub_remove; + store.ctx = &ctx; + + ca_store_cfg(&cfg, &store); + REQUIRE(wolfcert_server_start(&cfg, &srv) == want); + REQUIRE(srv == NULL); + return 0; +} + +static int test_mixed_read_failure(void) +{ + if (mixed_read_failure(WOLFCERT_ERR_NOT_FOUND, WOLFCERT_ERR_IO, + WOLFCERT_ERR_IO)) + return 1; + if (mixed_read_failure(WOLFCERT_ERR_IO, WOLFCERT_ERR_NOT_FOUND, + WOLFCERT_ERR_IO)) + return 1; + if (mixed_read_failure(WOLFCERT_ERR_NOT_FOUND, WOLFCERT_ERR_MEMORY, + WOLFCERT_ERR_MEMORY)) + return 1; + + /* Both absent is still an empty store, which bootstraps rather than + * failing; one absent beside one good read is still incomplete. */ + return mixed_read_failure(WOLFCERT_ERR_NOT_FOUND, WOLFCERT_OK, + WOLFCERT_ERR_PARSE); +} + /* Backend that forwards to a real store but fails the nth write, so a * bootstrap can be interrupted between the certificate and the key. */ typedef struct { @@ -175,6 +230,13 @@ static int flaky_remove(void* ctx_, const char* key) return in->remove(in->ctx, key); } +static int failing_remove(void* ctx_, const char* key) +{ + (void)ctx_; + (void)key; + return WOLFCERT_ERR_IO; +} + /* A key write that fails once the certificate has landed must take the * certificate with it: a cert-only store is rejected by every later load. */ static int test_save_rollback(void) @@ -214,6 +276,55 @@ static int test_save_rollback(void) return 0; } +/* A rollback the store cannot perform must not be reported as a plain write + * failure: the certificate stays behind and poisons every later start, so the + * diagnostic has to say so. */ +static int rollback_unavailable(int have_remove) +{ + WolfCertStoreOps* mem = wolfcert_store_memory_open(NULL); + FlakyCtx fctx; + WolfCertStoreOps store; + WolfCertServerCfgSrv cfg; + WolfCertServer* srv = NULL; + WolfCertBuffer left = { 0 }; + + REQUIRE(mem != NULL); + + memset(&fctx, 0, sizeof(fctx)); + fctx.inner = mem; + fctx.fail_at = 2; + + memset(&store, 0, sizeof(store)); + store.read = flaky_read; + store.write = flaky_write; + store.remove = have_remove ? failing_remove : NULL; + store.ctx = &fctx; + + ca_store_cfg(&cfg, &store); + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_ERR_IO); + REQUIRE(srv == NULL); + REQUIRE(strstr(wolfcert_last_error_message(), "rolled back") != NULL); + + /* The certificate really is still there, and the next start refuses it. */ + REQUIRE(mem->read(mem->ctx, "ca.cert.der", &left) == WOLFCERT_OK); + wolfcert_buffer_free(&left); + + fctx.fail_at = 0; + REQUIRE(wolfcert_server_start(&cfg, &srv) == WOLFCERT_ERR_PARSE); + REQUIRE(srv == NULL); + + wolfcert_store_memory_close(mem); + return 0; +} + +static int test_rollback_unavailable(void) +{ + if (rollback_unavailable(0)) + return 1; + + return rollback_unavailable(1); +} + /* A store holding one half of the pair is damaged, not empty: starting * against it must fail rather than mint a CA over the surviving half. */ static int partial_store_rejected(const char* present) @@ -646,6 +757,10 @@ int main(void) return 1; if (test_save_rollback()) return 1; + if (test_rollback_unavailable()) + return 1; + if (test_mixed_read_failure()) + return 1; if (test_partial_store_rejected()) return 1; if (test_ca_persists_across_starts()) From 7f6c39d12dc51293fe844fcd2db488c902203cfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Fri, 11 Sep 2026 11:21:53 +0200 Subject: [PATCH 11/18] Bound server I/O by shutdown, not by which direction stalled The accept loop arms SO_RCVTIMEO and SO_SNDTIMEO on every accepted connection, and the three retry loops around wolfSSL_accept(), wolfSSL_read() and wolfSSL_write() each treated one direction's expiry as resumable and the other's as fatal. TranslateIoReturnCode() maps a send-side EAGAIN or ETIMEDOUT to WOLFSSL_CBIO_ERR_WANT_WRITE, so: - wolfSSL_accept() continued on WANT_READ only, and a client whose receive window stalls the server's Certificate flight past the timeout had its handshake abandoned where a blocking socket would have waited. An ML-DSA chain is tens of kilobytes, so this is reachable. - wolfSSL_read() returns WANT_WRITE whenever the record layer must send first, which is what TLS 1.3 post-handshake auth and a key update do. - wolfSSL_write() had the mirror gap on WANT_READ. All three now resume on either direction through tls_want_io(). That alone would let a peer hold the handler indefinitely, because the loops observe the stopping flag only when an I/O call returns. A peer that keeps supplying data never lets a timeout expire, and the keep-alive loop checks the flag only between requests, so one trickled request header held shutdown for as long as the peer kept writing. Both helpers now check the flag before their first I/O. The two have to move together: a direction-agnostic retry without the stopping check is the trickling-peer hang, and the stopping check alone leaves the three one-directional loops. The idle-stop test could not tell a successful shutdown from a server that had already left wolfcert_server_run() on its own: stop_and_wait() polled a flag the serving thread could have set before stop() was called, and the run result was discarded, so a premature handshake or handler failure passed every case. ServerCtx carries run_rc, stop_and_wait() refuses a thread that has already returned, and each listener case asserts WOLFCERT_OK after the join. A fourth case drives a peer trickling an unterminated request header a byte at a time, sized to outlast the shutdown deadline several times over so a server that keeps consuming cannot pass by reaching the end of it. --- src/server.c | 34 +++++-- tests/integration/test_server_stop_idle.c | 119 ++++++++++++++++++---- 2 files changed, 126 insertions(+), 27 deletions(-) diff --git a/src/server.c b/src/server.c index ecd8b4c..7d8e037 100644 --- a/src/server.c +++ b/src/server.c @@ -51,21 +51,35 @@ #define WOLFCERT_SERVER_POLL_MS 200 #endif +/* A timeout armed on the accepted connection surfaces as WANT_READ or + * WANT_WRITE depending on which direction stalled, and either is resumable. */ +static int tls_want_io(WOLFSSL* ssl, int ret) +{ + int err = wolfSSL_get_error(ssl, ret); + + return err == WOLFSSL_ERROR_WANT_READ || err == WOLFSSL_ERROR_WANT_WRITE; +} + ssize_t wolfcert_io_recv(WolfCertServer* srv, int fd, void* buf, size_t len) { ssize_t r; + /* A trickling peer never times out, so the loops below never see this. */ + if (srv != NULL && WOLFSSL_ATOMIC_LOAD(srv->stopping)) + return -1; + /* A connection the accept loop armed carries a receive timeout, so its - * expiry is a retry rather than an error: wolfSSL reports it as WANT_READ, + * expiry is a retry rather than an error: wolfSSL reports it as a want, * a raw socket as EAGAIN. Retrying stops once shutdown is requested. */ if (srv != NULL && srv->tls_current != NULL) { int tr; + /* wolfSSL_read() wants a write whenever the record layer must send + * first, as post-handshake auth and a key update both do. */ do { tr = wolfSSL_read(srv->tls_current, buf, (int)len); } - while (tr <= 0 && - wolfSSL_get_error(srv->tls_current, tr) == WOLFSSL_ERROR_WANT_READ && + while (tr <= 0 && tls_want_io(srv->tls_current, tr) && !WOLFSSL_ATOMIC_LOAD(srv->stopping)); return tr <= 0 ? -1 : (ssize_t)tr; @@ -86,6 +100,9 @@ ssize_t wolfcert_io_send(WolfCertServer* srv, int fd, const void* buf, size_t le { ssize_t r; + if (srv != NULL && WOLFSSL_ATOMIC_LOAD(srv->stopping)) + return -1; + /* Mirrors wolfcert_io_recv: the send timeout bounds a peer that stops * reading, and its expiry is a retry rather than an error. Callers write * through send_all(), so a short write is already handled. */ @@ -95,8 +112,7 @@ ssize_t wolfcert_io_send(WolfCertServer* srv, int fd, const void* buf, size_t le do { tr = wolfSSL_write(srv->tls_current, buf, (int)len); } - while (tr <= 0 && - wolfSSL_get_error(srv->tls_current, tr) == WOLFSSL_ERROR_WANT_WRITE && + while (tr <= 0 && tls_want_io(srv->tls_current, tr) && !WOLFSSL_ATOMIC_LOAD(srv->stopping)); return tr <= 0 ? -1 : (ssize_t)tr; @@ -411,14 +427,12 @@ int wolfcert_server_run(WolfCertServer* srv) if (ssl != NULL) { wolfSSL_set_fd(ssl, cs); - /* A timed-out handshake read surfaces as WANT_READ, which - * is resumable: keep going until it completes, genuinely - * fails, or shutdown is requested. */ + /* A stalled flight is resumable either way round: a large + * chain blocks on the send timeout, not the receive one. */ do { ret = wolfSSL_accept(ssl); } - while (ret != WOLFSSL_SUCCESS && - wolfSSL_get_error(ssl, ret) == WOLFSSL_ERROR_WANT_READ && + while (ret != WOLFSSL_SUCCESS && tls_want_io(ssl, ret) && !WOLFSSL_ATOMIC_LOAD(srv->stopping)); if (ret == WOLFSSL_SUCCESS) { diff --git a/tests/integration/test_server_stop_idle.c b/tests/integration/test_server_stop_idle.c index 812095e..abbc4eb 100644 --- a/tests/integration/test_server_stop_idle.c +++ b/tests/integration/test_server_stop_idle.c @@ -22,7 +22,7 @@ * connects and then sends nothing must not pin the serving thread past * wolfcert_server_stop(). * - * Three idle points are exercised, each entered only once the peer has seen + * Four blocking points are exercised, each entered only once the peer has seen * the server reach it, so no case can pass without the server parked: * 1. A ClientHello answered by the server's flight, so the server is parked * in wolfSSL_accept() waiting for the rest of the handshake. @@ -30,13 +30,16 @@ * parked in the protocol handler's read. * 3. A served GetCACaps on a plaintext SCEP listener, so the keep-alive * loop is parked in recv() on the next request. + * 4. The same listener with a peer that trickles an unterminated request, so + * every receive succeeds and no timeout ever expires. * - * In each case the test calls wolfcert_server_stop() and requires - * wolfcert_server_run() to return inside a bounded wait; a thread still - * running at the deadline cannot be joined, so the test reports the - * failure and exits immediately rather than hanging. + * In each case the test requires the server to still be running, calls + * wolfcert_server_stop(), and requires wolfcert_server_run() to return + * WOLFCERT_OK inside a bounded wait; a thread still running at the deadline + * cannot be joined, so the test reports the failure and exits immediately + * rather than hanging. * - * A fourth case covers the other entry point: wolfcert_server_serve_fd() + * A fifth case covers the other entry point: wolfcert_server_serve_fd() * runs on a caller-supplied fd that the accept loop never armed, so a * would-block read there must fail instead of retrying forever. */ @@ -61,6 +64,7 @@ #include #include #include +#include #include #include #include @@ -80,6 +84,7 @@ typedef struct { WolfCertServer* srv; + int run_rc; /* Atomic, not volatile: the poll below needs a happens-before edge * against the serving thread, the same way srv->stopping does. */ wolfSSL_Atomic_Int returned; @@ -89,7 +94,7 @@ static void* server_thread(void* arg) { ServerCtx* ctx = (ServerCtx*)arg; - wolfcert_server_run(ctx->srv); + ctx->run_rc = wolfcert_server_run(ctx->srv); WOLFSSL_ATOMIC_STORE(ctx->returned, 1); return NULL; @@ -157,11 +162,15 @@ static int connect_after_getcacaps(uint16_t port) #endif /* WOLFCERT_HAVE_SCEP */ /* Stop the server and wait for its thread to leave wolfcert_server_run(). - * Returns 0 when it did, -1 on the deadline. */ + * Returns 0 when it did, -1 on the deadline, -2 if it had already returned -- + * a case that must not be scored as a successful shutdown. */ static int stop_and_wait(ServerCtx* ctx) { int waited; + if (WOLFSSL_ATOMIC_LOAD(ctx->returned)) + return -2; + if (wolfcert_server_stop(ctx->srv) != WOLFCERT_OK) return -1; @@ -175,8 +184,14 @@ static int stop_and_wait(ServerCtx* ctx) return -1; } -static int fail_stuck(const char* which) +static int fail_stop(const char* which, int rc) { + if (rc == -2) { + fprintf(stderr, "FAIL %s: server returned before stop() was called\n", + which); + return 1; + } + fprintf(stderr, "FAIL %s: did not return within %d ms\n", which, STOP_DEADLINE_MS); fflush(stderr); @@ -186,6 +201,38 @@ static int fail_stuck(const char* which) } #ifdef WOLFCERT_HAVE_SCEP +/* A peer that keeps supplying bytes never lets a receive time out, so the + * handler only leaves its read if the server checks the stopping flag. Sends + * a request header that never terminates, a byte at a time. */ +typedef struct { + int fd; + wolfSSL_Atomic_Int halt; +} TrickleCtx; + +/* Outlast the shutdown deadline several times over, so a server that keeps + * consuming cannot reach the end of the trickle and pass by timing out. */ +#define TRICKLE_STEP_MS 20 +#define TRICKLE_MAX ((STOP_DEADLINE_MS * 3) / TRICKLE_STEP_MS) + +static void* trickle_thread(void* arg) +{ + TrickleCtx* ctx = (TrickleCtx*)arg; + static const char head[] = "GET /?operation=GetCACaps&pad="; + int i; + + if (send(ctx->fd, head, sizeof(head) - 1, 0) != (ssize_t)(sizeof(head) - 1)) + return NULL; + + for (i = 0; i < TRICKLE_MAX && !WOLFSSL_ATOMIC_LOAD(ctx->halt); i++) { + if (send(ctx->fd, "a", 1, 0) != 1) + break; + + sleep_ms(TRICKLE_STEP_MS); + } + + return NULL; +} + typedef struct { WolfCertServer* srv; int fd; @@ -213,13 +260,20 @@ int main(void) ServerCtx ctx; pthread_t tid; TestTlsConn conn; + int stop_rc; #ifdef WOLFCERT_HAVE_SCEP ServeFdCtx serve_ctx; + TrickleCtx trickle; + pthread_t ttid; int fd; int sp[2]; int waited; #endif + /* The trickle peer keeps writing into a connection the server tears down + * on stop, so the test must survive the reset. */ + signal(SIGPIPE, SIG_IGN); + REQUIRE(wolfcert_init(NULL) == WOLFCERT_OK); REQUIRE(gen_server_identity(&tls_cert, &tls_cert_len, &tls_key, &tls_key_len) == 0); @@ -242,10 +296,12 @@ int main(void) tls_cert, tls_cert_len, STOP_DEADLINE_MS) == 0); - if (stop_and_wait(&ctx) != 0) - return fail_stuck("idle before handshake"); + stop_rc = stop_and_wait(&ctx); + if (stop_rc != 0) + return fail_stop("idle before handshake", stop_rc); REQUIRE(pthread_join(tid, NULL) == 0); + REQUIRE(ctx.run_rc == WOLFCERT_OK); test_tls_close(&conn); wolfcert_server_free(ctx.srv); @@ -257,10 +313,12 @@ int main(void) REQUIRE(test_tls_connect(&conn, wolfcert_server_port(ctx.srv), tls_cert, tls_cert_len) == 0); - if (stop_and_wait(&ctx) != 0) - return fail_stuck("idle after handshake"); + stop_rc = stop_and_wait(&ctx); + if (stop_rc != 0) + return fail_stop("idle after handshake", stop_rc); REQUIRE(pthread_join(tid, NULL) == 0); + REQUIRE(ctx.run_rc == WOLFCERT_OK); test_tls_close(&conn); wolfcert_server_free(ctx.srv); @@ -278,14 +336,41 @@ int main(void) fd = connect_after_getcacaps(wolfcert_server_port(ctx.srv)); REQUIRE(fd >= 0); - if (stop_and_wait(&ctx) != 0) - return fail_stuck("idle on plaintext listener"); + stop_rc = stop_and_wait(&ctx); + if (stop_rc != 0) + return fail_stop("idle on plaintext listener", stop_rc); + + REQUIRE(pthread_join(tid, NULL) == 0); + REQUIRE(ctx.run_rc == WOLFCERT_OK); + close(fd); + wolfcert_server_free(ctx.srv); + + /* 4. Parked in recv() with a peer that keeps trickling: no timeout ever + * expires, so only a stopping check can end the handler's read. */ + memset(&ctx, 0, sizeof(ctx)); + REQUIRE(wolfcert_server_start(&cfg, &ctx.srv) == WOLFCERT_OK); + REQUIRE(pthread_create(&tid, NULL, server_thread, &ctx) == 0); + + fd = connect_after_getcacaps(wolfcert_server_port(ctx.srv)); + REQUIRE(fd >= 0); + + memset(&trickle, 0, sizeof(trickle)); + trickle.fd = fd; + REQUIRE(pthread_create(&ttid, NULL, trickle_thread, &trickle) == 0); + sleep_ms(TRICKLE_STEP_MS * 5); + + stop_rc = stop_and_wait(&ctx); + WOLFSSL_ATOMIC_STORE(trickle.halt, 1); + REQUIRE(pthread_join(ttid, NULL) == 0); + if (stop_rc != 0) + return fail_stop("trickling peer", stop_rc); REQUIRE(pthread_join(tid, NULL) == 0); + REQUIRE(ctx.run_rc == WOLFCERT_OK); close(fd); wolfcert_server_free(ctx.srv); - /* 4. serve_fd() on a non-blocking fd the accept loop never armed: the + /* 5. serve_fd() on a non-blocking fd the accept loop never armed: the * read must surface the error instead of spinning on EAGAIN. */ memset(&ctx, 0, sizeof(ctx)); REQUIRE(wolfcert_server_start(&cfg, &ctx.srv) == WOLFCERT_OK); @@ -305,7 +390,7 @@ int main(void) } if (!WOLFSSL_ATOMIC_LOAD(serve_ctx.returned)) - return fail_stuck("serve_fd on a non-blocking fd"); + return fail_stop("serve_fd on a non-blocking fd", -1); REQUIRE(pthread_join(tid, NULL) == 0); close(sp[0]); From 85ed9be629eeb43b1119c324a908d65dd617dd8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Fri, 11 Sep 2026 11:22:00 +0200 Subject: [PATCH 12/18] Pin the givenName drop between CSR and issued certificate wolfcert_csr_build() accepts GN= and encodes it into the CSR, but the issued certificate never carries it: wolfSSL's GetRDN() reaches a subject component only through certNameSubject[], which is indexed by id - 3 and runs out at id 22, so ASN_GIVEN_NAME (0x2a) never reaches SetSubject() and dc.subjectGN stays NULL. The COPY_SUBJ in wolfcert_ca_issue() has nothing to read, and the RDN is dropped without an error. Nothing in the library can close that on its own, so the behaviour is pinned at both ends instead: the test asserts the CSR really does carry 2.5.4.42 and that the issued certificate really does lose it. When wolfSSL decodes the component the second assertion fails, which is the signal to drop the workaround comment in src/ca_issue.c. --- tests/integration/test_est_roundtrip.c | 52 +++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_est_roundtrip.c b/tests/integration/test_est_roundtrip.c index eefad7c..43fef2b 100644 --- a/tests/integration/test_est_roundtrip.c +++ b/tests/integration/test_est_roundtrip.c @@ -214,10 +214,58 @@ static int rdn_is(const char* p, int len, const char* want) memcmp(p, want, (size_t)len) == 0; } +/* givenName is the one RDN the CSR builder accepts that the CA cannot carry + * over: wolfSSL's GetRDN() reaches subjectGN only through a table that stops + * short of ASN_GIVEN_NAME, so the copy in wolfcert_ca_issue() reads nothing. + * Pinned at both ends, so the day wolfSSL decodes it this test fails and the + * workaround in src/ca_issue.c goes with it. */ +static int enroll_givenname_dropped(const WolfCertServerCfg* client_cfg) +{ + /* 2.5.4.42 (id-at-givenName), as it appears in an AttributeType OID. */ + static const uint8_t gn_oid[] = { 0x06, 0x03, 0x55, 0x04, 0x2a }; + WolfCertKeyCfg kcfg = { .type = TEST_ENROLL_KEY_TYPE, .param = TEST_ENROLL_KEY_PARAM, + .dev_id = WOLFCERT_DEVID_SOFTWARE }; + WolfCertKey* dk = NULL; + WolfCertCertMeta meta = { .subject_dn = "CN=device-gn,GN=Jane" }; + WolfCertBuffer csr = { 0 }; + WolfCertBuffer issued = { 0 }; + DerBuffer* der = NULL; + DecodedCert dc; + size_t i; + int in_csr = 0; + + REQUIRE(wolfcert_key_generate(&kcfg, &dk) == WOLFCERT_OK); + REQUIRE(wolfcert_csr_build(dk, &meta, &csr) == WOLFCERT_OK); + + for (i = 0; csr.len >= sizeof(gn_oid) && i <= csr.len - sizeof(gn_oid); ++i) { + if (memcmp(csr.data + i, gn_oid, sizeof(gn_oid)) == 0) + in_csr = 1; + } + REQUIRE(in_csr); + + REQUIRE(wolfcert_est_simple_enroll(client_cfg, csr.data, csr.len, &issued) + == WOLFCERT_OK); + REQUIRE(wc_PemToDer(issued.data, (long)issued.len, CERT_TYPE, &der, + NULL, NULL, NULL) == 0); + + wc_InitDecodedCert(&dc, der->buffer, der->length, NULL); + REQUIRE(wc_ParseCert(&dc, CERT_TYPE, NO_VERIFY, NULL) == 0); + REQUIRE(rdn_is(dc.subjectCN, dc.subjectCNLen, "device-gn")); + REQUIRE(dc.subjectGN == NULL && dc.subjectGNLen == 0); + + wc_FreeDecodedCert(&dc); + wc_FreeDer(&der); + wolfcert_buffer_free(&csr); + wolfcert_buffer_free(&issued); + wolfcert_key_free(dk); + return 0; +} + /* Regression: every subject RDN the CSR builder accepts must survive into the * issued certificate. The CA rebuilds the subject field by field, so a name * component it has no copy for is dropped without any error. givenName is - * absent below because wolfSSL's decoder never reports it back. */ + * absent below because wolfSSL's decoder never reports it back; see + * enroll_givenname_dropped(). */ static int enroll_check_subject_rdns(const WolfCertServerCfg* client_cfg) { WolfCertKeyCfg kcfg = { .type = TEST_ENROLL_KEY_TYPE, .param = TEST_ENROLL_KEY_PARAM, @@ -407,6 +455,8 @@ int main(void) return 1; /* Full subject-RDN round-trip guard. */ + if (enroll_givenname_dropped(&client_cfg)) + return 1; if (enroll_check_subject_rdns(&client_cfg)) return 1; From 6de13ddce443fa84f51a3ed1d73244c928e1ce9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Fri, 11 Sep 2026 11:40:12 +0200 Subject: [PATCH 13/18] Gate the server tests on the server, not on a protocol test_server_ca_store covers the CA store, which no protocol owns, but sat under WOLFCERT_ENABLE_SCEP. The CI matrix builds cmake-nonrsa-macos with SCEP off, where the whole file disappeared - including the only coverage of the Ed25519 and Ed448 reload path. test_server_stop_idle had the mirror problem: gated on EST while carrying a SCEP case. Both now build under WOLFCERT_ENABLE_SERVER alone, in CMake and automake. Neither test cares which protocol the listener speaks, so each picks one that was compiled in; EST has no plaintext mode, so the CA-store test mints a throwaway server identity for that variant. Verified by building with each protocol disabled in turn: 24/24 with SCEP off, 15/15 with EST off, both tests present and passing in each. --- Makefile.am | 18 ++++++----- tests/CMakeLists.txt | 24 +++++++++------ tests/integration/test_server_stop_idle.c | 10 +++++- tests/unit/test_server_ca_store.c | 37 ++++++++++++++++++++++- 4 files changed, 71 insertions(+), 18 deletions(-) diff --git a/Makefile.am b/Makefile.am index 54305b3..732b92a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -156,8 +156,7 @@ check_PROGRAMS += test_est_roundtrip test_est_tls_roundtrip test_est_mtls_roundt test_est_pha_roundtrip test_est_async_roundtrip \ test_est_csr_attrs_roundtrip test_est_csr_attrs_apply_roundtrip \ test_est_csr_attrs_enforce test_est_chunked_robustness \ - test_est_pending_roundtrip test_est_mldsa_roundtrip \ - test_server_stop_idle + test_est_pending_roundtrip test_est_mldsa_roundtrip test_est_roundtrip_SOURCES = tests/integration/test_est_roundtrip.c test_est_roundtrip_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread test_est_tls_roundtrip_SOURCES = tests/integration/test_est_tls_roundtrip.c @@ -180,8 +179,6 @@ test_est_chunked_robustness_SOURCES = tests/integration/test_est_chunked_robustn test_est_chunked_robustness_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread test_est_pending_roundtrip_SOURCES = tests/integration/test_est_pending_roundtrip.c test_est_pending_roundtrip_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread -test_server_stop_idle_SOURCES = tests/integration/test_server_stop_idle.c -test_server_stop_idle_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread endif endif @@ -191,8 +188,7 @@ test_scep_msg_SOURCES = tests/unit/test_scep_msg.c test_scep_msg_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/src test_scep_msg_LDADD = libwolfcert.la $(WOLFSSL_LIBS) if WOLFCERT_HAVE_SERVER -check_PROGRAMS += test_scep_roundtrip test_scep_poll_roundtrip test_scep_async_roundtrip \ - test_server_ca_store +check_PROGRAMS += test_scep_roundtrip test_scep_poll_roundtrip test_scep_async_roundtrip test_scep_roundtrip_SOURCES = tests/integration/test_scep_roundtrip.c test_scep_roundtrip_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/src test_scep_roundtrip_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread @@ -200,11 +196,19 @@ test_scep_poll_roundtrip_SOURCES = tests/integration/test_scep_poll_roundtrip.c test_scep_poll_roundtrip_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread test_scep_async_roundtrip_SOURCES = tests/integration/test_scep_async_roundtrip.c test_scep_async_roundtrip_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread +endif +endif + +# Protocol-agnostic server coverage: gated on the server alone, so a build with +# either protocol disabled keeps it. +if WOLFCERT_HAVE_SERVER +check_PROGRAMS += test_server_stop_idle test_server_ca_store +test_server_stop_idle_SOURCES = tests/integration/test_server_stop_idle.c +test_server_stop_idle_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread test_server_ca_store_SOURCES = tests/unit/test_server_ca_store.c test_server_ca_store_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/src test_server_ca_store_LDADD = libwolfcert.la $(WOLFSSL_LIBS) -lpthread endif -endif TESTS = $(check_PROGRAMS) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2c85ca4..90a052e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -101,14 +101,24 @@ if(WOLFCERT_ENABLE_EST) target_link_libraries(test_est_mldsa_roundtrip PRIVATE wolfcert Threads::Threads) add_test(NAME est_mldsa_roundtrip COMMAND test_est_mldsa_roundtrip) - add_executable(test_server_stop_idle integration/test_server_stop_idle.c) - target_link_libraries(test_server_stop_idle PRIVATE wolfcert Threads::Threads) - add_test(NAME server_stop_idle COMMAND test_server_stop_idle) - # A stop that cannot interrupt an idle peer hangs the serving thread. - set_tests_properties(server_stop_idle PROPERTIES TIMEOUT 60) endif() endif() +# Protocol-agnostic server coverage: gated on the server alone, so a build with +# either protocol disabled keeps it. +if(WOLFCERT_ENABLE_SERVER) + add_executable(test_server_stop_idle integration/test_server_stop_idle.c) + target_link_libraries(test_server_stop_idle PRIVATE wolfcert Threads::Threads) + add_test(NAME server_stop_idle COMMAND test_server_stop_idle) + # A stop that cannot interrupt an idle peer hangs the serving thread. + set_tests_properties(server_stop_idle PROPERTIES TIMEOUT 60) + + add_executable(test_server_ca_store unit/test_server_ca_store.c) + target_include_directories(test_server_ca_store PRIVATE ${CMAKE_SOURCE_DIR}/src) + target_link_libraries(test_server_ca_store PRIVATE wolfcert Threads::Threads) + add_test(NAME server_ca_store COMMAND test_server_ca_store) +endif() + if(WOLFCERT_ENABLE_SCEP) add_executable(test_scep_msg unit/test_scep_msg.c) target_include_directories(test_scep_msg PRIVATE ${CMAKE_SOURCE_DIR}/src) @@ -130,10 +140,6 @@ if(WOLFCERT_ENABLE_SCEP AND WOLFCERT_ENABLE_SERVER) target_link_libraries(test_scep_async_roundtrip PRIVATE wolfcert Threads::Threads) add_test(NAME scep_async_roundtrip COMMAND test_scep_async_roundtrip) - add_executable(test_server_ca_store unit/test_server_ca_store.c) - target_include_directories(test_server_ca_store PRIVATE ${CMAKE_SOURCE_DIR}/src) - target_link_libraries(test_server_ca_store PRIVATE wolfcert Threads::Threads) - add_test(NAME server_ca_store COMMAND test_server_ca_store) endif() # The CLI's protocol scoping and keyword validation. Most cases need no diff --git a/tests/integration/test_server_stop_idle.c b/tests/integration/test_server_stop_idle.c index abbc4eb..82dba5b 100644 --- a/tests/integration/test_server_stop_idle.c +++ b/tests/integration/test_server_stop_idle.c @@ -82,6 +82,14 @@ #define STOP_DEADLINE_MS 5000 #define POLL_STEP_MS 10 +/* The accept loop is what these cases exercise, so any compiled-in protocol + * will do for the TLS listener; SCEP is absent from any NO_RSA build. */ +#if defined(WOLFCERT_HAVE_EST) + #define TLS_LISTENER_PROTO WOLFCERT_PROTO_EST +#else + #define TLS_LISTENER_PROTO WOLFCERT_PROTO_SCEP +#endif + typedef struct { WolfCertServer* srv; int run_rc; @@ -279,7 +287,7 @@ int main(void) &tls_key, &tls_key_len) == 0); memset(&cfg, 0, sizeof(cfg)); - cfg.protocol = WOLFCERT_PROTO_EST; + cfg.protocol = TLS_LISTENER_PROTO; cfg.bind_host = "127.0.0.1"; cfg.bind_port = 0; cfg.tls_cert_pem = tls_cert; diff --git a/tests/unit/test_server_ca_store.c b/tests/unit/test_server_ca_store.c index a756f2d..78a8596 100644 --- a/tests/unit/test_server_ca_store.c +++ b/tests/unit/test_server_ca_store.c @@ -21,11 +21,13 @@ #include #include "internal.h" #include "../test_static_mem.h" +#include "../integration/tls_test_util.h" #include #include #include +#include #include #define REQUIRE(cond) \ @@ -44,10 +46,34 @@ #define CA_KEY_PARAM 0 #endif +/* The CA store is protocol-agnostic; the listener just needs a protocol that + * was compiled in, and SCEP is absent from any NO_RSA build. EST has no + * plaintext mode, so that variant carries a throwaway server identity. */ +#if defined(WOLFCERT_HAVE_EST) + #define CA_STORE_PROTO WOLFCERT_PROTO_EST + #define CA_STORE_NEEDS_TLS 1 +#else + #define CA_STORE_PROTO WOLFCERT_PROTO_SCEP + #define CA_STORE_NEEDS_TLS 0 +#endif + +#if CA_STORE_NEEDS_TLS +static uint8_t* srv_cert_pem; +static size_t srv_cert_pem_len; +static uint8_t* srv_key_pem; +static size_t srv_key_pem_len; +#endif + static void ca_store_cfg(WolfCertServerCfgSrv* cfg, WolfCertStoreOps* store) { memset(cfg, 0, sizeof(*cfg)); - cfg->protocol = WOLFCERT_PROTO_SCEP; + cfg->protocol = CA_STORE_PROTO; +#if CA_STORE_NEEDS_TLS + cfg->tls_cert_pem = srv_cert_pem; + cfg->tls_cert_pem_len = srv_cert_pem_len; + cfg->tls_key_pem = srv_key_pem; + cfg->tls_key_pem_len = srv_key_pem_len; +#endif cfg->bind_host = "127.0.0.1"; cfg->ca_store = store; cfg->ca_key_type = CA_KEY_TYPE; @@ -749,6 +775,11 @@ int main(void) REQUIRE(test_static_mem_init() == 0); REQUIRE(wolfcert_init(NULL) == WOLFCERT_OK); +#if CA_STORE_NEEDS_TLS + REQUIRE(gen_server_identity(&srv_cert_pem, &srv_cert_pem_len, + &srv_key_pem, &srv_key_pem_len) == 0); +#endif + if (test_corrupt_ca_rejected()) return 1; if (test_load_io_error_rejected()) @@ -778,6 +809,10 @@ int main(void) if (test_ca_key_usage()) return 1; +#if CA_STORE_NEEDS_TLS + free(srv_cert_pem); + free(srv_key_pem); +#endif wolfcert_cleanup(); printf("OK\n"); return 0; From eaee403da4f357680ded60210f40080803991dba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Fri, 11 Sep 2026 11:48:17 +0200 Subject: [PATCH 14/18] Carry street and the string encoding into the issued certificate wolfSSL has no API to move a decoded subject into a Cert, so the CA rebuilds it RDN by RDN and any component without a copy is dropped silently. Three gaps, now split out into wolfcert_copy_csr_subject(): - street (2.5.4.9) is decoded and encodable, and was simply not copied. It is also added to the CSR builder's RDN table, which had no key for it. - The DirectoryString choice was not carried, so wc_InitCert()'s UTF8String default replaced whatever the requester used. A PrintableString RDN came back as UTF8String, which is a different subject to anything comparing encoded names. - An RDN longer than the fixed CertName field was truncated at 63 bytes with no error, issuing a certificate stating a subject the CSR did not ask for. It is now refused with WOLFCERT_ERR_BAD_ARG. Not fixed, because they cannot be: the jurisdiction RDNs are decode-only in wolfSSL - CertName carries joiC and joiSt, but the generator's nameOid[] table and GetOneCertName() have no entry for either, so a copy would never emit. Confirmed by generating a request with both set and finding neither the value nor the JOI OID prefix in the DER. A one-line note records it so the copy is not "restored" later. The over-long case is driven through wolfcert_copy_csr_subject() directly: no wolfSSL-built CSR can carry an RDN past CTC_NAME_SIZE, so there is no CSR to enroll. The encoding case goes end to end through a request built on wolfSSL with PrintableString set explicitly. --- Makefile.am | 1 + src/ca_issue.c | 76 +++++++++++++++++--------- src/csr.c | 1 + src/internal.h | 3 + tests/CMakeLists.txt | 1 + tests/integration/test_est_roundtrip.c | 64 +++++++++++++++++++++- tests/integration/tls_test_util.h | 18 ++++++ tests/unit/test_csr.c | 36 ++++++++++++ 8 files changed, 174 insertions(+), 26 deletions(-) diff --git a/Makefile.am b/Makefile.am index 732b92a..a01ec1d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -125,6 +125,7 @@ test_smoke_LDADD = libwolfcert.la $(WOLFSSL_LIBS) test_keygen_SOURCES = tests/unit/test_keygen.c test_keygen_LDADD = libwolfcert.la $(WOLFSSL_LIBS) test_csr_SOURCES = tests/unit/test_csr.c +test_csr_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/src test_csr_LDADD = libwolfcert.la $(WOLFSSL_LIBS) test_store_SOURCES = tests/unit/test_store.c test_store_LDADD = libwolfcert.la $(WOLFSSL_LIBS) diff --git a/src/ca_issue.c b/src/ca_issue.c index ebe3f6e..20e8e19 100644 --- a/src/ca_issue.c +++ b/src/ca_issue.c @@ -695,15 +695,57 @@ static int flatten_csr_san(DecodedCert* dc, Cert* nc, void* heap) return WOLFCERT_OK; } +/* Truncating would issue a certificate stating a subject the CSR did not ask + * for, so an over-long RDN is refused instead. */ #define COPY_SUBJ(field, dst) \ - do { \ - if (dc.field != NULL && dc.field##Len > 0) { \ - size_t n = (size_t)dc.field##Len < CTC_NAME_SIZE - 1 \ - ? (size_t)dc.field##Len : CTC_NAME_SIZE - 1; \ - memcpy(dst, dc.field, n); dst[n] = '\0'; \ - } \ + do { \ + if (dc->field != NULL && dc->field##Len > 0) { \ + if ((size_t)dc->field##Len >= sizeof(dst)) \ + return WOLFCERT_ERR(WOLFCERT_ERR_BAD_ARG, "ca", \ + "CSR subject %s is %d bytes, limit %d", #field, \ + dc->field##Len, (int)sizeof(dst) - 1); \ + memcpy(dst, dc->field, (size_t)dc->field##Len); \ + dst[dc->field##Len] = '\0'; \ + } \ } while (0) +#define COPY_SUBJ_E(field, dst, encdst) \ + do { \ + COPY_SUBJ(field, dst); \ + if (dc->field != NULL && dc->field##Len > 0) \ + encdst = dc->field##Enc; \ + } while (0) + +/* wolfSSL has no API to carry a decoded subject into a Cert, so it is rebuilt + * RDN by RDN and a component with no copy here is dropped without an error. */ +WOLFCERT_TEST_VIS int wolfcert_copy_csr_subject(const DecodedCert* dc, Cert* nc) +{ + COPY_SUBJ_E(subjectCN, nc->subject.commonName, nc->subject.commonNameEnc); + COPY_SUBJ_E(subjectO, nc->subject.org, nc->subject.orgEnc); + COPY_SUBJ_E(subjectOU, nc->subject.unit, nc->subject.unitEnc); + COPY_SUBJ_E(subjectC, nc->subject.country, nc->subject.countryEnc); + COPY_SUBJ_E(subjectST, nc->subject.state, nc->subject.stateEnc); + COPY_SUBJ_E(subjectL, nc->subject.locality, nc->subject.localityEnc); + COPY_SUBJ_E(subjectStreet, nc->subject.street, nc->subject.streetEnc); + COPY_SUBJ_E(subjectSN, nc->subject.sur, nc->subject.surEnc); + /* wolfSSL's GetRDN() reaches subjectGN only through a table that stops + * short of ASN_GIVEN_NAME, so this copy has nothing to read yet. */ + COPY_SUBJ_E(subjectGN, nc->subject.givenName, nc->subject.givenNameEnc); + COPY_SUBJ(subjectEmail, nc->subject.email); + COPY_SUBJ_E(subjectSND, nc->subject.serialDev, nc->subject.serialDevEnc); + COPY_SUBJ_E(subjectUID, nc->subject.userId, nc->subject.userIdEnc); + COPY_SUBJ_E(subjectPC, nc->subject.postalCode, nc->subject.postalCodeEnc); +#ifdef WOLFSSL_CERT_EXT + COPY_SUBJ_E(subjectBC, nc->subject.busCat, nc->subject.busCatEnc); + /* No subjectJC/subjectJS: wolfSSL decodes the jurisdiction RDNs but its + * generator has no encoder entry for them, so a copy would never emit. */ +#endif + return WOLFCERT_OK; +} + +#undef COPY_SUBJ_E +#undef COPY_SUBJ + int wolfcert_ca_issue(WolfCertCa* ca, const uint8_t* csr_der, size_t csr_len, uint8_t** out_cert, size_t* out_len) @@ -748,25 +790,9 @@ int wolfcert_ca_issue(WolfCertCa* ca, if (rc == 0) { wc_InitCert_ex(nc, heap, WOLFCERT_DEVID_SOFTWARE); - COPY_SUBJ(subjectCN, nc->subject.commonName); - COPY_SUBJ(subjectO, nc->subject.org); - COPY_SUBJ(subjectOU, nc->subject.unit); - COPY_SUBJ(subjectC, nc->subject.country); - COPY_SUBJ(subjectST, nc->subject.state); - COPY_SUBJ(subjectL, nc->subject.locality); - COPY_SUBJ(subjectSN, nc->subject.sur); - /* wolfSSL only stores subject ids up to ASN_USER_ID, so givenName - * never arrives; copy it anyway for when that gap closes. */ - COPY_SUBJ(subjectGN, nc->subject.givenName); - COPY_SUBJ(subjectEmail, nc->subject.email); - COPY_SUBJ(subjectSND, nc->subject.serialDev); - COPY_SUBJ(subjectUID, nc->subject.userId); - COPY_SUBJ(subjectPC, nc->subject.postalCode); -#ifdef WOLFSSL_CERT_EXT - COPY_SUBJ(subjectBC, nc->subject.busCat); -#endif - - if (wc_SetIssuerBuffer(nc, ca->cert_der, (int)ca->cert_der_len) != 0) + rc = wolfcert_copy_csr_subject(&dc, nc); + if (rc == WOLFCERT_OK && + wc_SetIssuerBuffer(nc, ca->cert_der, (int)ca->cert_der_len) != 0) rc = WOLFCERT_ERR_CRYPTO; } diff --git a/src/csr.c b/src/csr.c index cdde43b..d21966f 100644 --- a/src/csr.c +++ b/src/csr.c @@ -73,6 +73,7 @@ static const struct rdn_field rdn_fields[] = { { "UID", 3, offsetof(CertName, userId), CTC_NAME_SIZE }, { "userId", 6, offsetof(CertName, userId), CTC_NAME_SIZE }, { "postalCode", 10, offsetof(CertName, postalCode), CTC_NAME_SIZE }, + { "street", 6, offsetof(CertName, street), CTC_NAME_SIZE }, #ifdef WOLFSSL_CERT_EXT { "businessCategory", 16, offsetof(CertName, busCat), CTC_NAME_SIZE }, #endif diff --git a/src/internal.h b/src/internal.h index 28981d8..cb93ece 100644 --- a/src/internal.h +++ b/src/internal.h @@ -40,6 +40,7 @@ #if defined(HAVE_GETPID) && !defined(WOLFSSL_NO_GETPID) #include #endif +#include #include #include #include @@ -159,6 +160,8 @@ WOLFCERT_TEST_VIS int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, void* heap); int wolfcert_ca_save(const WolfCertCa* ca, WolfCertStoreOps* store); WOLFCERT_TEST_VIS void wolfcert_ca_free(WolfCertCa* ca); +/* Rebuild an issued certificate's subject from a decoded CSR. */ +WOLFCERT_TEST_VIS int wolfcert_copy_csr_subject(const DecodedCert* dc, Cert* nc); WOLFCERT_TEST_VIS int wolfcert_ca_issue(WolfCertCa* ca, const uint8_t* csr_der, size_t csr_len, uint8_t** out_cert, size_t* out_len); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 90a052e..c822392 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -9,6 +9,7 @@ target_link_libraries(test_keygen PRIVATE wolfcert) add_test(NAME keygen COMMAND test_keygen) add_executable(test_csr unit/test_csr.c) +target_include_directories(test_csr PRIVATE ${CMAKE_SOURCE_DIR}/src) target_link_libraries(test_csr PRIVATE wolfcert) add_test(NAME csr COMMAND test_csr) diff --git a/tests/integration/test_est_roundtrip.c b/tests/integration/test_est_roundtrip.c index 43fef2b..c0ecc20 100644 --- a/tests/integration/test_est_roundtrip.c +++ b/tests/integration/test_est_roundtrip.c @@ -208,6 +208,65 @@ static int enroll_check_san(const WolfCertServerCfg* client_cfg) return 0; } +/* wolfcert_csr_build() leaves wolfSSL's UTF8String default in place, so a + * PrintableString CSR has to be built straight on wolfSSL. */ +static int enroll_raw_csr(const WolfCertServerCfg* client_cfg, + int want_rc, DecodedCert* dc, DerBuffer** der) +{ + WC_RNG rng; + test_signkey key; + Cert req; + byte csr[8192]; + WolfCertBuffer issued = { 0 }; + int sz; + + REQUIRE(wc_InitRng(&rng) == 0); + REQUIRE(test_signkey_make(&key, &rng) == 0); + + REQUIRE(wc_InitCert(&req) == 0); + strcpy(req.subject.commonName, "device-enc"); + req.subject.commonNameEnc = CTC_PRINTABLE; + strcpy(req.subject.org, "Acme"); + req.subject.orgEnc = CTC_PRINTABLE; + req.sigType = TEST_CERT_SIGTYPE; + + sz = test_sign_certreq(&req, csr, sizeof(csr), &key, &rng); + REQUIRE(sz > 0); + + REQUIRE(wolfcert_est_simple_enroll(client_cfg, csr, (size_t)sz, &issued) + == want_rc); + + if (want_rc == WOLFCERT_OK) { + REQUIRE(wc_PemToDer(issued.data, (long)issued.len, CERT_TYPE, der, + NULL, NULL, NULL) == 0); + wc_InitDecodedCert(dc, (*der)->buffer, (*der)->length, NULL); + REQUIRE(wc_ParseCert(dc, CERT_TYPE, NO_VERIFY, NULL) == 0); + } + + wolfcert_buffer_free(&issued); + test_signkey_free(&key); + wc_FreeRng(&rng); + return 0; +} + +/* The DirectoryString choice the requester used has to survive: a + * PrintableString RDN coming back as UTF8String changes the subject. */ +static int enroll_preserves_string_encoding(const WolfCertServerCfg* client_cfg) +{ + DecodedCert dc; + DerBuffer* der = NULL; + + if (enroll_raw_csr(client_cfg, WOLFCERT_OK, &dc, &der)) + return 1; + + REQUIRE(dc.subjectCNEnc == CTC_PRINTABLE); + REQUIRE(dc.subjectOEnc == CTC_PRINTABLE); + + wc_FreeDecodedCert(&dc); + wc_FreeDer(&der); + return 0; +} + static int rdn_is(const char* p, int len, const char* want) { return p != NULL && len == (int)strlen(want) && @@ -275,7 +334,7 @@ static int enroll_check_subject_rdns(const WolfCertServerCfg* client_cfg) WolfCertCertMeta meta = { .subject_dn = "CN=device-rdn,O=Acme,OU=Devices,C=US,ST=Washington," - "L=Seattle,SN=Doe," + "L=Seattle,street=1 Pike Place,SN=Doe," "emailAddress=jane@example.com,serialNumber=SRL-42," "UID=factory-1,postalCode=98109" #ifdef WOLFSSL_CERT_EXT @@ -309,6 +368,7 @@ static int enroll_check_subject_rdns(const WolfCertServerCfg* client_cfg) REQUIRE(rdn_is(dc.subjectSND, dc.subjectSNDLen, "SRL-42")); REQUIRE(rdn_is(dc.subjectUID, dc.subjectUIDLen, "factory-1")); REQUIRE(rdn_is(dc.subjectPC, dc.subjectPCLen, "98109")); + REQUIRE(rdn_is(dc.subjectStreet, dc.subjectStreetLen, "1 Pike Place")); #ifdef WOLFSSL_CERT_EXT REQUIRE(rdn_is(dc.subjectBC, dc.subjectBCLen, "Manufacturing")); #endif @@ -457,6 +517,8 @@ int main(void) /* Full subject-RDN round-trip guard. */ if (enroll_givenname_dropped(&client_cfg)) return 1; + if (enroll_preserves_string_encoding(&client_cfg)) + return 1; if (enroll_check_subject_rdns(&client_cfg)) return 1; diff --git a/tests/integration/tls_test_util.h b/tests/integration/tls_test_util.h index fd96c7a..4ef6a62 100644 --- a/tests/integration/tls_test_util.h +++ b/tests/integration/tls_test_util.h @@ -133,6 +133,24 @@ static inline int test_sign_selfcert(Cert* cert, uint8_t* der, int der_sz, #endif } +/* Sign the (already populated) CSR into `der`. Returns the signed DER length, + * or <= 0 on error. */ +static inline int test_sign_certreq(Cert* req, uint8_t* der, int der_sz, + test_signkey* key, WC_RNG* rng) +{ +#if !defined(NO_RSA) + if (wc_MakeCertReq(req, der, (word32)der_sz, key, NULL) <= 0) + return -1; + return wc_SignCert(req->bodySz, req->sigType, der, (word32)der_sz, + key, NULL, rng); +#else + if (wc_MakeCertReq(req, der, (word32)der_sz, NULL, key) <= 0) + return -1; + return wc_SignCert(req->bodySz, req->sigType, der, (word32)der_sz, + NULL, key, rng); +#endif +} + /* Serialize the private key to DER. Returns DER length, or <= 0 on error. */ static inline int test_signkey_to_der(test_signkey* key, uint8_t* der, int der_sz) diff --git a/tests/unit/test_csr.c b/tests/unit/test_csr.c index 3273d72..600f276 100644 --- a/tests/unit/test_csr.c +++ b/tests/unit/test_csr.c @@ -21,6 +21,7 @@ #define _DARWIN_C_SOURCE /* expose memmem/strcasestr/INADDR_LOOPBACK on macOS */ #include +#include "internal.h" #include "../test_static_mem.h" #include @@ -144,10 +145,45 @@ static int build_with_extras(void) return 0; } +#ifdef WOLFCERT_HAVE_SERVER +/* A CSR RDN longer than wolfSSL's fixed CertName field must be refused, not + * issued truncated: a certificate naming a subject the CSR did not ask for is + * worse than a failed enrolment. Driven directly because no wolfSSL-built CSR + * can carry an over-long RDN in the first place. */ +static int subject_copy_rejects_oversized_rdn(void) +{ + char longcn[CTC_NAME_SIZE + 8]; + DecodedCert dc; + Cert nc; + + memset(longcn, 'A', sizeof(longcn)); + memset(&dc, 0, sizeof(dc)); + REQUIRE(wc_InitCert(&nc) == 0); + + dc.subjectCN = longcn; + dc.subjectCNLen = CTC_NAME_SIZE; + dc.subjectCNEnc = CTC_PRINTABLE; + REQUIRE(wolfcert_copy_csr_subject(&dc, &nc) == WOLFCERT_ERR_BAD_ARG); + + /* One byte under the limit still copies, and carries its encoding. */ + dc.subjectCNLen = CTC_NAME_SIZE - 1; + REQUIRE(wolfcert_copy_csr_subject(&dc, &nc) == WOLFCERT_OK); + REQUIRE(strlen(nc.subject.commonName) == CTC_NAME_SIZE - 1); + REQUIRE(nc.subject.commonNameEnc == CTC_PRINTABLE); + + return 0; +} +#endif + int main(void) { REQUIRE(test_static_mem_init() == 0); REQUIRE(wolfcert_init(NULL) == WOLFCERT_OK); + +#ifdef WOLFCERT_HAVE_SERVER + if (subject_copy_rejects_oversized_rdn()) + return 1; +#endif #ifdef WOLFCERT_HAVE_ECC if (build_and_reparse(WOLFCERT_KEY_ECC, 256)) return 1; From 5bfce12e89b327859f37fa4fb9814abf01243c91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Fri, 11 Sep 2026 11:49:15 +0200 Subject: [PATCH 15/18] Mark basicConstraints critical on the generated CA RFC 5280 section 4.2.1.9: "Conforming CAs MUST include this extension in all CA certificates that contain public keys used to validate digital signatures on certificates and MUST mark the extension as critical in such certificates." wolfSSL emits the extension whenever Cert.isCA is set but leaves it non-critical unless basicConstCrit is set too, which the generator never did, so a relying party configured to reject a non-critical basicConstraints on a CA would refuse the chain. ca_key_usage_set() now also requires the extension to be present, critical and asserting CA:TRUE, alongside the key usages it already checked. The keyUsage citation in both files pointed at RFC 5280 section 4.2.1.3, which only defines the bit names that the usage string already spells out. The MUST that mandates this exact set, RSA conditional included, is RFC 8894 section 2.1.2. --- src/ca_issue.c | 5 ++++- tests/unit/test_server_ca_store.c | 10 +++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/ca_issue.c b/src/ca_issue.c index 20e8e19..27f81c3 100644 --- a/src/ca_issue.c +++ b/src/ca_issue.c @@ -91,11 +91,14 @@ static int gen_self_signed_cert(WolfCertCa* ca) snprintf(cert->subject.country, sizeof(cert->subject.country), "%s", "US"); cert->isCA = 1; + /* RFC 5280 section 4.2.1.9 MUST: a CA whose key validates certificate + * signatures marks basicConstraints critical. */ + cert->basicConstCrit = 1; cert->selfSigned = 1; cert->daysValid = 3650; cert->sigType = alg->ctc_sig_default; - /* RFC 5280 section 4.2.1.3: this key signs certificates and SCEP CertReps, + /* RFC 8894 section 2.1.2: this key signs certificates and SCEP CertReps, * and on RSA it also decrypts the pkcsPKIEnvelope. */ const char* usage = ca->type == WOLFCERT_KEY_RSA ? "keyCertSign,cRLSign,digitalSignature,keyEncipherment" diff --git a/tests/unit/test_server_ca_store.c b/tests/unit/test_server_ca_store.c index 78a8596..bc0e46f 100644 --- a/tests/unit/test_server_ca_store.c +++ b/tests/unit/test_server_ca_store.c @@ -611,9 +611,10 @@ static int test_mismatched_ca_rejected(void) return 0; } -/* RFC 5280 section 4.2.1.3: the CA signs certificates and, for SCEP, both +/* RFC 8894 section 2.1.2 MUST: the CA signs certificates and, for SCEP, both * signs CertReps and decrypts the pkcsPKIEnvelope, so its certificate must - * assert those usages rather than omitting the extension. */ + * assert those usages rather than omitting the extension. RFC 5280 section + * 4.2.1.9 MUST: basicConstraints is present and critical on such a CA. */ static int ca_key_usage_set(WolfCertKeyType type) { WolfCertStoreOps* store = wolfcert_store_memory_open(NULL); @@ -630,7 +631,10 @@ static int ca_key_usage_set(WolfCertKeyType type) wc_InitDecodedCert(&dc, cert.data, (word32)cert.len, NULL); REQUIRE(wc_ParseCert(&dc, CERT_TYPE, NO_VERIFY, NULL) == 0); - if (dc.extKeyUsageSet == 0 || + if (dc.extBasicConstSet == 0 || dc.extBasicConstCrit == 0 || dc.isCA == 0) { + rc = 1; + } + else if (dc.extKeyUsageSet == 0 || (dc.extKeyUsage & KEYUSE_KEY_CERT_SIGN) == 0 || (dc.extKeyUsage & KEYUSE_CRL_SIGN) == 0 || (dc.extKeyUsage & KEYUSE_DIGITAL_SIG) == 0) { From 3ceaea7fff79a58a8ed96b552c7d87212d8bf486 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Fri, 11 Sep 2026 11:50:41 +0200 Subject: [PATCH 16/18] Correct three RFC citations and drop the restated contracts Three of the section numbers added in this series point at the wrong text: - http.c's fragment note cited RFC 7230 section 5.3, which never mentions the fragment. The synthesized leading slash is section 5.3.1; excluding the fragment from the target is section 5.1. - host_is_ip_literal() cited RFC 3986 section 3.2.2 for both the URL and the Host header. 3986 governs the URL host; the Host field is RFC 7230 section 5.4. - The CA keyUsage set cited RFC 5280 section 4.2.1.3, which only defines the bit names the usage string already spells out. The MUST mandating this set is RFC 8894 section 2.1.2. The same contract restated at every site that touches it is a second copy to keep in sync, so four of those copies are cut to a line saying what the local code does. The EST-requires-TLS contract now carries its section number once, on the field and function that declare it in wolfcert/server.h. --- src/http.c | 10 +++++----- src/server.c | 2 -- tests/integration/test_est_tls_roundtrip.c | 4 +--- tests/unit/test_http.c | 8 +++----- tests/unit/test_server_ca_store.c | 6 ++---- wolfcert/server.h | 5 +++-- 6 files changed, 14 insertions(+), 21 deletions(-) diff --git a/src/http.c b/src/http.c index d0fa037..59a0dd7 100644 --- a/src/http.c +++ b/src/http.c @@ -102,8 +102,8 @@ WOLFCERT_TEST_VIS void wolfcert_http_url_free(WolfCertUrl* u) } /* wolfcert_http_url_parse stores an IPv6 literal with its brackets stripped, so - * a host carrying a colon is one: anything re-emitted into a URL or a Host - * header has to bracket it again (RFC 3986 section 3.2.2). */ + * a host carrying a colon is one: re-emitting it needs the brackets back, in a + * URL (RFC 3986 section 3.2.2) and in a Host header (RFC 7230 section 5.4). */ static int host_is_ip_literal(const char* host) { return strchr(host, ':') != NULL; @@ -254,9 +254,9 @@ WOLFCERT_TEST_VIS int wolfcert_http_url_parse(const char* url, WolfCertUrl* out, host_end = end; } - /* A pathless authority may still be followed by a query or a fragment, so - * the request target gets a synthesized leading slash. RFC 7230 section - * 5.3: the fragment is client-side only and never goes on the wire. */ + /* RFC 7230 section 5.3.1 synthesizes the leading slash for an empty path; + * section 5.1 excludes the fragment from the target, so it never goes on + * the wire. */ const char* frag = strchr(host_end, '#'); size_t tlen = frag ? (size_t)(frag - host_end) : strlen(host_end); size_t plen = (*host_end == '/') ? tlen : tlen + 1; diff --git a/src/server.c b/src/server.c index 7d8e037..ef0a309 100644 --- a/src/server.c +++ b/src/server.c @@ -235,8 +235,6 @@ int wolfcert_server_start(const WolfCertServerCfgSrv* cfg, WolfCertServer** out) if (ops == NULL) return WOLFCERT_ERR_UNSUPPORTED; - /* RFC 7030 has no plaintext mode. Reject here rather than in tls_setup(), - * which runs after the CA has been generated and written to the store. */ if (cfg->protocol == WOLFCERT_PROTO_EST && (cfg->tls_cert_pem == NULL || cfg->tls_key_pem == NULL)) return WOLFCERT_ERR(WOLFCERT_ERR_TLS, "server", diff --git a/tests/integration/test_est_tls_roundtrip.c b/tests/integration/test_est_tls_roundtrip.c index b4bfa43..939a229 100644 --- a/tests/integration/test_est_tls_roundtrip.c +++ b/tests/integration/test_est_tls_roundtrip.c @@ -71,9 +71,7 @@ int main(void) size_t tls_key_len = 0; REQUIRE(gen_server_identity(&tls_cert, &tls_cert_len, &tls_key, &tls_key_len) == 0); - /* RFC 7030 has no plaintext mode, so an EST listener configured without a - * TLS identity must be refused at start rather than serve /simpleenroll - * over cleartext HTTP. */ + /* An EST listener with no TLS identity must be refused at start. */ WolfCertStoreOps* plain_store = wolfcert_store_memory_open(NULL); REQUIRE(plain_store != NULL); WolfCertServerCfgSrv plain = { diff --git a/tests/unit/test_http.c b/tests/unit/test_http.c index d2fc3dc..8dd2e4c 100644 --- a/tests/unit/test_http.c +++ b/tests/unit/test_http.c @@ -97,8 +97,7 @@ static int test_url_parser(void) REQUIRE(strcmp(u.path, "/?operation=GetCACaps") == 0); wolfcert_http_url_free(&u); - /* RFC 7230 section 5.3: the fragment is client-side only, so it must not - * reach the request target. */ + /* A fragment must not reach the request target. */ REQUIRE(wolfcert_http_url_parse("http://ca.example#frag", &u, NULL) == WOLFCERT_OK); REQUIRE(strcmp(u.host, "ca.example") == 0); REQUIRE(strcmp(u.path, "/") == 0); @@ -728,9 +727,8 @@ static int test_request_transfer_encoding(void) return 0; } -/* RFC 3986 section 3.2.2: an IPv6 literal stays bracketed in the Host header, - * or a virtual-host match against "::1:8443" fails. Both request builders - * carry their own copy of the bracketing, so drive each one. */ +/* Both request builders carry their own copy of the bracketing, so drive + * each one: an unbracketed IPv6 Host header fails a virtual-host match. */ static int ipv6_host_header(int use_session) { struct capture_ctx cc = { 0 }; diff --git a/tests/unit/test_server_ca_store.c b/tests/unit/test_server_ca_store.c index bc0e46f..6627f1e 100644 --- a/tests/unit/test_server_ca_store.c +++ b/tests/unit/test_server_ca_store.c @@ -611,10 +611,8 @@ static int test_mismatched_ca_rejected(void) return 0; } -/* RFC 8894 section 2.1.2 MUST: the CA signs certificates and, for SCEP, both - * signs CertReps and decrypts the pkcsPKIEnvelope, so its certificate must - * assert those usages rather than omitting the extension. RFC 5280 section - * 4.2.1.9 MUST: basicConstraints is present and critical on such a CA. */ +/* The generated CA asserts the key usages a relying party checks, and a + * critical basicConstraints. */ static int ca_key_usage_set(WolfCertKeyType type) { WolfCertStoreOps* store = wolfcert_store_memory_open(NULL); diff --git a/wolfcert/server.h b/wolfcert/server.h index c4494f8..274340f 100644 --- a/wolfcert/server.h +++ b/wolfcert/server.h @@ -57,8 +57,9 @@ typedef struct { * the protocol handler. tls_client_ca_pem, when set, enables mutual * TLS (WOLFSSL_VERIFY_PEER) against the supplied client-CA bundle. * - * Mandatory for WOLFCERT_PROTO_EST (RFC 7030 has no plaintext mode): - * wolfcert_server_start() returns WOLFCERT_ERR_TLS without them. + * Mandatory for WOLFCERT_PROTO_EST, which RFC 7030 section 3.1 defines + * over TLS only: wolfcert_server_start() returns WOLFCERT_ERR_TLS + * without them. * Optional for WOLFCERT_PROTO_SCEP, which authenticates at the * pkiMessage layer and may be served over cleartext HTTP. * From 475452f0949e5f5e702d669416beffe11c6a9aff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Fri, 11 Sep 2026 11:54:22 +0200 Subject: [PATCH 17/18] Share the helpers that had grown second copies Four quantities or routines had more than one spelling, and the divergence of two of them is what earlier commits in this series were fixing: - The response allowance (body cap plus header budget) was open-coded in the blocking reader and computed by nb_rx_max() in the non-blocking one. Now one rx_max(size_t) that both call. - ca_key_buf_free() in ca_issue.c was the exact body of the wipe-then-free already written twice in store.c, and being file-local neither could use it. Now wolfcert_buffer_free_secure(), beside wolfcert_buffer_free(). - host_is_ip_literal() answered "is this an IPv6 literal" with a colon search, correct only because of an invariant set in another function. wolfcert_parse_ip() answers it directly and is used 150 lines away. - nanosleep() was open-coded three times across the integration tests. Now test_sleep_ms() in tls_test_util.h, which all three include. listen_loopback()/listen_loopback6() are left alone: they differ in address family, sockaddr type and the setsockopt each needs, so folding them costs more branching than the copy. --- src/ca_issue.c | 16 ++-------- src/http.c | 30 +++++++++++-------- src/internal.h | 3 ++ src/store.c | 6 ++-- src/wolfcert.c | 21 +++++++++++++ .../integration/test_est_chunked_robustness.c | 20 ++++--------- tests/integration/test_server_stop_idle.c | 17 +++-------- tests/integration/test_tls_http.c | 3 +- tests/integration/tls_test_util.h | 10 +++++++ 9 files changed, 66 insertions(+), 60 deletions(-) diff --git a/src/ca_issue.c b/src/ca_issue.c index 27f81c3..24fb99b 100644 --- a/src/ca_issue.c +++ b/src/ca_issue.c @@ -269,16 +269,6 @@ static int ca_check_stored_pair(const WolfCertKeyAlg* alg, WolfCertKey* key, return rc; } -/* The stored bytes are the CA private key, so every exit wipes them before - * releasing the buffer, as wolfcert_ca_free() does for the loaded copy. */ -static void ca_key_buf_free(WolfCertBuffer* key_buf) -{ - if (key_buf->data != NULL && key_buf->len > 0) - wc_ForceZero(key_buf->data, (word32)key_buf->len); - - wolfcert_buffer_free(key_buf); -} - int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, void* heap) { if (ca == NULL || store == NULL) @@ -296,7 +286,7 @@ int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, void* heap) if (cert_rc != WOLFCERT_OK || key_rc != WOLFCERT_OK) { wolfcert_buffer_free(&cert_buf); - ca_key_buf_free(&key_buf); + wolfcert_buffer_free_secure(&key_buf); if (cert_rc == WOLFCERT_ERR_NOT_FOUND && key_rc == WOLFCERT_ERR_NOT_FOUND) return WOLFCERT_ERR_NOT_FOUND; @@ -335,7 +325,7 @@ int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, void* heap) if (rc != WOLFCERT_OK) { a->free_(&shim); wolfcert_buffer_free(&cert_buf); - ca_key_buf_free(&key_buf); + wolfcert_buffer_free_secure(&key_buf); return rc; } @@ -353,7 +343,7 @@ int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, void* heap) } wolfcert_buffer_free(&cert_buf); - ca_key_buf_free(&key_buf); + wolfcert_buffer_free_secure(&key_buf); return WOLFCERT_ERR(WOLFCERT_ERR_PARSE, "ca", "stored CA key does not decode as any supported algorithm"); } diff --git a/src/http.c b/src/http.c index 59a0dd7..b44736a 100644 --- a/src/http.c +++ b/src/http.c @@ -101,12 +101,15 @@ WOLFCERT_TEST_VIS void wolfcert_http_url_free(WolfCertUrl* u) u->scheme = u->host = u->path = NULL; } -/* wolfcert_http_url_parse stores an IPv6 literal with its brackets stripped, so - * a host carrying a colon is one: re-emitting it needs the brackets back, in a - * URL (RFC 3986 section 3.2.2) and in a Host header (RFC 7230 section 5.4). */ +/* An IPv6 literal is stored with its brackets stripped, and re-emitting it + * needs them back in a URL (RFC 3986 section 3.2.2) and in a Host header + * (RFC 7230 section 5.4). */ static int host_is_ip_literal(const char* host) { - return strchr(host, ':') != NULL; + uint8_t ip[16]; + size_t ip_len = 0; + + return wolfcert_parse_ip(host, ip, &ip_len) == WOLFCERT_OK && ip_len == 16; } /* Build the "scheme://host[:port]" origin for a parsed URL into a freshly @@ -1114,13 +1117,19 @@ static int http_write_request(WolfCertConn* c, const WolfCertUrl* u, * requests - HTTP/1.1 pipelining is effectively dead on the wire, and * the async state machine has its own per-request residual tracking * that doesn't depend on this helper. */ +/* The response allowance: the body cap the caller asked for, plus the header + * budget. Both readers size their buffer from this one spelling. */ +static size_t rx_max(size_t max_body) +{ + return max_body + WOLFCERT_HTTP_HEADER_BUDGET; +} + static int http_read_response(WolfCertConn* c, size_t max_body, WolfCertHttpResponse* resp, void* heap) { - DynBuf rx = { .heap = heap, - .max = max_body + WOLFCERT_HTTP_HEADER_BUDGET }; + DynBuf rx = { .heap = heap, .max = rx_max(max_body) }; int hdr_end = read_headers(c, &rx); if (hdr_end < 0) { WOLFCERT_XFREE(rx.buf, heap); @@ -1486,11 +1495,6 @@ static int nb_write(WolfCertConn* c, const uint8_t* buf, size_t len, size_t* off } /* Total accumulator allowance: the body cap plus the header budget. */ -static size_t nb_rx_max(const WolfCertHttpSession* s) -{ - return s->max_body + WOLFCERT_HTTP_HEADER_BUDGET; -} - /* Ensure the rx buffer has room for `need` more bytes. */ static int nb_rx_reserve(WolfCertHttpSession* s, size_t need) { @@ -1498,7 +1502,7 @@ static int nb_rx_reserve(WolfCertHttpSession* s, size_t need) if (want <= s->sm_rx_cap) return WOLFCERT_OK; - size_t max = nb_rx_max(s); + size_t max = rx_max(s->max_body); if (want > max) return WOLFCERT_ERR_PROTOCOL; @@ -1528,7 +1532,7 @@ static int nb_read_some(WolfCertHttpSession* s, int* ended) /* Read at most what the allowance still permits, so a response that * ends inside the final quantum is not rejected before it is read. */ - size_t room = nb_rx_max(s) - s->sm_rx_len; + size_t room = rx_max(s->max_body) - s->sm_rx_len; uint8_t probe; uint8_t* dst; diff --git a/src/internal.h b/src/internal.h index cb93ece..82e42ac 100644 --- a/src/internal.h +++ b/src/internal.h @@ -160,6 +160,9 @@ WOLFCERT_TEST_VIS int wolfcert_ca_load(WolfCertCa* ca, WolfCertStoreOps* store, void* heap); int wolfcert_ca_save(const WolfCertCa* ca, WolfCertStoreOps* store); WOLFCERT_TEST_VIS void wolfcert_ca_free(WolfCertCa* ca); +/* wolfcert_buffer_free() after wiping: for a buffer that held key material. */ +void wolfcert_buffer_free_secure(WolfCertBuffer* buf); + /* Rebuild an issued certificate's subject from a decoded CSR. */ WOLFCERT_TEST_VIS int wolfcert_copy_csr_subject(const DecodedCert* dc, Cert* nc); WOLFCERT_TEST_VIS int wolfcert_ca_issue(WolfCertCa* ca, const uint8_t* csr_der, diff --git a/src/store.c b/src/store.c index 0fd7459..618d467 100644 --- a/src/store.c +++ b/src/store.c @@ -444,8 +444,7 @@ int wolfcert_store_write_key(WolfCertStoreOps* store, const char* key_name, return rc; rc = store->write(store->ctx, key_name, pem.data, pem.len, 1); - wc_ForceZero(pem.data, (word32)pem.len); - wolfcert_buffer_free(&pem); + wolfcert_buffer_free_secure(&pem); return rc; } @@ -464,8 +463,7 @@ int wolfcert_store_read_key(WolfCertStoreOps* store, const char* key_name, return rc; rc = wolfcert_key_from_pem(pem.data, pem.len, store->heap, out_key); - wc_ForceZero(pem.data, (word32)pem.len); - wolfcert_buffer_free(&pem); + wolfcert_buffer_free_secure(&pem); return rc; } diff --git a/src/wolfcert.c b/src/wolfcert.c index 1722d8e..85b7a40 100644 --- a/src/wolfcert.c +++ b/src/wolfcert.c @@ -59,3 +59,24 @@ void wolfcert_buffer_free(WolfCertBuffer* buf) buf->len = 0; buf->heap = NULL; } + +#define WOLFCERT_FORCEZERO_CHUNK 0x10000000U + +void wolfcert_buffer_free_secure(WolfCertBuffer* buf) +{ + if (buf != NULL && buf->data != NULL && buf->len > 0) { + uint8_t* p = buf->data; + size_t left = buf->len; + + while (left > 0) { + word32 chunk = (left > WOLFCERT_FORCEZERO_CHUNK) + ? WOLFCERT_FORCEZERO_CHUNK : (word32)left; + + wc_ForceZero(p, chunk); + p += chunk; + left -= chunk; + } + } + + wolfcert_buffer_free(buf); +} diff --git a/tests/integration/test_est_chunked_robustness.c b/tests/integration/test_est_chunked_robustness.c index d9ef56b..d17fb72 100644 --- a/tests/integration/test_est_chunked_robustness.c +++ b/tests/integration/test_est_chunked_robustness.c @@ -113,18 +113,6 @@ static int send_and_read_status(uint16_t port, return (int)n; } -/* Sleep helper that nudges the request segments below toward landing in - * distinct recv() calls rather than being coalesced into one buffer. - * Best-effort only: TCP guarantees no recv() boundaries, so this just - * makes the intended segmentation likely, not certain. */ -static void nap_ms(long ms) -{ - struct timespec ts; - ts.tv_sec = ms / 1000; - ts.tv_nsec = (ms % 1000) * 1000000L; - nanosleep(&ts, NULL); -} - /* Shape #1: a chunk-size line longer than 8 hex digits. The parser * must reject this rather than letting the shift-accumulate silently * wrap. */ @@ -220,9 +208,11 @@ static int accept_multisegment_chunked_body(uint16_t port) REQUIRE(test_tls_connect(&c, port, g_tls_cert, g_tls_cert_len) == 0); REQUIRE(test_tls_write(&c, hdr, strlen(hdr)) == 0); - nap_ms(80); + /* Best-effort segmentation: TCP guarantees no recv() boundaries, so the + * sleeps only make the intended split likely. */ + test_sleep_ms(80); (void)test_tls_write(&c, seg2, strlen(seg2)); - nap_ms(80); + test_sleep_ms(80); (void)test_tls_write(&c, seg3, strlen(seg3)); while (n + 1 < sizeof(status)) { @@ -342,7 +332,7 @@ static int keepalive_after_split_trailer(uint16_t port) /* Request #1: last-chunk line first, trailer CRLF withheld into its * own segment so a premature "0\r\n" completion leaves it unread. */ REQUIRE(test_tls_write(&c, req1_head, req1_head_len) == 0); - nap_ms(80); + test_sleep_ms(80); REQUIRE(test_tls_write(&c, req1_tail, strlen(req1_tail)) == 0); /* Wait for request #1's response head before sending request #2. */ diff --git a/tests/integration/test_server_stop_idle.c b/tests/integration/test_server_stop_idle.c index 82dba5b..4cf54f3 100644 --- a/tests/integration/test_server_stop_idle.c +++ b/tests/integration/test_server_stop_idle.c @@ -108,15 +108,6 @@ static void* server_thread(void* arg) return NULL; } -static void sleep_ms(int ms) -{ - struct timespec ts; - - ts.tv_sec = ms / 1000; - ts.tv_nsec = (long)(ms % 1000) * 1000000L; - nanosleep(&ts, NULL); -} - #ifdef WOLFCERT_HAVE_SCEP /* Connect to 127.0.0.1:port. Returns the fd, or -1. */ static int connect_loopback(uint16_t port) @@ -186,7 +177,7 @@ static int stop_and_wait(ServerCtx* ctx) if (WOLFSSL_ATOMIC_LOAD(ctx->returned)) return 0; - sleep_ms(POLL_STEP_MS); + test_sleep_ms(POLL_STEP_MS); } return -1; @@ -235,7 +226,7 @@ static void* trickle_thread(void* arg) if (send(ctx->fd, "a", 1, 0) != 1) break; - sleep_ms(TRICKLE_STEP_MS); + test_sleep_ms(TRICKLE_STEP_MS); } return NULL; @@ -365,7 +356,7 @@ int main(void) memset(&trickle, 0, sizeof(trickle)); trickle.fd = fd; REQUIRE(pthread_create(&ttid, NULL, trickle_thread, &trickle) == 0); - sleep_ms(TRICKLE_STEP_MS * 5); + test_sleep_ms(TRICKLE_STEP_MS * 5); stop_rc = stop_and_wait(&ctx); WOLFSSL_ATOMIC_STORE(trickle.halt, 1); @@ -394,7 +385,7 @@ int main(void) for (waited = 0; waited < STOP_DEADLINE_MS && !WOLFSSL_ATOMIC_LOAD(serve_ctx.returned); waited += POLL_STEP_MS) { - sleep_ms(POLL_STEP_MS); + test_sleep_ms(POLL_STEP_MS); } if (!WOLFSSL_ATOMIC_LOAD(serve_ctx.returned)) diff --git a/tests/integration/test_tls_http.c b/tests/integration/test_tls_http.c index 74976ad..fc7ecb3 100644 --- a/tests/integration/test_tls_http.c +++ b/tests/integration/test_tls_http.c @@ -136,8 +136,7 @@ int main(void) pthread_t tid; REQUIRE(pthread_create(&tid, NULL, srv_thread, &sc) == 0); for (int i = 0; i < 200 && WOLFSSL_ATOMIC_LOAD(sc.port) == 0; ++i) { - const struct timespec ts = { 0, 5 * 1000 * 1000 }; - nanosleep(&ts, NULL); + test_sleep_ms(5); } REQUIRE(WOLFSSL_ATOMIC_LOAD(sc.port) != 0); diff --git a/tests/integration/tls_test_util.h b/tests/integration/tls_test_util.h index 4ef6a62..835be7b 100644 --- a/tests/integration/tls_test_util.h +++ b/tests/integration/tls_test_util.h @@ -50,8 +50,18 @@ #include #include #include +#include #include +static inline void test_sleep_ms(long ms) +{ + struct timespec ts; + + ts.tv_sec = ms / 1000; + ts.tv_nsec = (ms % 1000) * 1000000L; + nanosleep(&ts, NULL); +} + /* A key algorithm + parameter the current build supports, for client * enrollments where the algorithm is incidental to what the test verifies. */ #if defined(WOLFCERT_HAVE_ECC) From 54cd99fa0234c597eee1b7f5cf585286612b6d5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Fri, 11 Sep 2026 11:57:43 +0200 Subject: [PATCH 18/18] Address the non-blocking review points - WOLFCERT_SERVER_POLL_MS meant both the listener's shutdown cadence and the per-connection socket timeout. Lowering it for faster shutdown also turned every peer read into a retry spin. The timeout is now WOLFCERT_SERVER_IO_TIMEOUT_MS, defaulting to the cadence. - The ML-DSA #error sat in key_algs.c rather than with the other resolved feature validation. Moved to check_config.h, which has to include dilithium.h for it: WOLFSSL_MLDSA_CHECK_KEY is resolved there, not in options.h. CLAUDE.md's hard-fail list and configure notes record it. - ecc_pub_check() put two MAX_ECC_BYTES-derived arrays on the stack unconditionally and discarded wc_ecc_make_pub()'s return. Both buffers come off the heap in one allocation, and a failed derivation is an error. - key_algs.h said pub_check "may adopt pub into key (ML-DSA)". Four of the five mutate: Ed25519 and Ed448 import the verified public half and ECC derives one. The note now says so, since it is what makes freeing a failed key load-bearing. - nb_read_some() used dst == &probe as a mode flag in two places. A named `probing` int says it once. --- CLAUDE.md | 7 ++++++- src/http.c | 13 ++++++++----- src/key_algs.c | 40 ++++++++++++++++++++++++++-------------- src/key_algs.h | 7 ++++--- src/server.c | 23 ++++++++++++++--------- 5 files changed, 58 insertions(+), 32 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 34f7a0f..c4e53c4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -32,7 +32,12 @@ configure time if the installed wolfSSL lacks any of `HAVE_PKCS7`, `WOLFSSL_KEY_GEN`, `WOLF_CRYPTO_CB`, `WOLFSSL_BASE64_ENCODE`, `OPENSSL_EXTRA`, `WOLFSSL_ALT_NAMES`, or `WOLFSSL_CERT_NAME_ALL`, or if it was built with `NO_AES` / `NO_SHA256`, or if it provides neither -TLS 1.2 nor TLS 1.3. +TLS 1.2 nor TLS 1.3. With ML-DSA enabled it additionally needs +`WOLFSSL_MLDSA_CHECK_KEY` (`wc_MlDsaKey_CheckKey()`), which reloading an +ML-DSA CA from a store calls -- checked when `src/key_algs.c` compiles, +since only `dilithium.h` resolves that macro. `--enable-mldsa` gives it by +default; it is lost only if wolfSSL is built with +`WOLFSSL_DILITHIUM_NO_CHECK_KEY` or `WOLFSSL_MLDSA_VERIFY_ONLY`. **Key algorithms are gated** by `WOLFCERT_HAVE_` (RSA, ECC, ED25519, ED448, MLDSA). RSA, ECC, Ed25519, Ed448 and ML-DSA are each diff --git a/src/http.c b/src/http.c index b44736a..200bab8 100644 --- a/src/http.c +++ b/src/http.c @@ -1535,12 +1535,15 @@ static int nb_read_some(WolfCertHttpSession* s, int* ended) size_t room = rx_max(s->max_body) - s->sm_rx_len; uint8_t probe; uint8_t* dst; + int probing = 0; if (room == 0) { /* An EOF-delimited body ending exactly on the allowance is legal, so - * a full accumulator still has to look for the close. */ - dst = &probe; - room = 1; + * a full accumulator still has to look for the close. Any byte that + * arrives instead puts the response over the allowance. */ + dst = &probe; + room = 1; + probing = 1; } else { if (room > WOLFCERT_HTTP_READ_CHUNK) @@ -1556,7 +1559,7 @@ static int nb_read_some(WolfCertHttpSession* s, int* ended) if (s->conn.ssl) { int r = wolfSSL_read(s->conn.ssl, dst, (int)room); if (r > 0) { - if (dst == &probe) + if (probing) return WOLFCERT_ERR_PROTOCOL; s->sm_rx_len += (size_t)r; @@ -1586,7 +1589,7 @@ static int nb_read_some(WolfCertHttpSession* s, int* ended) if (r > 0) { if ((size_t)r > room) return WOLFCERT_ERR_IO; - if (dst == &probe) + if (probing) return WOLFCERT_ERR_PROTOCOL; s->sm_rx_len += (size_t)r; diff --git a/src/key_algs.c b/src/key_algs.c index 66753f8..fcf3769 100644 --- a/src/key_algs.c +++ b/src/key_algs.c @@ -38,6 +38,11 @@ #endif #ifdef WOLFCERT_HAVE_MLDSA # include +/* Checked here, not in check_config.h: dilithium.h derives this macro only + * once settings.h has been parsed. */ +# ifndef WOLFSSL_MLDSA_CHECK_KEY +# error "wolfSSL is missing wc_MlDsaKey_CheckKey(); wolfCert's ML-DSA support needs it. Rebuild wolfSSL without WOLFSSL_DILITHIUM_NO_CHECK_KEY / WOLFSSL_MLDSA_VERIFY_ONLY." +# endif #endif #include @@ -216,27 +221,40 @@ static int ecc_pub_check(struct WolfCertKey* k, const uint8_t* pub, word32 pub_len) { ecc_key* cert_key; - byte mine[ECC_X963_CAP]; - byte theirs[ECC_X963_CAP]; - word32 mine_len = sizeof(mine); - word32 theirs_len = sizeof(theirs); + byte* mine; + byte* theirs; + word32 mine_len = ECC_X963_CAP; + word32 theirs_len = ECC_X963_CAP; word32 idx = 0; int rc; + mine = (byte*)WOLFCERT_XMALLOC(2 * ECC_X963_CAP, k->heap); + if (mine == NULL) + return WOLFCERT_ERR_MEMORY; + theirs = mine + ECC_X963_CAP; + cert_key = (ecc_key*)WOLFCERT_XMALLOC(sizeof(*cert_key), k->heap); - if (cert_key == NULL) + if (cert_key == NULL) { + WOLFCERT_XFREE(mine, k->heap); return WOLFCERT_ERR_MEMORY; + } rc = wc_ecc_init_ex(cert_key, k->heap, k->dev_id); if (rc != 0) { WOLFCERT_XFREE(cert_key, k->heap); + WOLFCERT_XFREE(mine, k->heap); return WOLFCERT_ERR_WC(rc, "keygen", "ecc_init_ex"); } /* A SEC1 private key need not carry its public point, so derive it when * the decoder did not supply one. */ - if (((ecc_key*)k->impl)->type == ECC_PRIVATEKEY_ONLY) - (void)wc_ecc_make_pub((ecc_key*)k->impl, NULL); + if (((ecc_key*)k->impl)->type == ECC_PRIVATEKEY_ONLY) { + rc = wc_ecc_make_pub((ecc_key*)k->impl, NULL); + if (rc != 0) { + rc = WOLFCERT_ERR_WC(rc, "keygen", "ecc_make_pub"); + goto out; + } + } rc = wc_EccPublicKeyDecode(pub, &idx, cert_key, pub_len); if (rc != 0) @@ -257,6 +275,7 @@ static int ecc_pub_check(struct WolfCertKey* k, const uint8_t* pub, out: wc_ecc_free(cert_key); WOLFCERT_XFREE(cert_key, k->heap); + WOLFCERT_XFREE(mine, k->heap); return rc; } @@ -492,13 +511,6 @@ static int mldsa_priv_to_der(const struct WolfCertKey* k, uint8_t* buf, word32 c return wc_MlDsaKey_PrivateKeyToDer((MlDsaKey*)k->impl, buf, cap); } -/* Reloading an ML-DSA CA from a store needs wc_MlDsaKey_CheckKey(); say so - * here rather than letting the link fail. */ -#ifndef WOLFSSL_MLDSA_CHECK_KEY -#error "wolfCert's ML-DSA support needs wc_MlDsaKey_CheckKey(): rebuild wolfSSL " \ - "without WOLFSSL_DILITHIUM_NO_CHECK_KEY / WOLFSSL_MLDSA_VERIFY_ONLY." -#endif - /* Unlike its siblings this hook mutates `key`: the certificate's public half * is adopted into it, since none can be derived from a PKCS#8 v1 private key. * A key that fails the check therefore carries an unverified public half and diff --git a/src/key_algs.h b/src/key_algs.h index cf387c4..ae1480a 100644 --- a/src/key_algs.h +++ b/src/key_algs.h @@ -59,9 +59,10 @@ typedef struct WolfCertKeyAlg { * Returns written length (>0) on success, negative on error. */ int (*priv_to_der)(const struct WolfCertKey* key, uint8_t* buf, word32 cap); - /* Confirm the private key belongs to the given public key. An - * implementation may adopt `pub` into `key` when the private encoding - * carries no public half (ML-DSA), so a key that fails must be freed. */ + /* Confirm the private key belongs to the given public key. Most + * implementations mutate `key` to do it - importing the verified public + * half (Ed25519/Ed448, ML-DSA) or deriving it (ECC) - so a key that fails + * carries an unverified public half and must be freed. */ int (*pub_check) (struct WolfCertKey* key, const uint8_t* pub, word32 pub_len); /* wc_*_free + free(key->impl). */ diff --git a/src/server.c b/src/server.c index ef0a309..cf9c987 100644 --- a/src/server.c +++ b/src/server.c @@ -43,14 +43,19 @@ #include -/* Shutdown cadence: how often wolfcert_server_run() wakes to re-check the - * stopping flag while idle at the listener, and the send/receive timeouts put - * on an accepted connection so a stalled peer cannot hold the handler. Bounds - * shutdown latency; not performance-critical. */ +/* How often wolfcert_server_run() wakes to re-check the stopping flag while + * idle at the listener. Bounds shutdown latency; not performance-critical. */ #ifndef WOLFCERT_SERVER_POLL_MS #define WOLFCERT_SERVER_POLL_MS 200 #endif +/* Send/receive timeout on an accepted connection, so a stalled peer cannot + * hold the handler. Separate from the listener cadence: every expiry is a + * retry, so lowering this spins the handler rather than speeding shutdown. */ +#ifndef WOLFCERT_SERVER_IO_TIMEOUT_MS +#define WOLFCERT_SERVER_IO_TIMEOUT_MS WOLFCERT_SERVER_POLL_MS +#endif + /* A timeout armed on the accepted connection surfaces as WANT_READ or * WANT_WRITE depending on which direction stalled, and either is resumable. */ static int tls_want_io(WOLFSSL* ssl, int ret) @@ -406,8 +411,8 @@ int wolfcert_server_run(WolfCertServer* srv) /* Bound how long a read or write on this connection can block, so a * peer that goes silent or stops reading cannot hold the handler past * wolfcert_server_stop(). */ - poll_to.tv_sec = WOLFCERT_SERVER_POLL_MS / 1000; - poll_to.tv_usec = (WOLFCERT_SERVER_POLL_MS % 1000) * 1000; + poll_to.tv_sec = WOLFCERT_SERVER_IO_TIMEOUT_MS / 1000; + poll_to.tv_usec = (WOLFCERT_SERVER_IO_TIMEOUT_MS % 1000) * 1000; if (setsockopt(cs, SOL_SOCKET, SO_RCVTIMEO, &poll_to, sizeof(poll_to)) != 0 || setsockopt(cs, SOL_SOCKET, SO_SNDTIMEO, &poll_to, @@ -490,9 +495,9 @@ int wolfcert_server_stop(WolfCertServer* srv) if (srv == NULL) return WOLFCERT_ERR_BAD_ARG; - /* Signal the accept loop to exit. Both the listener poll and the reads on - * an accepted connection use a WOLFCERT_SERVER_POLL_MS timeout and - * re-check this flag, so no fd surgery is needed here -- + /* Signal the accept loop to exit. The listener poll and the reads on an + * accepted connection are both bounded and re-check this flag, so no fd + * surgery is needed here -- * wolfcert_server_free() closes listen_fd after the serving thread is * joined. Setting the flag from another thread (test harness) or a signal * handler (wolfcert-server CLI) is safe: the store is atomic. */