crypto: always reload tls-auth/crypt key contexts
PATCH v2 1/3 crypto: always reload tls-auth/crypt key contexts
In preparation to having tls-auth/crypt keys per connection block, it is important to ensure that such material is always reload upon SIGUSR1, no matter is 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.
Trac: #720
---
v2:
- introduce this patch
src/openvpn/buffer.c | 29 +++++++++++++++ src/openvpn/buffer.h | 13 +++++++ src/openvpn/crypto.c | 20 ++-------- src/openvpn/init.c | 87 +++++++++++++++++++++++++++++--------------- 4 files changed, 102 insertions(+), 47 deletions(-)
src/openvpn/buffer.c +29 -0
@@ -1332,3 +1332,32 @@ buffer_list_file(const char *fn, int max_line_len) }return bl; } + +struct buffer +keyfile_to_buffer (const char *file,int max_size,struct gc_arena *gc) +{ +size_t size; +struct buffer in =alloc_buf_gc (max_size, gc); +int 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 ) + { +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); + + in.len = size; + +return in; +}
src/openvpn/buffer.h +13 -0
@@ -1112,4 +1112,17 @@ void buffer_list_aggregate_separator(struct buffer_list *bl,struct buffer_list *buffer_list_file (const char *fn,int max_line_len); +/** +* keyfile_to_buffer - copy the content of a file into a buffer +* +* @param file path to the file to read +* @param max_size maximum size of the buffer to allocate +* @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 +*/ +struct buffer keyfile_to_buffer (const char *file,int max_size, +struct gc_arena *gc); +#endif /* BUFFER_H */
src/openvpn/crypto.c +3 -17
@@ -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,8 @@ 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 ) - { -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); + in =keyfile_to_buffer (file,2048 , &gc); + size = in.len; } cp = (unsigned char *)in.data;
src/openvpn/init.c +57 -30
117 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH v2 2/3 make tls-auth a per-connection-block option
Different VPN servers may use different tls-auth keys. For this reason it is convenient to make tls-auth a per-connection-block option so that the user is allowed to specify one key per remote.
If no tls-auth option is specified in a given connection block, the global one, if any, is used.
If persist-key is specified, tls-auth keys are pre-loaded during startup (same as they were embeeded in the config file) so that later there is no need to access the key files.
Trac: #720
---
v2:
- convert tls-auth keyfile to inline key if persist-key was specified
doc/openvpn.8 | 2 + src/openvpn/init.c | 10 ++--- src/openvpn/options.c | 90 +++++++++++++++++++++++++++++++++++-------- src/openvpn/options.h | 5 +++ 4 files changed, 85 insertions(+), 22 deletions(-)
doc/openvpn.8 +2 -0
@@ -361,6 +361,7 @@ block: .B fragment, .B http\-proxy, .B http\-proxy\-option, +.B key\-direction, .B link\-mtu, .B local, .B lport,
@@ -372,6 +373,7 @@ block: .B remote, .B rport, .B socks\-proxy, +.B tls\-auth, .B tun\-mtu and .B tun\-mtu\-extra.
src/openvpn/init.c +5 -5
@@ -2504,7 +2504,7 @@ do_init_tls_wrap_key(struct context *c)const struct options *options = &c->options;/* TLS handshake authentication (--tls-auth) */ -if (options->tls_auth_file) +if (options->ce.tls_auth_file) {/* Initialize key_type for tls-auth with auth only */ CLEAR (c->c1.ks.tls_auth_key_type);
@@ -2522,9 +2522,9 @@ do_init_tls_wrap_key(struct context *c)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, + options->ce.tls_auth_file, + options->ce.tls_auth_file_inline, + options->ce.key_direction,"Control Channel Authentication" ,"tls-auth" ); }
@@ -2803,7 +2803,7 @@ do_init_crypto_tls(struct context *c, const unsigned int flags)#endif /* TLS handshake authentication (--tls-auth) */ -if (options->tls_auth_file) +if (options->ce.tls_auth_file) { to.tls_wrap.mode =TLS_WRAP_AUTH ; to.tls_wrap.opt.key_ctx_bi = c->c1.ks.tls_wrap_key;
src/openvpn/options.c +73 -17
147 lines not on this page · raw message
src/openvpn/options.h +5 -0
@@ -130,6 +130,11 @@ struct connection_entry#define CE_MAN_QUERY_REMOTE_MASK (0x07)#define CE_MAN_QUERY_REMOTE_SHIFT (2)unsigned int flags; + +/* Shared secret used for TLS control channel authentication */ +const char *tls_auth_file; +const char *tls_auth_file_inline; +int key_direction; };struct remote_entry
Signature · 1 line
2.17.1
PATCH v2 3/3 make tls-crypt a per-connection-block option
In reply to Antonio QuartulliSimilarly to tls-auth, different remotes may use different tls-crypt keys.
Allow the user to define a different key in each connection block.
If no tls-crypt option is specified in a given connection block, the global one, if any, is used.
If persist-key is specified, tls-crypt keys are pre-loaded during startup (same as they were embeeded in the config file) so that later there is no need to access the key files.
Trac: #720
---
v2:
- convert tls-crypt keyfile to inline key if persist-key was specified
doc/openvpn.8 | 1 + src/openvpn/init.c | 8 +++---- src/openvpn/options.c | 56 +++++++++++++++++++++++++++++++++++-------- src/openvpn/options.h | 4 ++++ 4 files changed, 55 insertions(+), 14 deletions(-)
doc/openvpn.8 +1 -0
@@ -374,6 +374,7 @@ block: .B rport, .B socks\-proxy, .B tls\-auth, +.B tls\-crypt, .B tun\-mtu and .B tun\-mtu\-extra.
src/openvpn/init.c +4 -4
@@ -2529,11 +2529,11 @@ do_init_tls_wrap_key(struct context *c) }/* TLS handshake encryption+authentication (--tls-crypt) */ -if (options->tls_crypt_file) +if (options->ce.tls_crypt_file) {tls_crypt_init_key (&c->c1.ks.tls_wrap_key, - options->tls_crypt_file, - options->tls_crypt_inline, options->tls_server); + options->ce.tls_crypt_file, + options->ce.tls_crypt_inline, options->tls_server); } }
@@ -2814,7 +2814,7 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) }/* TLS handshake encryption (--tls-crypt) */ -if (options->tls_crypt_file) +if (options->ce.tls_crypt_file) { to.tls_wrap.mode =TLS_WRAP_CRYPT ; to.tls_wrap.opt.key_ctx_bi = c->c1.ks.tls_wrap_key;
src/openvpn/options.c +46 -10
102 lines not on this page · raw message
src/openvpn/options.h +4 -0
@@ -135,6 +135,10 @@ struct connection_entryconst char *tls_auth_file;const char *tls_auth_file_inline;int key_direction; + +/* Shared secret used for TLS control channel authenticated encryption */ +const char *tls_crypt_file; +const char *tls_crypt_inline; };struct remote_entry
Signature · 1 line
2.17.1
Hi,
Quoted 5 lines · Antonio Quartulli
In preparation to having tls-auth/crypt keys per connection block, it is important to ensure that such material is always reload upon SIGUSR1, no matter is persist-key was specified or not.
Quoted 7 lines
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.
Trac: #720
Cc Steffan Karger <[email protected]...>Signed-off-by Antonio Quartulli <[email protected]...>
as clarified on IRC with Gert, applying this patch alone temporary breaks the persist-key logic and this would not be good when doing bisect.
I'll send v3 implementing what is needed to ensure everything still works, even after having applied this patch alone.
Cheers,
Signature · 1 line
Antonio Quartulli