commit deadd9e1311204415754dcfa404bec4bf3cd557c from: Omar Polo date: Fri Jun 09 09:28:26 2023 UTC readd proxy certs and `require client ca' support Was temporarly disabled during the transition to real privsep. While here, fix a memory leak when using `require client ca'. Also, avoid leaking info about the parent address space layout to server processes by not sending pointer values. commit - c144b1b6f831446f82e201db1ab7fadab4cf11f0 commit + deadd9e1311204415754dcfa404bec4bf3cd557c blob - b37fed0bd10cb69ce224619d98146a6f13136850 blob + 3eaff8416924b089325ab0d9409af849c1c2fd90 --- config.c +++ config.c @@ -88,6 +88,8 @@ config_free(void) if (l->dirfd != -1) close(l->dirfd); + free(l->reqca_path); + X509_STORE_free(l->reqca); free(l); } @@ -103,8 +105,12 @@ config_free(void) TAILQ_FOREACH_SAFE(p, &h->proxies, proxies, tp) { TAILQ_REMOVE(&h->proxies, p, proxies); - tls_unload_file(p->cert, p->certlen); - tls_unload_file(p->key, p->keylen); + free(p->cert_path); + free(p->cert); + free(p->key_path); + free(p->key); + free(p->reqca_path); + X509_STORE_free(p->reqca); free(p); } @@ -116,7 +122,7 @@ config_free(void) } static int -config_send_file(struct privsep *ps, int fd, int type) +config_send_file(struct privsep *ps, int type, int fd, void *data, size_t l) { int n, m, id, d; @@ -124,18 +130,33 @@ config_send_file(struct privsep *ps, int fd, int type) n = -1; proc_range(ps, id, &n, &m); for (n = 0; n < m; ++n) { - if ((d = dup(fd)) == -1) - fatal("dup"); - if (proc_compose_imsg(ps, id, n, type, -1, d, NULL, 0) + d = -1; + if (fd != -1 && (d = dup(fd)) == -1) + fatal("dup %d", fd); + if (proc_compose_imsg(ps, id, n, type, -1, d, data, l) == -1) return -1; } - close(fd); + if (fd != -1) + close(fd); return 0; } static int +config_open_send(struct privsep *ps, int type, const char *path) +{ + int fd; + + log_debug("sending %s", path); + + if ((fd = open(path, O_RDONLY)) == -1) + fatal("can't open %s", path); + + return config_send_file(ps, type, fd, NULL, 0); +} + +static int make_socket(int port, int family) { int sock, v; @@ -199,7 +220,7 @@ config_send_socks(struct conf *conf) if ((sock = make_socket(conf->port, AF_INET)) == -1) return -1; - if (config_send_file(ps, sock, IMSG_RECONF_SOCK4) == -1) + if (config_send_file(ps, IMSG_RECONF_SOCK4, sock, NULL, 0) == -1) return -1; if (!conf->ipv6) @@ -208,7 +229,7 @@ config_send_socks(struct conf *conf) if ((sock = make_socket(conf->port, AF_INET6)) == -1) return -1; - if (config_send_file(ps, sock, IMSG_RECONF_SOCK6) == -1) + if (config_send_file(ps, IMSG_RECONF_SOCK6, sock, NULL, 0) == -1) return -1; return 0; @@ -276,26 +297,40 @@ config_send(struct conf *conf, struct fcgi *fcgi, stru log_debug("sending certificate %s", h->cert_path); if ((fd = open(h->cert_path, O_RDONLY)) == -1) fatal("can't open %s", h->cert_path); - if (config_send_file(ps, fd, IMSG_RECONF_CERT) == -1) + if (config_send_file(ps, IMSG_RECONF_CERT, fd, NULL, 0) == -1) return -1; log_debug("sending key %s", h->key_path); if ((fd = open(h->key_path, O_RDONLY)) == -1) fatal("can't open %s", h->key_path); - if (config_send_file(ps, fd, IMSG_RECONF_KEY) == -1) + if (config_send_file(ps, IMSG_RECONF_KEY, fd, NULL, 0) == -1) return -1; if (h->ocsp_path != NULL) { log_debug("sending ocsp %s", h->ocsp_path); if ((fd = open(h->ocsp_path, O_RDONLY)) == -1) fatal("can't open %s", h->ocsp_path); - if (config_send_file(ps, fd, IMSG_RECONF_OCSP) == -1) + if (config_send_file(ps, IMSG_RECONF_OCSP, fd, + NULL, 0) == -1) return -1; } TAILQ_FOREACH(l, &h->locations, locations) { - if (proc_compose(ps, PROC_SERVER, IMSG_RECONF_LOC, - l, sizeof(*l)) == -1) + struct location lcopy; + int fd = -1; + + memcpy(&lcopy, l, sizeof(lcopy)); + lcopy.reqca_path = NULL; + lcopy.reqca = NULL; + lcopy.dirfd = -1; + memset(&lcopy.locations, 0, sizeof(lcopy.locations)); + + if (l->reqca_path != NULL && + (fd = open(l->reqca_path, O_RDONLY)) == -1) + fatal("can't open %s", l->reqca_path); + + if (config_send_file(ps, IMSG_RECONF_LOC, fd, + &lcopy, sizeof(lcopy)) == -1) return -1; } @@ -321,9 +356,41 @@ config_send(struct conf *conf, struct fcgi *fcgi, stru return -1; TAILQ_FOREACH(p, &h->proxies, proxies) { - if (proc_compose(ps, PROC_SERVER, IMSG_RECONF_PROXY, - p, sizeof(*p)) == -1) + struct proxy pcopy; + int fd = -1; + + memcpy(&pcopy, p, sizeof(pcopy)); + pcopy.cert_path = NULL; + pcopy.cert = NULL; + pcopy.certlen = 0; + pcopy.key_path = NULL; + pcopy.key = NULL; + pcopy.keylen = 0; + pcopy.reqca_path = NULL; + pcopy.reqca = NULL; + + if (p->reqca_path != NULL) { + fd = open(p->reqca_path, O_RDONLY); + if (fd == -1) + fatal("can't open %s", p->reqca_path); + } + + if (config_send_file(ps, IMSG_RECONF_PROXY, fd, + &pcopy, sizeof(pcopy)) == -1) return -1; + + if (p->cert_path != NULL && + config_open_send(ps, IMSG_RECONF_PROXY_CERT, + p->cert_path) == -1) + return -1; + + if (p->key_path != NULL && + config_open_send(ps, IMSG_RECONF_PROXY_KEY, + p->key_path) == -1) + return -1; + + if (proc_flush_imsg(ps, PROC_SERVER, -1) == -1) + return -1; } if (proc_flush_imsg(ps, PROC_SERVER, -1) == -1) @@ -372,6 +439,7 @@ int config_recv(struct conf *conf, struct imsg *imsg) { static struct vhost *h; + static struct proxy *p; struct privsep *ps = conf->ps; struct etm m; struct fcgi *f; @@ -388,6 +456,7 @@ config_recv(struct conf *conf, struct imsg *imsg) case IMSG_RECONF_START: config_free(); h = NULL; + p = NULL; break; case IMSG_RECONF_MIME: @@ -451,6 +520,9 @@ config_recv(struct conf *conf, struct imsg *imsg) strlcpy(vh->domain, vht.domain, sizeof(vh->domain)); h = vh; TAILQ_INSERT_TAIL(&hosts, h, vhosts); + + /* reset proxy */ + p = NULL; break; case IMSG_RECONF_CERT: @@ -503,8 +575,13 @@ config_recv(struct conf *conf, struct imsg *imsg) loc->fcgi = -1; memcpy(loc, imsg->data, datalen); - loc->dirfd = -1; /* XXX */ - loc->reqca = NULL; /* XXX */ + + if (imsg->fd != -1) { + loc->reqca = load_ca(imsg->fd); + if (loc->reqca == NULL) + fatalx("failed to load CA"); + } + TAILQ_INSERT_TAIL(&h->locations, loc, locations); break; @@ -533,12 +610,43 @@ config_recv(struct conf *conf, struct imsg *imsg) IMSG_SIZE_CHECK(imsg, proxy); proxy = xcalloc(1, sizeof(*proxy)); memcpy(proxy, imsg->data, datalen); - proxy->reqca = NULL; /* XXX */ - proxy->cert = proxy->key = NULL; /* XXX */ - proxy->certlen = proxy->keylen = 0; /* XXX */ + + if (imsg->fd != -1) { + proxy->reqca = load_ca(imsg->fd); + if (proxy->reqca == NULL) + fatal("failed to load CA"); + } + TAILQ_INSERT_TAIL(&h->proxies, proxy, proxies); + p = proxy; break; + case IMSG_RECONF_PROXY_CERT: + log_debug("receiving proxy cert"); + if (p == NULL) + fatalx("recv'd proxy cert without proxy"); + if (p->cert != NULL) + fatalx("proxy cert already received"); + if (imsg->fd == -1) + fatalx("no fd for IMSG_RECONF_PROXY_CERT"); + if (load_file(imsg->fd, &p->cert, &p->certlen) == -1) + fatalx("failed to load cert for proxy %s of %s", + p->host, h->domain); + break; + + case IMSG_RECONF_PROXY_KEY: + log_debug("receiving proxy key"); + if (p == NULL) + fatalx("recv'd proxy key without proxy"); + if (p->key != NULL) + fatalx("proxy key already received"); + if (imsg->fd == -1) + fatalx("no fd for IMSG_RECONF_PROXY_KEY"); + if (load_file(imsg->fd, &p->key, &p->keylen) == -1) + fatalx("failed to load key for proxy %s of %s", + p->host, h->domain); + break; + case IMSG_RECONF_END: if (proc_compose(ps, PROC_PARENT, IMSG_RECONF_DONE, NULL, 0) == -1) blob - 5cf782bcff040748f446724b81bab54eb01e9c43 blob + db7f8cbe59724d03a7dedb62fe5254b2424e3ab2 --- gmid.h +++ gmid.h @@ -122,10 +122,13 @@ struct proxy { int notls; uint32_t protocols; int noverifyname; + char *cert_path; uint8_t *cert; size_t certlen; + char *key_path; uint8_t *key; size_t keylen; + char *reqca_path; X509_STORE *reqca; TAILQ_ENTRY(proxy) proxies; @@ -141,6 +144,7 @@ struct location { int block_code; char block_fmt[GEMINI_URL_LEN]; int strip; + char *reqca_path; X509_STORE *reqca; int disable_log; int fcgi; @@ -316,6 +320,8 @@ enum imsg_type { IMSG_RECONF_ENV, IMSG_RECONF_ALIAS, IMSG_RECONF_PROXY, + IMSG_RECONF_PROXY_CERT, + IMSG_RECONF_PROXY_KEY, IMSG_RECONF_END, IMSG_RECONF_DONE, @@ -421,7 +427,7 @@ char *absolutify_path(const char*); char *xstrdup(const char*); void *xcalloc(size_t, size_t); void gen_certificate(const char*, const char*, const char*); -X509_STORE *load_ca(const char*); +X509_STORE *load_ca(int); int validate_against_ca(X509_STORE*, const uint8_t*, size_t); struct vhost *new_vhost(void); struct location *new_location(void); blob - 7e4745ff0a39f7c387c37fbe8caabb27ffc8e025 blob + 7209d7b1803788efb43f61057e45ca9f8c2d288f --- parse.y +++ parse.y @@ -301,8 +301,8 @@ proxy : PROXY { advance_proxy(); } if (*proxy->host == '\0') yyerror("invalid proxy block: missing `relay-to' option"); - if ((proxy->cert == NULL && proxy->key != NULL) || - (proxy->cert != NULL && proxy->key == NULL)) + if ((proxy->cert_path == NULL && proxy->key_path != NULL) || + (proxy->cert_path != NULL && proxy->key_path == NULL)) yyerror("invalid proxy block: missing cert or key"); } ; @@ -337,20 +337,14 @@ proxy_opts : /* empty */ ; proxy_opt : CERT string { - tls_unload_file(proxy->cert, proxy->certlen); + free(proxy->cert); ensure_absolute_path($2); - proxy->cert = tls_load_file($2, &proxy->certlen, NULL); - if (proxy->cert == NULL) - yyerror("can't load cert %s", $2); - free($2); + proxy->cert_path = $2; } | KEY string { - tls_unload_file(proxy->key, proxy->keylen); + free(proxy->key); ensure_absolute_path($2); - proxy->key = tls_load_file($2, &proxy->keylen, NULL); - if (proxy->key == NULL) - yyerror("can't load key %s", $2); - free($2); + proxy->key_path = $2; } | PROTOCOLS string { if (tls_config_parse_protocols(&proxy->protocols, $2) == -1) @@ -365,9 +359,7 @@ proxy_opt : CERT string { } | REQUIRE CLIENT CA string { ensure_absolute_path($4); - if ((proxy->reqca = load_ca($4)) == NULL) - yyerror("couldn't load ca cert: %s", $4); - free($4); + proxy->reqca_path = $4; } | SNI string { (void) strlcpy(proxy->sni, $2, sizeof(proxy->sni)); @@ -432,9 +424,7 @@ locopt : AUTO INDEX bool { loc->auto_index = $3 ? 1 : | LOG bool { loc->disable_log = !$2; } | REQUIRE CLIENT CA string { ensure_absolute_path($4); - if ((loc->reqca = load_ca($4)) == NULL) - yyerror("couldn't load ca cert: %s", $4); - free($4); + loc->reqca_path = $4; } | ROOT string { (void) strlcpy(loc->dir, $2, sizeof(loc->dir)); blob - 0437b3bab76508e98b4a89156482f1888d91c9b3 blob + a186ce857f9f8e33cba0bea6aed1ddb45bbb1ce9 --- regress/regress +++ regress/regress @@ -47,13 +47,13 @@ run_test test_custom_index_default_type_per_location run_test test_auto_index run_test test_block run_test test_block_return_fmt -# run_test test_require_client_ca # XXX: needs to be readded +run_test test_require_client_ca run_test test_root_inside_location run_test test_root_inside_location_with_redirect # run_test test_fastcgi XXX: needs to be fixed run_test test_macro_expansion run_test test_proxy_relay_to -# run_test test_proxy_with_certs# XXX: needs to be readded +run_test test_proxy_with_certs # run_test test_unknown_host # XXX: breaks on some distro run_test test_include_mime blob - e14f94ee2bf4651c5ff8ccbce69747cf5f83a05f blob + 6b7422cd0a0ae8a6abe67ff8adb11eba25d0c351 --- server.c +++ server.c @@ -1484,6 +1484,8 @@ server_dispatch_parent(int fd, struct privsep_proc *p, case IMSG_RECONF_ENV: case IMSG_RECONF_ALIAS: case IMSG_RECONF_PROXY: + case IMSG_RECONF_PROXY_CERT: + case IMSG_RECONF_PROXY_KEY: return config_recv(conf, imsg); case IMSG_RECONF_END: if (config_recv(conf, imsg) == -1) blob - 794f896e7a5dbeb7651fe5e8f14733350d42b091 blob + 9aac9dfbb42216fc0f8b8db86ffc8ca5353d8187 --- utils.c +++ utils.c @@ -177,17 +177,21 @@ gen_certificate(const char *hostname, const char *cert } X509_STORE * -load_ca(const char *path) +load_ca(int fd) { FILE *f = NULL; X509 *x = NULL; X509_STORE *store; - if ((store = X509_STORE_new()) == NULL) + if ((store = X509_STORE_new()) == NULL) { + close(fd); return NULL; + } - if ((f = fopen(path, "r")) == NULL) + if ((f = fdopen(fd, "r")) == NULL) { + close(fd); goto err; + } if ((x = PEM_read_X509(f, NULL, NULL, NULL)) == NULL) goto err;