From patchwork Fri Jul 6 23:04:20 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antonio Quartulli X-Patchwork-Id: 406 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director10.mail.ord1d.rsapps.net ([172.31.255.6]) by backend30.mail.ord1d.rsapps.net (Dovecot) with LMTP id s90cD4eCQFt2PQAAIUCqbw for ; Sat, 07 Jul 2018 05:06:15 -0400 Received: from proxy16.mail.iad3b.rsapps.net ([172.31.255.6]) by director10.mail.ord1d.rsapps.net (Dovecot) with LMTP id 81C8B4eCQFsBWgAApN4f7A ; Sat, 07 Jul 2018 05:06:15 -0400 Received: from smtp4.gate.iad3b ([172.31.255.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy16.mail.iad3b.rsapps.net with LMTP id +CN1BYeCQFtuLQAAPj+4aA ; Sat, 07 Jul 2018 05:06:15 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp4.gate.iad3b.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dmarc=none (p=nil; dis=none) header.from=unstable.cc X-Suspicious-Flag: YES X-Classification-ID: ffd8971e-81c4-11e8-9c95-525400789c6c-1-1 Received: from [216.105.38.7] ([216.105.38.7:16426] helo=lists.sourceforge.net) by smtp4.gate.iad3b.rsapps.net (envelope-from ) (ecelerity 4.2.1.56364 r(Core:4.2.1.14)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id E9/C1-13784-682804B5; Sat, 07 Jul 2018 05:06:14 -0400 Received: from [127.0.0.1] (helo=sfs-ml-1.v29.lw.sourceforge.com) by sfs-ml-1.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1fbj9U-0008Ul-Qe; Sat, 07 Jul 2018 09:05:12 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-1.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1fbj9T-0008UW-El for openvpn-devel@lists.sourceforge.net; Sat, 07 Jul 2018 09:05:11 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: MIME-Version:Content-Type:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=/cjNHsbDhOvYu+9q188ZSJwkIQVb7302oDsph4e70QM=; b=hBuZTcRY92exJJMHs0JULs3/rq E//q2rIBUamH6DIn0LyVAsTx2XL7Q5/21TfRL3sJdmma5xu7/Eej1Tqng2Se8KFUzOJMb4oH7Q/9X mudwnxOq6SYbTwN6bSsu5BrzevAbo+rt5yMikbPsH6FzafhvUFpq8Xn5Mh0PtWcj2XlY=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:MIME-Version: Content-Type:Content-Transfer-Encoding:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=/cjNHsbDhOvYu+9q188ZSJwkIQVb7302oDsph4e70QM=; b=Ox6ueyG/ew+ZLDgmOixN83seuB Z7y2rFgTgUWB/S7xQSr6a6SUytV7q6IA5UcYA33FKM6XqTTmccB+IINVmzfP7rwak/w88JeA7HOyU 0hpi6G8+zx0HYR9VSWJklQGRHHW0bBlWILziyFkIsjRADkqZ8XAMbyOv2OVgNlUf8WNY=; Received: from s2.neomailbox.net ([5.148.176.60]) by sfi-mx-3.v28.lw.sourceforge.com with esmtps (TLSv1.2:DHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) id 1fbj9Q-00C0q6-0i for openvpn-devel@lists.sourceforge.net; Sat, 07 Jul 2018 09:05:11 +0000 From: Antonio Quartulli To: openvpn-devel@lists.sourceforge.net Date: Sat, 7 Jul 2018 17:04:20 +0800 Message-Id: <20180707090421.25953-1-a@unstable.cc> X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [5.148.176.60 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record X-Headers-End: 1fbj9Q-00C0q6-0i Subject: [Openvpn-devel] [PATCH v4 1/2] crypto: always reload tls-auth/crypt key contexts X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Antonio Quartulli MIME-Version: 1.0 Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox In preparation to having tls-auth/crypt keys per connection block, it is important to ensure that such material is always reloaded upon SIGUSR1, no matter if `persist-key` was specified or not. This is required because when moving from one remote to the other the key may change and thus the key context needs to be refreshed. To ensure that the `persist-key` logic will still work as expected, the tls-auth/crypt key is pre-loaded so that the keyfile is not required at runtime. Trac: #720 Cc: Steffan Karger Signed-off-by: Antonio Quartulli --- v2: - introduce this patch v3: - add key per-loading logic to this patch to avoid temporary features breakages v4: - rebase on top of master - use buffer_read_from_file() implementation from "tls-crypt-v2: generate client keys". Minor change: in case of error, free buffer and remove it from gc (with new free_buf_gc()), if provided. - don't go FATAL inside buffer_read_from_file() in case of error, but let external logic decide src/openvpn/buffer.c | 61 ++++++++++++++++++++++++++++++ src/openvpn/buffer.h | 12 ++++++ src/openvpn/crypto.c | 21 +++-------- src/openvpn/init.c | 87 ++++++++++++++++++++++++++++--------------- src/openvpn/options.c | 26 +++++++++++++ 5 files changed, 161 insertions(+), 46 deletions(-) diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index becfeb93..7d4860db 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -189,6 +189,34 @@ free_buf(struct buffer *buf) CLEAR(*buf); } +static void +free_buf_gc(struct buffer *buf, struct gc_arena *gc) +{ + if (gc) + { + struct gc_entry **e = &gc->list; + + while (*e) + { + /* check if this object is the one we want to delete */ + if ((uint8_t *)((*e) + sizeof(*e)) == buf->data) + { + struct gc_entry *to_delete = *e; + + /* remove element from linked list and free it */ + *e = (*e)->next; + free(to_delete); + + break; + } + + e = &(*e)->next; + } + } + + free_buf(buf); +} + /* * Return a buffer for write that is a subset of another buffer */ @@ -1332,3 +1360,36 @@ buffer_list_file(const char *fn, int max_line_len) } return bl; } + +struct buffer +buffer_read_from_file(const char *filename, struct gc_arena *gc) +{ + struct buffer ret = { 0 }; + + platform_stat_t file_stat = {0}; + if (platform_stat(filename, &file_stat) < 0) + { + return ret; + } + + FILE *fp = platform_fopen(filename, "r"); + if (!fp) + { + return ret; + } + + const size_t size = file_stat.st_size; + ret = alloc_buf_gc(size + 1, gc); /* space for trailing \0 */ + ssize_t read_size = fread(BPTR(&ret), 1, size, fp); + if (read_size < 0) + { + free_buf_gc(&ret, gc); + goto cleanup; + } + ASSERT(buf_inc_len(&ret, read_size)); + buf_null_terminate(&ret); + +cleanup: + fclose(fp); + return ret; +} diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h index d848490a..a11c8bd9 100644 --- a/src/openvpn/buffer.h +++ b/src/openvpn/buffer.h @@ -1112,4 +1112,16 @@ void buffer_list_aggregate_separator(struct buffer_list *bl, struct buffer_list *buffer_list_file(const char *fn, int max_line_len); +/** + * buffer_read_from_file - copy the content of a file into a buffer + * + * @param file path to the file to read + * @param gc the garbage collector to use when allocating the buffer. It + * passed to alloc_buf_gc() and therefore can be NULL. + * + * @return the buffer storing the file content or an invalid buffer in case of + * error + */ +struct buffer buffer_read_from_file(const char *filename, struct gc_arena *gc); + #endif /* BUFFER_H */ diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index b59c1f73..5381ef2a 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -1224,7 +1224,7 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags) { struct gc_arena gc = gc_new(); struct buffer in; - int fd, size; + int size; uint8_t hex_byte[3] = {0, 0, 0}; const char *error_filename = file; @@ -1268,22 +1268,11 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags) } else /* 'file' is a filename which refers to a file containing the ascii key */ { - in = alloc_buf_gc(2048, &gc); - fd = platform_open(file, O_RDONLY, 0); - if (fd == -1) - { - msg(M_ERR, "Cannot open key file '%s'", file); - } - size = read(fd, in.data, in.capacity); - if (size < 0) - { + in = buffer_read_from_file(file, &gc); + if (!buf_valid(&in)) msg(M_FATAL, "Read error on key file ('%s')", file); - } - if (size == in.capacity) - { - msg(M_FATAL, "Key file ('%s') can be a maximum of %d bytes", file, (int)in.capacity); - } - close(fd); + + size = in.len; } cp = (unsigned char *)in.data; diff --git a/src/openvpn/init.c b/src/openvpn/init.c index b748357d..1b46b364 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2409,7 +2409,6 @@ key_schedule_free(struct key_schedule *ks, bool free_ssl_ctx) if (tls_ctx_initialised(&ks->ssl_ctx) && free_ssl_ctx) { tls_ctx_free(&ks->ssl_ctx); - free_key_ctx_bi(&ks->tls_wrap_key); } CLEAR(*ks); } @@ -2499,6 +2498,48 @@ do_init_crypto_static(struct context *c, const unsigned int flags) check_replay_consistency(&c->c1.ks.key_type, options->replay); } +/* + * Initialize the tls-auth/crypt key context + */ +static void +do_init_tls_wrap_key(struct context *c) +{ + const struct options *options = &c->options; + + /* TLS handshake authentication (--tls-auth) */ + if (options->tls_auth_file) + { + /* Initialize key_type for tls-auth with auth only */ + CLEAR(c->c1.ks.tls_auth_key_type); + if (!streq(options->authname, "none")) + { + c->c1.ks.tls_auth_key_type.digest = md_kt_get(options->authname); + c->c1.ks.tls_auth_key_type.hmac_length = + md_kt_size(c->c1.ks.tls_auth_key_type.digest); + } + else + { + msg(M_FATAL, "ERROR: tls-auth enabled, but no valid --auth " + "algorithm specified ('%s')", options->authname); + } + + crypto_read_openvpn_key(&c->c1.ks.tls_auth_key_type, + &c->c1.ks.tls_wrap_key, + options->tls_auth_file, + options->tls_auth_file_inline, + options->key_direction, + "Control Channel Authentication", "tls-auth"); + } + + /* TLS handshake encryption+authentication (--tls-crypt) */ + if (options->tls_crypt_file) + { + tls_crypt_init_key(&c->c1.ks.tls_wrap_key, + options->tls_crypt_file, + options->tls_crypt_inline, options->tls_server); + } +} + /* * Initialize the persistent component of OpenVPN's TLS mode, * which is preserved across SIGUSR1 resets. @@ -2548,35 +2589,8 @@ do_init_crypto_tls_c1(struct context *c) /* Initialize PRNG with config-specified digest */ prng_init(options->prng_hash, options->prng_nonce_secret_len); - /* TLS handshake authentication (--tls-auth) */ - if (options->tls_auth_file) - { - /* Initialize key_type for tls-auth with auth only */ - CLEAR(c->c1.ks.tls_auth_key_type); - if (!streq(options->authname, "none")) - { - c->c1.ks.tls_auth_key_type.digest = md_kt_get(options->authname); - c->c1.ks.tls_auth_key_type.hmac_length = - md_kt_size(c->c1.ks.tls_auth_key_type.digest); - } - else - { - msg(M_FATAL, "ERROR: tls-auth enabled, but no valid --auth " - "algorithm specified ('%s')", options->authname); - } - - crypto_read_openvpn_key(&c->c1.ks.tls_auth_key_type, - &c->c1.ks.tls_wrap_key, options->tls_auth_file, - options->tls_auth_file_inline, options->key_direction, - "Control Channel Authentication", "tls-auth"); - } - - /* TLS handshake encryption+authentication (--tls-crypt) */ - if (options->tls_crypt_file) - { - tls_crypt_init_key(&c->c1.ks.tls_wrap_key, options->tls_crypt_file, - options->tls_crypt_inline, options->tls_server); - } + /* initialize tls-auth/crypt key */ + do_init_tls_wrap_key(c); c->c1.ciphername = options->ciphername; c->c1.authname = options->authname; @@ -2598,6 +2612,12 @@ do_init_crypto_tls_c1(struct context *c) c->options.ciphername = c->c1.ciphername; c->options.authname = c->c1.authname; c->options.keysize = c->c1.keysize; + + /* + * tls-auth/crypt key can be configured per connection block, therefore + * we must reload it as it may have changed + */ + do_init_tls_wrap_key(c); } } @@ -3401,6 +3421,13 @@ do_close_tls(struct context *c) static void do_close_free_key_schedule(struct context *c, bool free_ssl_ctx) { + /* + * always free the tls_auth/crypt key. If persist_key is true, the key will + * be reloaded from memory (pre-cached) + */ + free_key_ctx_bi(&c->c1.ks.tls_wrap_key); + CLEAR(c->c1.ks.tls_wrap_key); + if (!(c->sig->signal_received == SIGUSR1 && c->options.persist_key)) { key_schedule_free(&c->c1.ks, free_ssl_ctx); diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 426057ab..b2ca2738 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3019,6 +3019,32 @@ options_postprocess_mutate(struct options *o) options_postprocess_mutate_ce(o, o->connection_list->array[i]); } + /* pre-cache tls-auth/crypt key file if persist-key was specified */ + if (o->persist_key) + { + if (o->tls_auth_file && !o->tls_auth_file_inline) + { + struct buffer in = buffer_read_from_file(o->tls_auth_file, &o->gc); + if (!buf_valid(&in)) + msg(M_FATAL, "Cannot pre-load tls-auth keyfile (%s)", + o->tls_auth_file); + + o->tls_auth_file = INLINE_FILE_TAG; + o->tls_auth_file_inline = (char *)in.data; + } + + if (o->tls_crypt_file && !o->tls_crypt_inline) + { + struct buffer in = buffer_read_from_file(o->tls_crypt_file, &o->gc); + if (!buf_valid(&in)) + msg(M_FATAL, "Cannot pre-load tls-crypt keyfile (%s)", + o->tls_auth_file); + + o->tls_crypt_file = INLINE_FILE_TAG; + o->tls_crypt_inline = (char *)in.data; + } + } + if (o->tls_server) { /* Check that DH file is specified, or explicitly disabled */