commit ca70f9d726bf282083b87e3e1cea53ebc93b93dd
parent 283cd6da54fcd20f21d8222437903a4081b06bf9
Author: Joris Vink <joris@coders.se>
Date: Tue, 19 Nov 2019 11:09:24 +0100
TLS improvements.
These changes improve the constraint kore had with client authentication and
multiple domains.
- Add kore_x509_subject_name() which will return a C string containing
the x509 subject name in full (in utf8).
- Log TLS errors if client authentication was turned on, will help debug
issues with client authentication in the future.
- If SNI was present in the TLS handshake, check it against the host specified
in the HTTP request and send a 421 in case they mismatch.
- Throw a 403 if client authentication was enabled but no client certificate
was specified.
Diffstat:
8 files changed, 134 insertions(+), 20 deletions(-)
diff --git a/include/kore/http.h b/include/kore/http.h
@@ -425,6 +425,7 @@ enum http_status_code {
HTTP_STATUS_UNSUPPORTED_MEDIA_TYPE = 415,
HTTP_STATUS_REQUEST_RANGE_INVALID = 416,
HTTP_STATUS_EXPECTATION_FAILED = 417,
+ HTTP_STATUS_MISDIRECTED_REQUEST = 421,
HTTP_STATUS_INTERNAL_ERROR = 500,
HTTP_STATUS_NOT_IMPLEMENTED = 501,
HTTP_STATUS_BAD_GATEWAY = 502,
diff --git a/include/kore/kore.h b/include/kore/kore.h
@@ -126,11 +126,7 @@ extern int daemon(int, int);
#define NETBUF_IS_STREAM 0x10
#define NETBUF_IS_FILEREF 0x20
-#define X509_GET_CN(c, o, l) \
- X509_NAME_get_text_by_NID(X509_get_subject_name(c), \
- NID_commonName, o, l)
-
-#define X509_CN_LENGTH (ub_common_name + 1)
+#define KORE_X509_COMMON_NAME_ONLY 0x0001
#define KORE_PEM_CERT_CHAIN 1
#define KORE_DER_CERT_DATA 2
@@ -203,6 +199,7 @@ TAILQ_HEAD(netbuf_head, netbuf);
#define CONN_WS_CLOSE_SENT 0x04
#define CONN_IS_BUSY 0x08
#define CONN_ACME_CHALLENGE 0x10
+#define CONN_LOG_TLS_FAILURE 0x20
#define KORE_IDLE_TIMER_MAX 5000
@@ -236,8 +233,9 @@ struct connection {
struct listener *owner;
X509 *cert;
SSL *ssl;
+ char *tls_sni;
int tls_reneg;
- u_int8_t flags;
+ u_int16_t flags;
void *hdlr_extra;
int (*handle)(struct connection *);
@@ -417,6 +415,7 @@ struct kore_module {
};
#if !defined(KORE_NO_HTTP)
+
struct kore_module_handle {
char *path;
char *func;
@@ -866,6 +865,7 @@ char *kore_read_line(FILE *, char *, size_t);
EVP_PKEY *kore_rsakey_load(const char *);
EVP_PKEY *kore_rsakey_generate(const char *);
+int kore_x509_subject_name(struct connection *, char **, int);
#if !defined(KORE_NO_HTTP)
void kore_websocket_handshake(struct http_request *,
diff --git a/src/accesslog.c b/src/accesslog.c
@@ -47,7 +47,6 @@ static void accesslog_flush(struct kore_domain *, u_int64_t, int);
static u_int64_t time_cache = 0;
static char tbuf[128] = { '\0' };
-char cnbuf[1024] = { '\0' };
static struct kore_buf *logbuf = NULL;
@@ -67,8 +66,8 @@ kore_accesslog(struct http_request *req)
size_t avail;
time_t curtime;
int len, attempts;
- char addr[INET6_ADDRSTRLEN];
const char *ptr, *method, *cn, *referer;
+ char addr[INET6_ADDRSTRLEN], *cn_value;
switch (req->method) {
case HTTP_METHOD_GET:
@@ -103,9 +102,12 @@ kore_accesslog(struct http_request *req)
req->agent = "-";
cn = "-";
+ cn_value = NULL;
+
if (req->owner->cert != NULL) {
- if (X509_GET_CN(req->owner->cert, cnbuf, sizeof(cnbuf)) != -1)
- cn = cnbuf;
+ if (kore_x509_subject_name(req->owner, &cn_value,
+ KORE_X509_COMMON_NAME_ONLY))
+ cn = cn_value;
}
switch (req->owner->family) {
@@ -193,6 +195,7 @@ kore_accesslog(struct http_request *req)
break;
}
+ kore_free(cn_value);
accesslog_unlock(worker);
}
diff --git a/src/connection.c b/src/connection.c
@@ -63,12 +63,13 @@ kore_connection_new(void *owner)
c->ssl = NULL;
c->cert = NULL;
- c->tls_reneg = 0;
c->flags = 0;
c->rnb = NULL;
c->snb = NULL;
c->owner = owner;
c->handle = NULL;
+ c->tls_reneg = 0;
+ c->tls_sni = NULL;
c->disconnect = NULL;
c->hdlr_extra = NULL;
c->proto = CONN_PROTO_UNKNOWN;
@@ -251,7 +252,6 @@ kore_connection_handle(struct connection *c)
{
int r;
struct listener *listener;
- char cn[X509_CN_LENGTH];
kore_debug("kore_connection_handle(%p) -> %d", c, c->state);
kore_connection_stop_idletimer(c);
@@ -280,6 +280,9 @@ kore_connection_handle(struct connection *c)
ssl_errno_s);
return (KORE_RESULT_ERROR);
}
+
+ if (primary_dom->cafile != NULL)
+ c->flags |= CONN_LOG_TLS_FAILURE;
}
ERR_clear_error();
@@ -293,6 +296,10 @@ kore_connection_handle(struct connection *c)
return (KORE_RESULT_OK);
default:
kore_debug("SSL_accept(): %s", ssl_errno_s);
+ if (c->flags & CONN_LOG_TLS_FAILURE) {
+ kore_log(LOG_NOTICE,
+ "SSL_accept: %s", ssl_errno_s);
+ }
return (KORE_RESULT_ERROR);
}
}
@@ -308,14 +315,7 @@ kore_connection_handle(struct connection *c)
if (SSL_get_verify_mode(c->ssl) & SSL_VERIFY_PEER) {
c->cert = SSL_get_peer_certificate(c->ssl);
if (c->cert == NULL) {
- kore_log(LOG_NOTICE,
- "no client certificate presented?");
- return (KORE_RESULT_ERROR);
- }
-
- if (X509_GET_CN(c->cert, cn, sizeof(cn)) == -1) {
- kore_log(LOG_NOTICE,
- "no CN found in client certificate");
+ kore_log(LOG_NOTICE, "no peer certificate");
return (KORE_RESULT_ERROR);
}
} else {
@@ -385,6 +385,9 @@ kore_connection_remove(struct connection *c)
if (c->cert != NULL)
X509_free(c->cert);
+ if (c->tls_sni != NULL)
+ kore_free(c->tls_sni);
+
close(c->fd);
if (c->hdlr_extra != NULL)
diff --git a/src/http.c b/src/http.c
@@ -1486,6 +1486,7 @@ static struct http_request *
http_request_new(struct connection *c, const char *host,
const char *method, char *path, const char *version)
{
+ struct kore_domain *dom;
struct http_request *req;
struct kore_module_handle *hdlr;
char *p, *hp;
@@ -1550,6 +1551,23 @@ http_request_new(struct connection *c, const char *host,
break;
}
+ if (c->owner->server->tls && c->tls_sni != NULL) {
+ if (strcasecmp(c->tls_sni, host)) {
+ http_error_response(c, HTTP_STATUS_MISDIRECTED_REQUEST);
+ return (NULL);
+ }
+ }
+
+ if ((dom = kore_domain_lookup(c->owner->server, host)) == NULL) {
+ http_error_response(c, HTTP_STATUS_NOT_FOUND);
+ return (NULL);
+ }
+
+ if (dom->cafile != NULL && c->cert == NULL) {
+ http_error_response(c, HTTP_STATUS_FORBIDDEN);
+ return (NULL);
+ }
+
req = kore_pool_get(&http_request_pool);
req->owner = c;
@@ -2209,6 +2227,9 @@ http_status_text(int status)
case HTTP_STATUS_EXPECTATION_FAILED:
r = "Expectation Failed";
break;
+ case HTTP_STATUS_MISDIRECTED_REQUEST:
+ r = "Misdirected Request";
+ break;
case HTTP_STATUS_INTERNAL_ERROR:
r = "Internal Server Error";
break;
diff --git a/src/kore.c b/src/kore.c
@@ -360,6 +360,9 @@ kore_tls_sni_cb(SSL *ssl, int *ad, void *arg)
sname = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
kore_debug("kore_tls_sni_cb(): received host %s", sname);
+ if (sname != NULL)
+ c->tls_sni = kore_strdup(sname);
+
if (sname != NULL &&
(dom = kore_domain_lookup(c->owner->server, sname)) != NULL) {
if (dom->ssl_ctx == NULL) {
@@ -375,6 +378,7 @@ kore_tls_sni_cb(SSL *ssl, int *ad, void *arg)
if (dom->cafile != NULL) {
SSL_set_verify(ssl, SSL_VERIFY_PEER |
SSL_VERIFY_FAIL_IF_NO_PEER_CERT, NULL);
+ c->flags |= CONN_LOG_TLS_FAILURE;
} else {
SSL_set_verify(ssl, SSL_VERIFY_NONE, NULL);
}
diff --git a/src/net.c b/src/net.c
@@ -373,6 +373,10 @@ net_write_tls(struct connection *c, size_t len, size_t *written)
/* FALLTHROUGH */
default:
kore_debug("SSL_write(): %s", ssl_errno_s);
+ if (c->flags & CONN_LOG_TLS_FAILURE) {
+ kore_log(LOG_NOTICE,
+ "SSL_write(): %s", ssl_errno_s);
+ }
return (KORE_RESULT_ERROR);
}
}
@@ -401,6 +405,8 @@ net_read_tls(struct connection *c, size_t *bytes)
case SSL_ERROR_WANT_WRITE:
c->evt.flags &= ~KORE_EVENT_READ;
return (KORE_RESULT_OK);
+ case SSL_ERROR_ZERO_RETURN:
+ return (KORE_RESULT_ERROR);
case SSL_ERROR_SYSCALL:
switch (errno) {
case EINTR:
@@ -416,6 +422,10 @@ net_read_tls(struct connection *c, size_t *bytes)
/* FALLTHROUGH */
default:
kore_debug("SSL_read(): %s", ssl_errno_s);
+ if (c->flags & CONN_LOG_TLS_FAILURE) {
+ kore_log(LOG_NOTICE,
+ "SSL_read(): %s", ssl_errno_s);
+ }
return (KORE_RESULT_ERROR);
}
}
diff --git a/src/utils.c b/src/utils.c
@@ -585,6 +585,78 @@ kore_worker_name(int id)
return (buf);
}
+int
+kore_x509_subject_name(struct connection *c, char **out, int flags)
+{
+ struct kore_buf buf;
+ u_int8_t *data;
+ ASN1_STRING *astr;
+ X509_NAME *name;
+ X509_NAME_ENTRY *entry;
+ const char *field;
+ int ret, idx, namelen, nid, len;
+
+ data = NULL;
+ ret = KORE_RESULT_ERROR;
+
+ kore_buf_init(&buf, 1024);
+
+ if (c->cert == NULL)
+ goto cleanup;
+
+ if ((name = X509_get_subject_name(c->cert)) == NULL)
+ goto cleanup;
+
+ namelen = sk_X509_NAME_ENTRY_num(name->entries);
+ if (namelen == 0)
+ goto cleanup;
+
+ for (idx = 0; idx < namelen; idx++) {
+ entry = sk_X509_NAME_ENTRY_value(name->entries, idx);
+ if (entry == NULL)
+ goto cleanup;
+
+ nid = OBJ_obj2nid(entry->object);
+ if ((field = OBJ_nid2sn(nid)) == NULL)
+ goto cleanup;
+
+ if ((astr = X509_NAME_ENTRY_get_data(entry)) == NULL)
+ goto cleanup;
+
+ data = NULL;
+ if ((len = ASN1_STRING_to_UTF8(&data, astr)) < 0)
+ goto cleanup;
+
+ if (flags & KORE_X509_COMMON_NAME_ONLY) {
+ if (nid == NID_commonName) {
+ kore_buf_append(&buf, data, len);
+ break;
+ }
+ } else {
+ kore_buf_appendf(&buf, "%s=", field);
+ kore_buf_append(&buf, data, len);
+ if (idx != (namelen - 1))
+ kore_buf_appendf(&buf, " ");
+ }
+
+ OPENSSL_free(data);
+ data = NULL;
+ }
+
+ ret = KORE_RESULT_OK;
+ *out = kore_buf_stringify(&buf, NULL);
+
+ buf.offset = 0;
+ buf.data = NULL;
+
+cleanup:
+ if (data != NULL)
+ OPENSSL_free(data);
+
+ kore_buf_cleanup(&buf);
+ return (ret);
+}
+
void
fatal(const char *fmt, ...)
{
