| Message ID | 20190510121114.30468-4-arne@rfc2549.org |
|---|---|
| State | Superseded |
| Delegated to: | David Sommerseth |
| Headers | show |
| Series | Auth token patches v3 | expand |
On 10/05/2019 14:11, Arne Schwabe wrote: > From: Arne Schwabe <arne@openvpn.net> > > This is useful for features that can use enither a persistent > or an ephemeral key. > > Patch V2: Move the functionality of generating a random key into a > separate function that acts as wrapper for pem_read_key_file > --- > src/openvpn/crypto.c | 22 ++++++++++++++++++++++ > src/openvpn/crypto.h | 15 +++++++++++++++ > 2 files changed, 37 insertions(+) > > diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c > index eb56421b..0e17c351 100644 > --- a/src/openvpn/crypto.c > +++ b/src/openvpn/crypto.c > @@ -1892,6 +1892,28 @@ cleanup: > return; > } > > +bool > +load_pem_file_or_ephemeral_key(struct buffer *key, const char *pem_name, > + const char *key_file, const char *key_inline) > +{ > + if (key_file == NULL) I don't like this approach. Why can't we have generate_ephemeral_key() basically doing the keyfile == NULL part below and put the "which key to use" logic in the only caller I could see in auth_token_init_secret() [added in a later patch] ? This makes the code clearer: ----------------------------------------------------------------- void auth_token_init_secret(struct key_ctx *key_ctx, const char *key_file, const char *key_inline) { /* .... */ bool key_loaded = false; if (key_file) { key_loaded = read_pem_key(...); } else { key_loaded = generate_ephemeral_key(...); } if (!key_loaded) { msg(M_FATAL, ....); } /* .... */ ----------------------------------------------------------------- You could also make it more compact, but not sure it's worth it here, as these functions requires a few longer argument lists: bool key_loaded = (key_file ? read_pem_key(...) : generate_ephemeral_key(...)); The point is: This makes read_pem_key() and generate_ephemeral_key() do only one simple and predictable task. The load_pem_file_or_ephemeral_key() does one of two things, depending on the argument. And since I only see we have only a single caller to the load_pem_file_or_ephemeral_key(), there's nothing to save on code reuse by wrapping it up like this. > + { > + msg(M_INFO, "Using random %s.", pem_name); > + uint8_t rand[BCAP(key)]; > + if (!rand_bytes(rand, BCAP(key))) > + { > + msg(M_WARN, "ERROR: could not generate random key"); > + return false; > + } > + buf_write(key, rand, BCAP(key)); > + return true; > + } > + else > + { > + return read_pem_key_file(key, pem_name, key_file, key_inline); > + } > +} > +
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index eb56421b..0e17c351 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -1892,6 +1892,28 @@ cleanup: return; } +bool +load_pem_file_or_ephemeral_key(struct buffer *key, const char *pem_name, + const char *key_file, const char *key_inline) +{ + if (key_file == NULL) + { + msg(M_INFO, "Using random %s.", pem_name); + uint8_t rand[BCAP(key)]; + if (!rand_bytes(rand, BCAP(key))) + { + msg(M_WARN, "ERROR: could not generate random key"); + return false; + } + buf_write(key, rand, BCAP(key)); + return true; + } + else + { + return read_pem_key_file(key, pem_name, key_file, key_inline); + } +} + bool read_pem_key_file(struct buffer *key, const char *pem_name, const char *key_file, const char *key_inline) diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h index c5947483..4ae02571 100644 --- a/src/openvpn/crypto.h +++ b/src/openvpn/crypto.h @@ -430,6 +430,21 @@ unsigned int crypto_max_overhead(void); void write_pem_key_file(const char *filename, const char *pem_name); +/** + * Read key material from a PEM encoded files into the key structure or if + * the filename is not passed generate a ephemeral key + * + * @param key the key structure that will hold the key material + * @param pem_name the name used in the pem encoding start/end lines + * @param key_file name of the file to read or NULL if an ephermal key + * should be generated + * @param key_inline a string holding the data in case of an inline key + * @return true if reading into key was successful or generated + */ +bool +load_pem_file_or_ephemeral_key(struct buffer *key, const char *pem_name, + const char *key_file, const char *key_inline); + /** * Read key material from a PEM encoded files into the key structure * @param key the key structure that will hold the key material
From: Arne Schwabe <arne@openvpn.net> This is useful for features that can use enither a persistent or an ephemeral key. Patch V2: Move the functionality of generating a random key into a separate function that acts as wrapper for pem_read_key_file --- src/openvpn/crypto.c | 22 ++++++++++++++++++++++ src/openvpn/crypto.h | 15 +++++++++++++++ 2 files changed, 37 insertions(+)