| Message ID | 20180820111136.1458-1-nuno.mcvmorais@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | John Crispin |
| Headers | show |
| Series | [OpenWrt-Devel] uhttpd: add support for mutual authentication (mTLS) | expand |
Hi, comments inline. > From: Nuno Morais <Nuno.mcvmorais@gmail.com> > > Optional mutual authentication (mTLS) > by providing a CA certificate through a new new flag "-M" > in order to verify client's identity. > > For B2B applications. > > This patch depends on patch > "[OpenWrt-Devel] [PATCH] ustream-ssl: add optional mutual authentication (mTLS)" > --- > main.c | 19 +++++++++++++++---- > tls.c | 38 ++++++++++++++++++++++++++++++++++++++ > tls.h | 6 ++++++ > 3 files changed, 59 insertions(+), 4 deletions(-) > > diff --git a/main.c b/main.c > index fb27665..178d710 100644 > --- a/main.c > +++ b/main.c > @@ -134,6 +134,7 @@ static int usage(const char *name) > " -s [addr:]port Like -p but provide HTTPS on this port\n" > " -C file ASN.1 server certificate file\n" > " -K file ASN.1 server private key file\n" > + " -M file ASN.1 certificate authority certificate file\n" > " -q Redirect all HTTP requests to HTTPS\n" > #endif > " -h directory Specify the document root, default is '.'\n" > @@ -223,7 +224,8 @@ int main(int argc, char **argv) > int bound = 0; > #ifdef HAVE_TLS > int n_tls = 0; > - const char *tls_key = NULL, *tls_crt = NULL; > + int n_mtls = 0; > + const char *tls_key = NULL, *tls_crt = NULL, *ca_crt = NULL; > #endif > > BUILD_BUG_ON(sizeof(uh_buf) < PATH_MAX); > @@ -232,7 +234,7 @@ int main(int argc, char **argv) > init_defaults_pre(); > signal(SIGPIPE, SIG_IGN); > > - while ((ch = getopt(argc, argv, "A:aC:c:Dd:E:fh:H:I:i:K:k:L:l:m:N:n:p:qRr:Ss:T:t:U:u:Xx:y:")) != -1) { > + while ((ch = getopt(argc, argv, "A:aC:c:Dd:E:fh:H:I:i:K:k:L:l:M:m:N:n:p:qRr:Ss:T:t:U:u:Xx:y:")) != -1) { > switch(ch) { > #ifdef HAVE_TLS > case 'C': > @@ -242,6 +244,11 @@ int main(int argc, char **argv) > case 'K': > tls_key = optarg; > break; > + > + case 'M': > + ca_crt = optarg; > + n_mtls++; > + break; > > case 'q': > conf.tls_redirect = 1; > @@ -477,8 +484,12 @@ int main(int argc, char **argv) > return 1; > } > > - if (uh_tls_init(tls_key, tls_crt)) > - return 1; > + if (n_mtls){ > + if (uh_mtls_init(tls_key, tls_crt, ca_crt)) > + return 1; > + } else if (uh_tls_init(tls_key, tls_crt)) > + return 1; Tabs. vs. space > + > } > #endif > > diff --git a/tls.c b/tls.c > index d969b82..bc1a19d 100644 > --- a/tls.c > +++ b/tls.c > @@ -66,6 +66,44 @@ int uh_tls_init(const char *key, const char *crt) > return 0; > } > > +int uh_mtls_init(const char *key, const char *crt, const char *ca_crt) > +{ Is the duplication of the TLS init code really required? Can't you simply make the 3rd parameter optional and decide based on it whether to enable the mutual TLS stuff or not? > + static bool _init = false; > + > + if (_init) > + return 0; > + > + _init = true; > + dlh = dlopen("libustream-ssl." LIB_EXT, RTLD_LAZY | RTLD_LOCAL); > + if (!dlh) { > + fprintf(stderr, "Failed to load ustream-ssl library: %s\n", dlerror()); > + return -ENOENT; > + } > + > + ops = dlsym(dlh, "ustream_ssl_ops"); > + if (!ops) { > + fprintf(stderr, "Could not find required symbol 'ustream_ssl_ops' in ustream-ssl library\n"); > + return -ENOENT; > + } > + > + ctx = ops->context_new(true); > + > + if (!ctx) { > + fprintf(stderr, "Failed to initialize ustream-ssl\n"); > + return -EINVAL; > + } > + > + if (ops->context_set_crt_file(ctx, crt) || > + ops->context_set_key_file(ctx, key) || > + ops->context_add_ca_crt_file(ctx, ca_crt) || > + ops->context_set_mutual_auth(ctx, 1)) { Tabs vs. space > + fprintf(stderr, "Failed to load certificates/key files\n"); > + return -EINVAL; > + } > + > + return 0; > +} > + > static void tls_ustream_read_cb(struct ustream *s, int bytes) > { > struct client *cl = container_of(s, struct client, ssl.stream); > diff --git a/tls.h b/tls.h > index 9be74ba..620ba8f 100644 > --- a/tls.h > +++ b/tls.h > @@ -22,12 +22,18 @@ > > #ifdef HAVE_TLS > > +int uh_mtls_init(const char *key, const char *crt, const char *ca_crt); > int uh_tls_init(const char *key, const char *crt); > void uh_tls_client_attach(struct client *cl); > void uh_tls_client_detach(struct client *cl); > > #else > > +static inline int uh_mtls_init(const char *key, const char *crt, const char *ca_crt) > +{ > + return -1; > +} > + > static inline int uh_tls_init(const char *key, const char *crt) > { > return -1; >
diff --git a/main.c b/main.c index fb27665..178d710 100644 --- a/main.c +++ b/main.c @@ -134,6 +134,7 @@ static int usage(const char *name) " -s [addr:]port Like -p but provide HTTPS on this port\n" " -C file ASN.1 server certificate file\n" " -K file ASN.1 server private key file\n" + " -M file ASN.1 certificate authority certificate file\n" " -q Redirect all HTTP requests to HTTPS\n" #endif " -h directory Specify the document root, default is '.'\n" @@ -223,7 +224,8 @@ int main(int argc, char **argv) int bound = 0; #ifdef HAVE_TLS int n_tls = 0; - const char *tls_key = NULL, *tls_crt = NULL; + int n_mtls = 0; + const char *tls_key = NULL, *tls_crt = NULL, *ca_crt = NULL; #endif BUILD_BUG_ON(sizeof(uh_buf) < PATH_MAX); @@ -232,7 +234,7 @@ int main(int argc, char **argv) init_defaults_pre(); signal(SIGPIPE, SIG_IGN); - while ((ch = getopt(argc, argv, "A:aC:c:Dd:E:fh:H:I:i:K:k:L:l:m:N:n:p:qRr:Ss:T:t:U:u:Xx:y:")) != -1) { + while ((ch = getopt(argc, argv, "A:aC:c:Dd:E:fh:H:I:i:K:k:L:l:M:m:N:n:p:qRr:Ss:T:t:U:u:Xx:y:")) != -1) { switch(ch) { #ifdef HAVE_TLS case 'C': @@ -242,6 +244,11 @@ int main(int argc, char **argv) case 'K': tls_key = optarg; break; + + case 'M': + ca_crt = optarg; + n_mtls++; + break; case 'q': conf.tls_redirect = 1; @@ -477,8 +484,12 @@ int main(int argc, char **argv) return 1; } - if (uh_tls_init(tls_key, tls_crt)) - return 1; + if (n_mtls){ + if (uh_mtls_init(tls_key, tls_crt, ca_crt)) + return 1; + } else if (uh_tls_init(tls_key, tls_crt)) + return 1; + } #endif diff --git a/tls.c b/tls.c index d969b82..bc1a19d 100644 --- a/tls.c +++ b/tls.c @@ -66,6 +66,44 @@ int uh_tls_init(const char *key, const char *crt) return 0; } +int uh_mtls_init(const char *key, const char *crt, const char *ca_crt) +{ + static bool _init = false; + + if (_init) + return 0; + + _init = true; + dlh = dlopen("libustream-ssl." LIB_EXT, RTLD_LAZY | RTLD_LOCAL); + if (!dlh) { + fprintf(stderr, "Failed to load ustream-ssl library: %s\n", dlerror()); + return -ENOENT; + } + + ops = dlsym(dlh, "ustream_ssl_ops"); + if (!ops) { + fprintf(stderr, "Could not find required symbol 'ustream_ssl_ops' in ustream-ssl library\n"); + return -ENOENT; + } + + ctx = ops->context_new(true); + + if (!ctx) { + fprintf(stderr, "Failed to initialize ustream-ssl\n"); + return -EINVAL; + } + + if (ops->context_set_crt_file(ctx, crt) || + ops->context_set_key_file(ctx, key) || + ops->context_add_ca_crt_file(ctx, ca_crt) || + ops->context_set_mutual_auth(ctx, 1)) { + fprintf(stderr, "Failed to load certificates/key files\n"); + return -EINVAL; + } + + return 0; +} + static void tls_ustream_read_cb(struct ustream *s, int bytes) { struct client *cl = container_of(s, struct client, ssl.stream); diff --git a/tls.h b/tls.h index 9be74ba..620ba8f 100644 --- a/tls.h +++ b/tls.h @@ -22,12 +22,18 @@ #ifdef HAVE_TLS +int uh_mtls_init(const char *key, const char *crt, const char *ca_crt); int uh_tls_init(const char *key, const char *crt); void uh_tls_client_attach(struct client *cl); void uh_tls_client_detach(struct client *cl); #else +static inline int uh_mtls_init(const char *key, const char *crt, const char *ca_crt) +{ + return -1; +} + static inline int uh_tls_init(const char *key, const char *crt) { return -1;
From: Nuno Morais <Nuno.mcvmorais@gmail.com> Optional mutual authentication (mTLS) by providing a CA certificate through a new new flag "-M" in order to verify client's identity. For B2B applications. This patch depends on patch "[OpenWrt-Devel] [PATCH] ustream-ssl: add optional mutual authentication (mTLS)" --- main.c | 19 +++++++++++++++---- tls.c | 38 ++++++++++++++++++++++++++++++++++++++ tls.h | 6 ++++++ 3 files changed, 59 insertions(+), 4 deletions(-)