[OpenWrt-Devel] [PATCH] uhttpd: add support for mutual authentication (mTLS)

[Jo-Philipp Wich]

Hi,

comments inline.
> From: Nuno Morais <Nuno.mcvmorais at 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;
> 


_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel