Skip to contentSkip to conversation

convert *_inline attributes to bool

PATCH Superseded Antonio Quartulli 3 participants 9 messages Dec 2, 2017 – May 7, 2020

lore
Share & tools
lore
#0 lore raw

PATCH v7 1/1 convert *_inline attributes to bool

Carrying around the INLINE_TAG is not really efficient, because it requires a strcmp() to be performed every time we want to understand if the data is stored inline or not.

Convert all the *_inline attributes to bool to make the logic easier and checks more efficient.

---

Changes from v6:
- rebased on top of latest master

Changes from v5:
- fix function invocation alignment in options.c:options_postprocess_filechecks()
- fix typ0 in function invocation in options.c:options_postprocess_filechecks()
- fix doxygen comment for function tls_ctx_reload_crl() in ssl.c

Changes from v4:
- remove newline accidentally added in v4

Changes from v3:
- some code style adjustment in options.c:check_inline_file()
- move print_if_inline() from misc.c to crypto.c and rename it to print_key_filename()
- make comment of check_file_access_inline() and check_file_access_chroot_inline() doxygen compliant
- remove *is_inline argument in check_inline_file() and use its return value instead
- move declarations of is_inline to narrower scope in options.c
- move return type of plugin_option_list_add() to its own line

Changes from v2:
- fix indentation in ssl_openssl.c
- do not attempt to push inline'd options
- do not attempt to parse inline'd plugin
- introduce check_file_access_inline() and check_file_access_chroot_inline()
- introduce OPT_P_INLINE to specify when an option is allowed to be inline. Options not having this permission will fail to be parsed if is_inline is true

Changes from v1:
- remove the INLINE_TAG from the options parsing logic at all. Now a boolean variable is passed around
- add print_if_inline() helper function (to misc.c/h) to make sure we never print the inline data, but only the INLINE tag. Such function checks also for NULL pointers
- make sure print_if_inline() is always used when printing possibly inline data
- remove the INLINE_TAG from the options parsing logic at all. Now a boolean variable is passed around
- fix alignment error in comment
- remove CHKACC_INLINE from check_file_access() logic: this function is now not invoked at all in case of inline data

 src/openvpn/crypto.c      |  36 ++++--
 src/openvpn/crypto.h      |  13 ++-
 src/openvpn/misc.c        |   6 +-
 src/openvpn/misc.h        |   3 +-
 src/openvpn/options.c     | 282 ++++++++++++++++++++++++++--------------------
 src/openvpn/options.h     |  21 ++--
 src/openvpn/plugin.c      |   5 +-
 src/openvpn/plugin.h      |   3 +-
 src/openvpn/push.c        |   2 +-
 src/openvpn/push.h        |   3 +-
 src/openvpn/ssl.c         |   6 +-
 src/openvpn/ssl_backend.h |  64 ++++++-----
 src/openvpn/ssl_common.h  |   2 +-
 src/openvpn/ssl_mbedtls.c |  63 +++++------
 src/openvpn/ssl_openssl.c |  92 ++++++++-------
 src/openvpn/tls_crypt.c   |   2 +-
 src/openvpn/tls_crypt.h   |   9 +-
 17 files changed, 342 insertions(+), 270 deletions(-)
src/openvpn/crypto.c +24 -12
index 3f3caa1c..d04880b8 100644
@@ -1176,25 +1176,24 @@ test_crypto(struct crypto_options *co, struct frame *frame)
 
 void
 crypto_read_openvpn_key(const struct key_type *key_type,
-                        struct key_ctx_bi *ctx, const char *key_file, const char *key_inline,
+                        struct key_ctx_bi *ctx, const char *key_file, bool key_inline,
                         const int key_direction, const char *key_name, const char *opt_name)
 {
     struct key2 key2;
     struct key_direction_state kds;
+    unsigned int flags = RKF_MUST_SUCCEED;
 
     if (key_inline)
     {
-        read_key_file(&key2, key_inline, RKF_MUST_SUCCEED|RKF_INLINE);
-    }
-    else
-    {
-        read_key_file(&key2, key_file, RKF_MUST_SUCCEED);
+        flags |= RKF_INLINE;
     }
+    read_key_file(&key2, key_file, flags);
 
     if (key2.n != 2)
     {
         msg(M_ERR, "File '%s' does not have OpenVPN Static Key format.  Using "
-            "free-form passphrase file is not supported anymore.", key_file);
+            "free-form passphrase file is not supported anymore.",
+            print_key_filename(key_file, key_inline));
     }
 
     /* check for and fix highly unlikely key problems */
@@ -1228,7 +1227,6 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags)
     struct buffer in;
     int fd, size;
     uint8_t hex_byte[3] = {0, 0, 0};
-    const char *error_filename = file;
 
     /* parse info */
     const unsigned char *cp;
@@ -1266,7 +1264,6 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags)
     {
         size = strlen(file) + 1;
         buf_set_read(&in, (const uint8_t *)file, size);
-        error_filename = INLINE_FILE_TAG;
     }
     else /* 'file' is a filename which refers to a file containing the ascii key */
     {
@@ -1371,7 +1368,9 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags)
                 {
                     msg(M_FATAL,
                         (isprint(c) ? printable_char_fmt : unprintable_char_fmt),
-                        c, line_num, error_filename, count, onekeylen, keylen);
+                        c, line_num,
+                        print_key_filename(file, flags & RKF_INLINE), count,
+                        onekeylen, keylen);
                 }
             }
             ++line_index;
@@ -1392,13 +1391,15 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags)
         if (!key2->n)
         {
             msg(M_FATAL, "Insufficient key material or header text not found in file '%s' (%d/%d/%d bytes found/min/max)",
-                error_filename, count, onekeylen, keylen);
+                print_key_filename(file, flags & RKF_INLINE), count, onekeylen,
+                keylen);
         }
 
         if (state != PARSE_FINISHED)
         {
             msg(M_FATAL, "Footer text not found in file '%s' (%d/%d/%d bytes found/min/max)",
-                error_filename, count, onekeylen, keylen);
+                print_key_filename(file, flags & RKF_INLINE), count, onekeylen,
+                keylen);
         }
     }
 
@@ -1843,4 +1844,15 @@ translate_cipher_name_to_openvpn(const char *cipher_name)
     return pair->openvpn_name;
 }
 
+const char *
+print_key_filename(const char *str, bool is_inline)
+{
+    if (is_inline)
+    {
+        return INLINE_FILE_TAG;
+    }
+
+    return np(str);
+}
+
 #endif /* ENABLE_CRYPTO */
src/openvpn/crypto.h +12 -1
index 6d60ef8c..07423fd5 100644
@@ -481,7 +481,7 @@ void key2_print(const struct key2 *k,
                 const char *prefix1);
 
 void crypto_read_openvpn_key(const struct key_type *key_type,
-                             struct key_ctx_bi *ctx, const char *key_file, const char *key_inline,
+                             struct key_ctx_bi *ctx, const char *key_file, bool key_inline,
                              const int key_direction, const char *key_name, const char *opt_name);
 
 /*
@@ -513,6 +513,17 @@ key_ctx_bi_defined(const struct key_ctx_bi *key)
     return key->encrypt.cipher || key->encrypt.hmac || key->decrypt.cipher || key->decrypt.hmac;
 }
 
+/**
+ * To be used when printing a string that may contain inline data.
+ *
+ * If "is_inline" is true, return the inline tag.
+ * If "is_inline" is false and "str" is not NULL, return "str".
+ * Return the constant string "[NULL]" otherwise.
+ *
+ * @param str       the original string to return when is_inline is false
+ * @param is_inline true when str contains an inline data of some sort
+ */
+const char *print_key_filename(const char *str, bool is_inline);
 
 #endif /* ENABLE_CRYPTO */
 #endif /* CRYPTO_H */
src/openvpn/misc.c +3 -3
index 6d53cbfb..f082284b 100644
@@ -1444,12 +1444,12 @@ make_arg_copy(char **p, struct gc_arena *gc)
 }
 
 const char **
-make_extended_arg_array(char **p, struct gc_arena *gc)
+make_extended_arg_array(char **p, bool is_inline, struct gc_arena *gc)
 {
     const int argc = string_array_len((const char **)p);
-    if (!strcmp(p[0], INLINE_FILE_TAG) && argc == 2)
+    if (is_inline)
     {
-        return make_inline_array(p[1], gc);
+        return make_inline_array(p[0], gc);
     }
     else if (argc == 0)
     {
src/openvpn/misc.h +2 -1
index f6c810a2..34b4b0b1 100644
@@ -140,7 +140,8 @@ const char **make_env_array(const struct env_set *es,
 
 const char **make_arg_array(const char *first, const char *parms, struct gc_arena *gc);
 
-const char **make_extended_arg_array(char **p, struct gc_arena *gc);
+const char **make_extended_arg_array(char **p, bool is_inline,
+                                     struct gc_arena *gc);
 
 /* an analogue to the random() function, but use OpenSSL functions if available */
 #ifdef ENABLE_CRYPTO
src/openvpn/options.c +161 -121

505 lines not on this page · raw message

src/openvpn/options.h +11 -10
index 035c6d15..75340efc 100644
@@ -471,7 +471,7 @@ struct options
 #ifdef ENABLE_CRYPTO
     /* Cipher parms */
     const char *shared_secret_file;
-    const char *shared_secret_file_inline;
+    bool shared_secret_file_inline;
     int key_direction;
     const char *ciphername;
     bool ncp_enabled;
@@ -510,13 +510,13 @@ struct options
     const char *tls_export_cert;
     const char *crl_file;
 
-    const char *ca_file_inline;
-    const char *cert_file_inline;
-    const char *extra_certs_file_inline;
-    const char *crl_file_inline;
-    char *priv_key_file_inline;
-    const char *dh_file_inline;
-    const char *pkcs12_file_inline; /* contains the base64 encoding of pkcs12 file */
+    bool ca_file_inline;
+    bool cert_file_inline;
+    bool extra_certs_file_inline;
+    bool crl_file_inline;
+    bool priv_key_file_inline;
+    bool dh_file_inline;
+    bool pkcs12_file_inline;
 
     int ns_cert_type; /* set to 0, NS_CERT_CHECK_SERVER, or NS_CERT_CHECK_CLIENT */
     unsigned remote_cert_ku[MAX_PARMS];
@@ -565,11 +565,11 @@ struct options
 
     /* Shared secret used for TLS control channel authentication */
     const char *tls_auth_file;
-    const char *tls_auth_file_inline;
+    bool tls_auth_file_inline;
 
     /* Shared secret used for TLS control channel authenticated encryption */
     const char *tls_crypt_file;
-    const char *tls_crypt_inline;
+    bool tls_crypt_inline;
 
     /* Allow only one session */
     bool single_session;
@@ -646,6 +646,7 @@ struct options
 #define OPT_P_SOCKFLAGS       (1<<26)
 #define OPT_P_CONNECTION      (1<<27)
 #define OPT_P_PEER_ID         (1<<28)
+#define OPT_P_INLINE          (1<<29)
 
 #define OPT_P_DEFAULT   (~(OPT_P_INSTANCE|OPT_P_PULL_MODE))
 
src/openvpn/plugin.c +3 -2
index 557b6bc7..b3007022 100644
@@ -160,12 +160,13 @@ plugin_option_list_new(struct gc_arena *gc)
 }
 
 bool
-plugin_option_list_add(struct plugin_option_list *list, char **p, struct gc_arena *gc)
+plugin_option_list_add(struct plugin_option_list *list, char **p,
+                       struct gc_arena *gc)
 {
     if (list->n < MAX_PLUGINS)
     {
         struct plugin_option *o = &list->plugins[list->n++];
-        o->argv = make_extended_arg_array(p, gc);
+        o->argv = make_extended_arg_array(p, false, gc);
         if (o->argv[0])
         {
             o->so_pathname = o->argv[0];
src/openvpn/plugin.h +2 -1
index 0cffee0f..818b6285 100644
@@ -106,7 +106,8 @@ struct plugin_return
 
 struct plugin_option_list *plugin_option_list_new(struct gc_arena *gc);
 
-bool plugin_option_list_add(struct plugin_option_list *list, char **p, struct gc_arena *gc);
+bool plugin_option_list_add(struct plugin_option_list *list, char **p,
+                            struct gc_arena *gc);
 
 #ifndef ENABLE_SMALL
 void plugin_option_list_print(const struct plugin_option_list *list, int msglevel);
src/openvpn/push.c +1 -1
index e7aecbb7..f632a171 100644
@@ -568,7 +568,7 @@ clone_push_list(struct options *o)
 void
 push_options(struct options *o, char **p, int msglevel, struct gc_arena *gc)
 {
-    const char **argv = make_extended_arg_array(p, gc);
+    const char **argv = make_extended_arg_array(p, false, gc);
     char *opt = print_argv(argv, gc, 0);
     push_option(o, opt, msglevel);
 }
src/openvpn/push.h +2 -1
index 4d42e81f..865c1bc9 100644
@@ -57,7 +57,8 @@ void clone_push_list(struct options *o);
 
 void push_option(struct options *o, const char *opt, int msglevel);
 
-void push_options(struct options *o, char **p, int msglevel, struct gc_arena *gc);
+void push_options(struct options *o, char **p, int msglevel,
+                  struct gc_arena *gc);
 
 void push_reset(struct options *o);
 
src/openvpn/ssl.c +3 -3
index 843bc393..a2c2d755 100644
@@ -553,7 +553,7 @@ tls_version_parse(const char *vstr, const char *extra)
  */
 static void
 tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file,
-                   const char *crl_file_inline)
+                   bool crl_file_inline)
 {
     /* if something goes wrong with stat(), we'll store 0 as mtime */
     platform_stat_t crl_stat = {0};
@@ -664,8 +664,8 @@ init_ssl(const struct options *options, struct tls_root_ctx *new_ctx)
         {
             char *external_certificate = management_query_cert(management,
                                                                options->management_certificate);
-            tls_ctx_use_external_private_key(new_ctx, INLINE_FILE_TAG,
-                                             external_certificate);
+            tls_ctx_use_external_private_key(new_ctx, external_certificate,
+                                             true);
             free(external_certificate);
         }
     }
src/openvpn/ssl_backend.h +35 -29

137 lines not on this page · raw message

src/openvpn/ssl_common.h +1 -1
index 25bffd5b..bdbf9bf9 100644
@@ -265,7 +265,7 @@ struct tls_options
     int verify_x509_type;
     const char *verify_x509_name;
     const char *crl_file;
-    const char *crl_file_inline;
+    bool crl_file_inline;
     int ns_cert_type;
     unsigned remote_cert_ku[MAX_PARMS];
     const char *remote_cert_eku;
src/openvpn/ssl_mbedtls.c +31 -32

166 lines not on this page · raw message

src/openvpn/ssl_openssl.c +45 -47

258 lines not on this page · raw message

src/openvpn/tls_crypt.c +1 -1
index 403060de..7ee03896 100644
@@ -67,7 +67,7 @@ tls_crypt_buf_overhead(void)
 
 void
 tls_crypt_init_key(struct key_ctx_bi *key, const char *key_file,
-                   const char *key_inline, bool tls_server)
+                   bool key_inline, bool tls_server)
 {
     const int key_direction = tls_server ?
                               KEY_DIRECTION_NORMAL : KEY_DIRECTION_INVERSE;
src/openvpn/tls_crypt.h +5 -4
index 4071ac94..7c3087f3 100644
@@ -93,13 +93,14 @@
  *
  * @param key           The key context to initialize
  * @param key_file      The file to read the key from (or the inline tag to
- *                      indicate and inline key).
- * @param key_inline    Array containing (zero-terminated) inline key, or NULL
- *                      if not used.
+ *                      indicate and inline key) or an array containing
+ *                      (zero-terminated) inline key.
+ * @param key_inline    True if key_file contains an inline key, False
+ *                      otherwise.
  * @param tls_server    Must be set to true is this is a TLS server instance.
  */
 void tls_crypt_init_key(struct key_ctx_bi *key, const char *key_file,
-                        const char *key_inline, bool tls_server);
+                        bool key_inline, bool tls_server);
 
 /**
  * Returns the maximum overhead (in bytes) added to the destination buffer by
Signature · 1 line
2.15.1
#1 lore raw

Hi,

Quoted 9 lines · Antonio Quartulli

Carrying around the INLINE_TAG is not really efficient, because it requires a strcmp() to be performed every time we want to understand if the data is stored inline or not.

Convert all the *_inline attributes to bool to make the logic easier and checks more efficient.

As discussed with Antonio right now, this got left in the cold for too long and much of the surrounding code got changed - so it does not apply anymore.

(Also, it was missing an ACK...)

Can I have a rebased version & an ACK on that, please? ;-)

gert

Signature · 6 lines
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
                             Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany                             [email protected]...
#2 lore raw

PATCH v8 1/1 convert *_inline attributes to bool

In reply to Antonio Quartulli

Carrying around the INLINE_TAG is not really efficient, because it requires a strcmp() to be performed every time we want to understand if the data is stored inline or not.

Convert all the *_inline attributes to bool to make the logic easier and checks more efficient.

---

Changes from v7:
- rebased on top of latest master (68e0b9db)

Changes from v6:
- rebased on top of latest master

Changes from v5:
- fix function invocation alignment in options.c:options_postprocess_filechecks()
- fix typ0 in function invocation in options.c:options_postprocess_filechecks()
- fix doxygen comment for function tls_ctx_reload_crl() in ssl.c

Changes from v4:
- remove newline accidentally added in v4

Changes from v3:
- some code style adjustment in options.c:check_inline_file()
- move print_if_inline() from misc.c to crypto.c and rename it to print_key_filename()
- make comment of check_file_access_inline() and check_file_access_chroot_inline() doxygen compliant
- remove *is_inline argument in check_inline_file() and use its return value instead
- move declarations of is_inline to narrower scope in options.c
- move return type of plugin_option_list_add() to its own line

Changes from v2:
- fix indentation in ssl_openssl.c
- do not attempt to push inline'd options
- do not attempt to parse inline'd plugin
- introduce check_file_access_inline() and check_file_access_chroot_inline()
- introduce OPT_P_INLINE to specify when an option is allowed to be inline. Options not having this permission will fail to be parsed if is_inline is true

Changes from v1:
- remove the INLINE_TAG from the options parsing logic at all. Now a boolean variable is passed around
- add print_if_inline() helper function (to misc.c/h) to make sure we never print the inline data, but only the INLINE tag. Such function checks also for NULL pointers
- make sure print_if_inline() is always used when printing possibly inline data
- remove the INLINE_TAG from the options parsing logic at all. Now a boolean variable is passed around
- fix alignment error in comment
- remove CHKACC_INLINE from check_file_access() logic: this function is now not invoked at all in case of inline data

 src/openvpn/crypto.c      |  39 +++--
 src/openvpn/crypto.h      |  17 ++-
 src/openvpn/misc.c        |   6 +-
 src/openvpn/misc.h        |   3 +-
 src/openvpn/options.c     | 300 +++++++++++++++++++++-----------------
 src/openvpn/options.h     |  25 ++--
 src/openvpn/plugin.c      |   5 +-
 src/openvpn/plugin.h      |   3 +-
 src/openvpn/push.c        |   2 +-
 src/openvpn/push.h        |   3 +-
 src/openvpn/ssl.c         |   4 +-
 src/openvpn/ssl_backend.h |  55 +++----
 src/openvpn/ssl_common.h  |   2 +-
 src/openvpn/ssl_mbedtls.c |  61 ++++----
 src/openvpn/ssl_openssl.c |  85 ++++++-----
 src/openvpn/tls_crypt.c   |   2 +-
 src/openvpn/tls_crypt.h   |   9 +-
 17 files changed, 344 insertions(+), 277 deletions(-)
src/openvpn/crypto.c +26 -13
index c8d95f3b..dc5b8f1b 100644
@@ -1174,25 +1174,25 @@ test_crypto(struct crypto_options *co, struct frame *frame)
 
 void
 crypto_read_openvpn_key(const struct key_type *key_type,
-                        struct key_ctx_bi *ctx, const char *key_file, const char *key_inline,
-                        const int key_direction, const char *key_name, const char *opt_name)
+                        struct key_ctx_bi *ctx, const char *key_file,
+                        bool key_inline, const int key_direction,
+                        const char *key_name, const char *opt_name)
 {
     struct key2 key2;
     struct key_direction_state kds;
+    unsigned int flags = RKF_MUST_SUCCEED;
 
     if (key_inline)
     {
-        read_key_file(&key2, key_inline, RKF_MUST_SUCCEED|RKF_INLINE);
-    }
-    else
-    {
-        read_key_file(&key2, key_file, RKF_MUST_SUCCEED);
+        flags |= RKF_INLINE;
     }
+    read_key_file(&key2, key_file, flags);
 
     if (key2.n != 2)
     {
         msg(M_ERR, "File '%s' does not have OpenVPN Static Key format.  Using "
-            "free-form passphrase file is not supported anymore.", key_file);
+            "free-form passphrase file is not supported anymore.",
+            print_key_filename(key_file, key_inline));
     }
 
     /* check for and fix highly unlikely key problems */
@@ -1226,7 +1226,6 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags)
     struct buffer in;
     int size;
     uint8_t hex_byte[3] = {0, 0, 0};
-    const char *error_filename = file;
 
     /* parse info */
     const unsigned char *cp;
@@ -1264,7 +1263,6 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags)
     {
         size = strlen(file) + 1;
         buf_set_read(&in, (const uint8_t *)file, size);
-        error_filename = INLINE_FILE_TAG;
     }
     else /* 'file' is a filename which refers to a file containing the ascii key */
     {
@@ -1358,7 +1356,9 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags)
                 {
                     msg(M_FATAL,
                         (isprint(c) ? printable_char_fmt : unprintable_char_fmt),
-                        c, line_num, error_filename, count, onekeylen, keylen);
+                        c, line_num,
+                        print_key_filename(file, flags & RKF_INLINE), count,
+                        onekeylen, keylen);
                 }
             }
             ++line_index;
@@ -1379,13 +1379,15 @@ read_key_file(struct key2 *key2, const char *file, const unsigned int flags)
         if (!key2->n)
         {
             msg(M_FATAL, "Insufficient key material or header text not found in file '%s' (%d/%d/%d bytes found/min/max)",
-                error_filename, count, onekeylen, keylen);
+                print_key_filename(file, flags & RKF_INLINE), count, onekeylen,
+                keylen);
         }
 
         if (state != PARSE_FINISHED)
         {
             msg(M_FATAL, "Footer text not found in file '%s' (%d/%d/%d bytes found/min/max)",
-                error_filename, count, onekeylen, keylen);
+                print_key_filename(file, flags & RKF_INLINE), count, onekeylen,
+                keylen);
         }
     }
 
@@ -1815,3 +1817,14 @@ translate_cipher_name_to_openvpn(const char *cipher_name)
 
     return pair->openvpn_name;
 }
+
+const char *
+print_key_filename(const char *str, bool is_inline)
+{
+    if (is_inline)
+    {
+        return INLINE_FILE_TAG;
+    }
+
+    return np(str);
+}
src/openvpn/crypto.h +15 -2
index f4b3dca3..bf54bb03 100644
@@ -487,8 +487,9 @@ void key2_print(const struct key2 *k,
                 const char *prefix1);
 
 void crypto_read_openvpn_key(const struct key_type *key_type,
-                             struct key_ctx_bi *ctx, const char *key_file, const char *key_inline,
-                             const int key_direction, const char *key_name, const char *opt_name);
+                             struct key_ctx_bi *ctx, const char *key_file,
+                             bool key_inline, const int key_direction,
+                             const char *key_name, const char *opt_name);
 
 /*
  * Inline functions
@@ -519,4 +520,16 @@ key_ctx_bi_defined(const struct key_ctx_bi *key)
     return key->encrypt.cipher || key->encrypt.hmac || key->decrypt.cipher || key->decrypt.hmac;
 }
 
+/**
+ * To be used when printing a string that may contain inline data.
+ *
+ * If "is_inline" is true, return the inline tag.
+ * If "is_inline" is false and "str" is not NULL, return "str".
+ * Return the constant string "[NULL]" otherwise.
+ *
+ * @param str       the original string to return when is_inline is false
+ * @param is_inline true when str contains an inline data of some sort
+ */
+const char *print_key_filename(const char *str, bool is_inline);
+
 #endif /* CRYPTO_H */
src/openvpn/misc.c +3 -3
index 71fa2135..a1067662 100644
@@ -624,12 +624,12 @@ make_arg_copy(char **p, struct gc_arena *gc)
 }
 
 const char **
-make_extended_arg_array(char **p, struct gc_arena *gc)
+make_extended_arg_array(char **p, bool is_inline, struct gc_arena *gc)
 {
     const int argc = string_array_len((const char **)p);
-    if (!strcmp(p[0], INLINE_FILE_TAG) && argc == 2)
+    if (is_inline)
     {
-        return make_inline_array(p[1], gc);
+        return make_inline_array(p[0], gc);
     }
     else if (argc == 0)
     {
src/openvpn/misc.h +2 -1
index 9d4601f0..ff6d6827 100644
@@ -50,7 +50,8 @@ void save_inetd_socket_descriptor(void);
 
 const char **make_arg_array(const char *first, const char *parms, struct gc_arena *gc);
 
-const char **make_extended_arg_array(char **p, struct gc_arena *gc);
+const char **make_extended_arg_array(char **p, bool is_inline,
+                                     struct gc_arena *gc);
 
 /* prepend a random prefix to hostname */
 const char *hostname_randomize(const char *hostname, struct gc_arena *gc);
src/openvpn/options.c +166 -134

549 lines not on this page · raw message

src/openvpn/options.h +13 -12
index acbd1087..e1ec34e8 100644
@@ -133,12 +133,12 @@ struct connection_entry
 
     /* Shared secret used for TLS control channel authentication */
     const char *tls_auth_file;
-    const char *tls_auth_file_inline;
+    bool 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;
+    bool tls_crypt_inline;
 };
 
 struct remote_entry
@@ -476,7 +476,7 @@ struct options
 
     /* Cipher parms */
     const char *shared_secret_file;
-    const char *shared_secret_file_inline;
+    bool shared_secret_file_inline;
     int key_direction;
     const char *ciphername;
     bool ncp_enabled;
@@ -515,13 +515,13 @@ struct options
     const char *tls_export_cert;
     const char *crl_file;
 
-    const char *ca_file_inline;
-    const char *cert_file_inline;
-    const char *extra_certs_file_inline;
-    const char *crl_file_inline;
-    char *priv_key_file_inline;
-    const char *dh_file_inline;
-    const char *pkcs12_file_inline; /* contains the base64 encoding of pkcs12 file */
+    bool ca_file_inline;
+    bool cert_file_inline;
+    bool extra_certs_file_inline;
+    bool crl_file_inline;
+    bool priv_key_file_inline;
+    bool dh_file_inline;
+    bool pkcs12_file_inline;
 
     int ns_cert_type; /* set to 0, NS_CERT_CHECK_SERVER, or NS_CERT_CHECK_CLIENT */
     unsigned remote_cert_ku[MAX_PARMS];
@@ -570,11 +570,11 @@ struct options
 
     /* Shared secret used for TLS control channel authentication */
     const char *tls_auth_file;
-    const char *tls_auth_file_inline;
+    bool tls_auth_file_inline;
 
     /* Shared secret used for TLS control channel authenticated encryption */
     const char *tls_crypt_file;
-    const char *tls_crypt_inline;
+    bool tls_crypt_inline;
 
     /* Allow only one session */
     bool single_session;
@@ -647,6 +647,7 @@ struct options
 #define OPT_P_SOCKFLAGS       (1<<26)
 #define OPT_P_CONNECTION      (1<<27)
 #define OPT_P_PEER_ID         (1<<28)
+#define OPT_P_INLINE          (1<<29)
 
 #define OPT_P_DEFAULT   (~(OPT_P_INSTANCE|OPT_P_PULL_MODE))
 
src/openvpn/plugin.c +3 -2
index 4d17c821..c5a61db6 100644
@@ -161,12 +161,13 @@ plugin_option_list_new(struct gc_arena *gc)
 }
 
 bool
-plugin_option_list_add(struct plugin_option_list *list, char **p, struct gc_arena *gc)
+plugin_option_list_add(struct plugin_option_list *list, char **p,
+                       struct gc_arena *gc)
 {
     if (list->n < MAX_PLUGINS)
     {
         struct plugin_option *o = &list->plugins[list->n++];
-        o->argv = make_extended_arg_array(p, gc);
+        o->argv = make_extended_arg_array(p, false, gc);
         if (o->argv[0])
         {
             o->so_pathname = o->argv[0];
src/openvpn/plugin.h +2 -1
index 791d476b..bf4d71bb 100644
@@ -106,7 +106,8 @@ struct plugin_return
 
 struct plugin_option_list *plugin_option_list_new(struct gc_arena *gc);
 
-bool plugin_option_list_add(struct plugin_option_list *list, char **p, struct gc_arena *gc);
+bool plugin_option_list_add(struct plugin_option_list *list, char **p,
+                            struct gc_arena *gc);
 
 #ifndef ENABLE_SMALL
 void plugin_option_list_print(const struct plugin_option_list *list, int msglevel);
src/openvpn/push.c +1 -1
index a7ec4dd6..6cf37565 100644
@@ -569,7 +569,7 @@ clone_push_list(struct options *o)
 void
 push_options(struct options *o, char **p, int msglevel, struct gc_arena *gc)
 {
-    const char **argv = make_extended_arg_array(p, gc);
+    const char **argv = make_extended_arg_array(p, false, gc);
     char *opt = print_argv(argv, gc, 0);
     push_option(o, opt, msglevel);
 }
src/openvpn/push.h +2 -1
index 5f6181e7..bf313297 100644
@@ -57,7 +57,8 @@ void clone_push_list(struct options *o);
 
 void push_option(struct options *o, const char *opt, int msglevel);
 
-void push_options(struct options *o, char **p, int msglevel, struct gc_arena *gc);
+void push_options(struct options *o, char **p, int msglevel,
+                  struct gc_arena *gc);
 
 void push_reset(struct options *o);
 
src/openvpn/ssl.c +2 -2
index 4257c33d..6b0f26d0 100644
@@ -555,7 +555,7 @@ tls_version_parse(const char *vstr, const char *extra)
  */
 static void
 tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file,
-                   const char *crl_file_inline)
+                   bool crl_file_inline)
 {
     /* if something goes wrong with stat(), we'll store 0 as mtime */
     platform_stat_t crl_stat = {0};
@@ -661,7 +661,7 @@ init_ssl(const struct options *options, struct tls_root_ctx *new_ctx)
     {
         char *cert = management_query_cert(management,
                                            options->management_certificate);
-        tls_ctx_load_cert_file(new_ctx, INLINE_FILE_TAG, cert);
+        tls_ctx_load_cert_file(new_ctx, cert, true);
         free(cert);
     }
 #endif
src/openvpn/ssl_backend.h +30 -25

117 lines not on this page · raw message

src/openvpn/ssl_common.h +1 -1
index 08ef6ffa..e5883c1a 100644
@@ -263,7 +263,7 @@ struct tls_options
     int verify_x509_type;
     const char *verify_x509_name;
     const char *crl_file;
-    const char *crl_file_inline;
+    bool crl_file_inline;
     int ns_cert_type;
     unsigned remote_cert_ku[MAX_PARMS];
     const char *remote_cert_eku;
src/openvpn/ssl_mbedtls.c +30 -31

158 lines not on this page · raw message

src/openvpn/ssl_openssl.c +42 -43

236 lines not on this page · raw message

src/openvpn/tls_crypt.c +1 -1
index 36ead84d..ba2e82c3 100644
@@ -66,7 +66,7 @@ tls_crypt_buf_overhead(void)
 
 void
 tls_crypt_init_key(struct key_ctx_bi *key, const char *key_file,
-                   const char *key_inline, bool tls_server)
+                   bool key_inline, bool tls_server)
 {
     const int key_direction = tls_server ?
                               KEY_DIRECTION_NORMAL : KEY_DIRECTION_INVERSE;
src/openvpn/tls_crypt.h +5 -4
index 067758ca..2b0d8f4b 100644
@@ -91,13 +91,14 @@
  *
  * @param key           The key context to initialize
  * @param key_file      The file to read the key from (or the inline tag to
- *                      indicate and inline key).
- * @param key_inline    Array containing (zero-terminated) inline key, or NULL
- *                      if not used.
+ *                      indicate and inline key) or an array containing
+ *                      (zero-terminated) inline key.
+ * @param key_inline    True if key_file contains an inline key, False
+ *                      otherwise.
  * @param tls_server    Must be set to true is this is a TLS server instance.
  */
 void tls_crypt_init_key(struct key_ctx_bi *key, const char *key_file,
-                        const char *key_inline, bool tls_server);
+                        bool key_inline, bool tls_server);
 
 /**
  * Returns the maximum overhead (in bytes) added to the destination buffer by
Signature · 1 line
2.19.1
#3 lore raw

PATCH v9 1/1 convert *_inline attributes to bool

Carrying around the INLINE_TAG is not really efficient, because it requires a strcmp() to be performed every time we want to understand if the data is stored inline or not.

Convert all the *_inline attributes to bool to make the logic easier and checks more efficient.

---

Changes from v8:
- rebased on top of latest master
- changed auth-token secret file inlining logic too (not present before)
- adapted test_tls_crypt.c
- adapted test_auth_token.c

Changes from v7:
- rebased on top of latest master (68e0b9db)

Changes from v6:
- rebased on top of latest master

Changes from v5:
- fix function invocation alignment in options.c:options_postprocess_filechecks()
- fix typ0 in function invocation in options.c:options_postprocess_filechecks()
- fix doxygen comment for function tls_ctx_reload_crl() in ssl.c

Changes from v4:
- remove newline accidentally added in v4

Changes from v3:
- some code style adjustment in options.c:check_inline_file()
- move print_if_inline() from misc.c to crypto.c and rename it to print_key_filename()
- make comment of check_file_access_inline() and check_file_access_chroot_inline() doxygen compliant
- remove *is_inline argument in check_inline_file() and use its return value instead
- move declarations of is_inline to narrower scope in options.c
- move return type of plugin_option_list_add() to its own line

Changes from v2:
- fix indentation in ssl_openssl.c
- do not attempt to push inline'd options
- do not attempt to parse inline'd plugin
- introduce check_file_access_inline() and check_file_access_chroot_inline()
- introduce OPT_P_INLINE to specify when an option is allowed to be inline. Options not having this permission will fail to be parsed if is_inline is true

Changes from v1:
- remove the INLINE_TAG from the options parsing logic at all. Now a boolean variable is passed around
- add print_if_inline() helper function (to misc.c/h) to make sure we never print the inline data, but only the INLINE tag. Such function checks also for NULL pointers
- make sure print_if_inline() is always used when printing possibly inline data
- remove the INLINE_TAG from the options parsing logic at all. Now a boolean variable is passed around
- fix alignment error in comment
- remove CHKACC_INLINE from check_file_access() logic: this function is now not invoked at all in case of inline data

 src/openvpn/auth_token.c                   |   2 +-
 src/openvpn/auth_token.h                   |   2 +-
 src/openvpn/crypto.c                       |  47 +--
 src/openvpn/crypto.h                       |  25 +-
 src/openvpn/init.c                         |   9 +-
 src/openvpn/misc.c                         |   6 +-
 src/openvpn/misc.h                         |   3 +-
 src/openvpn/options.c                      | 329 +++++++++++----------
 src/openvpn/options.h                      |  32 +-
 src/openvpn/plugin.c                       |   5 +-
 src/openvpn/plugin.h                       |   3 +-
 src/openvpn/push.c                         |   2 +-
 src/openvpn/push.h                         |   3 +-
 src/openvpn/ssl.c                          |   4 +-
 src/openvpn/ssl_backend.h                  |  55 ++--
 src/openvpn/ssl_common.h                   |   2 +-
 src/openvpn/ssl_mbedtls.c                  |  61 ++--
 src/openvpn/ssl_openssl.c                  |  85 +++---
 src/openvpn/tls_crypt.c                    |   8 +-
 src/openvpn/tls_crypt.h                    |  43 +--
 tests/unit_tests/openvpn/test_auth_token.c |   6 +-
 tests/unit_tests/openvpn/test_tls_crypt.c  |   3 +-
 22 files changed, 402 insertions(+), 333 deletions(-)
src/openvpn/auth_token.c +1 -1
index 6275299d..d9212ee7 100644
@@ -135,7 +135,7 @@ auth_token_write_server_key_file(const char *filename)
 
 void
 auth_token_init_secret(struct key_ctx *key_ctx, const char *key_file,
-                       const char *key_inline)
+                       bool key_inline)
 {
     struct key_type kt = auth_token_kt();
 
src/openvpn/auth_token.h +1 -1
index 4b014d44..6f34b760 100644
@@ -74,7 +74,7 @@ verify_auth_token(struct user_pass *up, struct tls_multi *multi,
  */
 void
 auth_token_init_secret(struct key_ctx *key_ctx, const char *key_file,
-                       const char *key_inline);
+                       bool key_inline);
 
 
 /**
src/openvpn/crypto.c +30 -17

118 lines not on this page · raw message

src/openvpn/crypto.h +20 -5
index 18a86ceb..6d8de94c 100644
@@ -444,13 +444,15 @@ generate_ephemeral_key(struct buffer *key, const char *pem_name);
  * Read key material from a PEM encoded files into the key structure
  * @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
- * @param key_inline    a string holding the data in case of an inline key
+ * @param key_file      name of the file to read or the key itself if
+ *                      key_inline is true
+ * @param key_inline    True if key_file contains an inline key, False
+ *                      otherwise.
  * @return              true if reading into key was successful
  */
 bool
 read_pem_key_file(struct buffer *key, const char *pem_name,
-                  const char *key_file, const char *key_inline);
+                  const char *key_file, bool key_inline);
 
 /* Minimum length of the nonce used by the PRNG */
 #define NONCE_SECRET_LEN_MIN 16
@@ -517,8 +519,9 @@ void key2_print(const struct key2 *k,
                 const char *prefix1);
 
 void crypto_read_openvpn_key(const struct key_type *key_type,
-                             struct key_ctx_bi *ctx, const char *key_file, const char *key_inline,
-                             const int key_direction, const char *key_name, const char *opt_name);
+                             struct key_ctx_bi *ctx, const char *key_file,
+                             bool key_inline, const int key_direction,
+                             const char *key_name, const char *opt_name);
 
 /*
  * Inline functions
@@ -550,4 +553,16 @@ key_ctx_bi_defined(const struct key_ctx_bi *key)
     return key->encrypt.cipher || key->encrypt.hmac || key->decrypt.cipher || key->decrypt.hmac;
 }
 
+/**
+ * To be used when printing a string that may contain inline data.
+ *
+ * If "is_inline" is true, return the inline tag.
+ * If "is_inline" is false and "str" is not NULL, return "str".
+ * Return the constant string "[NULL]" otherwise.
+ *
+ * @param str       the original string to return when is_inline is false
+ * @param is_inline true when str contains an inline data of some sort
+ */
+const char *print_key_filename(const char *str, bool is_inline);
+
 #endif /* CRYPTO_H */
src/openvpn/init.c +5 -4
index b3096dc8..3cb73fd4 100644
@@ -1108,7 +1108,7 @@ do_genkey(const struct options *options)
 
         tls_crypt_v2_write_client_key_file(options->genkey_filename,
                                            options->genkey_extra_data, options->tls_crypt_v2_file,
-                                           options->tls_crypt_v2_inline);
+                                           options->tls_crypt_v2_file_inline);
         return true;
     }
     else if (options->genkey && options->genkey_type == GENKEY_AUTH_TOKEN)
@@ -2633,7 +2633,8 @@ do_init_tls_wrap_key(struct context *c)
     {
         tls_crypt_init_key(&c->c1.ks.tls_wrap_key,
                            options->ce.tls_crypt_file,
-                           options->ce.tls_crypt_inline, options->tls_server);
+                           options->ce.tls_crypt_file_inline,
+			   options->tls_server);
     }
 
     /* tls-crypt with client-specific keys (--tls-crypt-v2) */
@@ -2643,14 +2644,14 @@ do_init_tls_wrap_key(struct context *c)
         {
             tls_crypt_v2_init_server_key(&c->c1.ks.tls_crypt_v2_server_key,
                                          true, options->ce.tls_crypt_v2_file,
-                                         options->ce.tls_crypt_v2_inline);
+                                         options->ce.tls_crypt_v2_file_inline);
         }
         else
         {
             tls_crypt_v2_init_client_key(&c->c1.ks.tls_wrap_key,
                                          &c->c1.ks.tls_crypt_v2_wkc,
                                          options->ce.tls_crypt_v2_file,
-                                         options->ce.tls_crypt_v2_inline);
+                                         options->ce.tls_crypt_v2_file_inline);
         }
     }
 
src/openvpn/misc.c +3 -3
index 9d739d57..b4eff3b4 100644
@@ -611,12 +611,12 @@ make_arg_copy(char **p, struct gc_arena *gc)
 }
 
 const char **
-make_extended_arg_array(char **p, struct gc_arena *gc)
+make_extended_arg_array(char **p, bool is_inline, struct gc_arena *gc)
 {
     const int argc = string_array_len((const char **)p);
-    if (!strcmp(p[0], INLINE_FILE_TAG) && argc == 2)
+    if (is_inline)
     {
-        return make_inline_array(p[1], gc);
+        return make_inline_array(p[0], gc);
     }
     else if (argc == 0)
     {
src/openvpn/misc.h +2 -1
index 2605c6d2..a3095c1f 100644
@@ -50,7 +50,8 @@ void save_inetd_socket_descriptor(void);
 
 const char **make_arg_array(const char *first, const char *parms, struct gc_arena *gc);
 
-const char **make_extended_arg_array(char **p, struct gc_arena *gc);
+const char **make_extended_arg_array(char **p, bool is_inline,
+                                     struct gc_arena *gc);
 
 /* prepend a random prefix to hostname */
 const char *hostname_randomize(const char *hostname, struct gc_arena *gc);
src/openvpn/options.c +180 -149

609 lines not on this page · raw message

src/openvpn/options.h +16 -16
index 4c1737e1..96e302bf 100644
@@ -133,17 +133,17 @@ struct connection_entry
 
     /* Shared secret used for TLS control channel authentication */
     const char *tls_auth_file;
-    const char *tls_auth_file_inline;
+    bool 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;
+    bool tls_crypt_file_inline;
 
     /* Client-specific secret or server key used for TLS control channel
      * authenticated encryption v2 */
     const char *tls_crypt_v2_file;
-    const char *tls_crypt_v2_inline;
+    bool tls_crypt_v2_file_inline;
 };
 
 struct remote_entry
@@ -480,7 +480,7 @@ struct options
     bool auth_token_call_auth;
     int auth_token_lifetime;
     const char *auth_token_secret_file;
-    const char *auth_token_secret_file_inline;
+    bool auth_token_secret_file_inline;
 
 #if PORT_SHARE
     char *port_share_host;
@@ -505,7 +505,7 @@ struct options
 
     /* Cipher parms */
     const char *shared_secret_file;
-    const char *shared_secret_file_inline;
+    bool shared_secret_file_inline;
     int key_direction;
     const char *ciphername;
     bool ncp_enabled;
@@ -529,12 +529,18 @@ struct options
     bool tls_server;
     bool tls_client;
     const char *ca_file;
+    bool ca_file_inline;
     const char *ca_path;
     const char *dh_file;
+    bool dh_file_inline;
     const char *cert_file;
+    bool cert_file_inline;
     const char *extra_certs_file;
+    bool extra_certs_file_inline;
     const char *priv_key_file;
+    bool priv_key_file_inline;
     const char *pkcs12_file;
+    bool pkcs12_file_inline;
     const char *cipher_list;
     const char *cipher_list_tls13;
     const char *tls_cert_profile;
@@ -544,14 +550,7 @@ struct options
     const char *verify_x509_name;
     const char *tls_export_cert;
     const char *crl_file;
-
-    const char *ca_file_inline;
-    const char *cert_file_inline;
-    const char *extra_certs_file_inline;
-    const char *crl_file_inline;
-    char *priv_key_file_inline;
-    const char *dh_file_inline;
-    const char *pkcs12_file_inline; /* contains the base64 encoding of pkcs12 file */
+    bool crl_file_inline;
 
     int ns_cert_type; /* set to 0, NS_CERT_CHECK_SERVER, or NS_CERT_CHECK_CLIENT */
     unsigned remote_cert_ku[MAX_PARMS];
@@ -600,16 +599,16 @@ struct options
 
     /* Shared secret used for TLS control channel authentication */
     const char *tls_auth_file;
-    const char *tls_auth_file_inline;
+    bool tls_auth_file_inline;
 
     /* Shared secret used for TLS control channel authenticated encryption */
     const char *tls_crypt_file;
-    const char *tls_crypt_inline;
+    bool tls_crypt_file_inline;
 
     /* Client-specific secret or server key used for TLS control channel
      * authenticated encryption v2 */
     const char *tls_crypt_v2_file;
-    const char *tls_crypt_v2_inline;
+    bool tls_crypt_v2_file_inline;
 
     const char *tls_crypt_v2_metadata;
 
@@ -691,6 +690,7 @@ struct options
 #define OPT_P_SOCKFLAGS       (1<<26)
 #define OPT_P_CONNECTION      (1<<27)
 #define OPT_P_PEER_ID         (1<<28)
+#define OPT_P_INLINE          (1<<29)
 
 #define OPT_P_DEFAULT   (~(OPT_P_INSTANCE|OPT_P_PULL_MODE))
 
src/openvpn/plugin.c +3 -2
index dba9d335..4de1d6b7 100644
@@ -161,12 +161,13 @@ plugin_option_list_new(struct gc_arena *gc)
 }
 
 bool
-plugin_option_list_add(struct plugin_option_list *list, char **p, struct gc_arena *gc)
+plugin_option_list_add(struct plugin_option_list *list, char **p,
+                       struct gc_arena *gc)
 {
     if (list->n < MAX_PLUGINS)
     {
         struct plugin_option *o = &list->plugins[list->n++];
-        o->argv = make_extended_arg_array(p, gc);
+        o->argv = make_extended_arg_array(p, false, gc);
         if (o->argv[0])
         {
             o->so_pathname = o->argv[0];
src/openvpn/plugin.h +2 -1
index 791d476b..bf4d71bb 100644
@@ -106,7 +106,8 @@ struct plugin_return
 
 struct plugin_option_list *plugin_option_list_new(struct gc_arena *gc);
 
-bool plugin_option_list_add(struct plugin_option_list *list, char **p, struct gc_arena *gc);
+bool plugin_option_list_add(struct plugin_option_list *list, char **p,
+                            struct gc_arena *gc);
 
 #ifndef ENABLE_SMALL
 void plugin_option_list_print(const struct plugin_option_list *list, int msglevel);
src/openvpn/push.c +1 -1
index 39a906d4..ab0cdf6a 100644
@@ -650,7 +650,7 @@ clone_push_list(struct options *o)
 void
 push_options(struct options *o, char **p, int msglevel, struct gc_arena *gc)
 {
-    const char **argv = make_extended_arg_array(p, gc);
+    const char **argv = make_extended_arg_array(p, false, gc);
     char *opt = print_argv(argv, gc, 0);
     push_option(o, opt, msglevel);
 }
src/openvpn/push.h +2 -1
index 070782dd..6cabc44d 100644
@@ -57,7 +57,8 @@ void clone_push_list(struct options *o);
 
 void push_option(struct options *o, const char *opt, int msglevel);
 
-void push_options(struct options *o, char **p, int msglevel, struct gc_arena *gc);
+void push_options(struct options *o, char **p, int msglevel,
+                  struct gc_arena *gc);
 
 void push_reset(struct options *o);
 
src/openvpn/ssl.c +2 -2
index 80e0d5ac..88b4cc7d 100644
@@ -558,7 +558,7 @@ tls_version_parse(const char *vstr, const char *extra)
  */
 static void
 tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file,
-                   const char *crl_file_inline)
+                   bool crl_file_inline)
 {
     /* if something goes wrong with stat(), we'll store 0 as mtime */
     platform_stat_t crl_stat = {0};
@@ -665,7 +665,7 @@ init_ssl(const struct options *options, struct tls_root_ctx *new_ctx)
     {
         char *cert = management_query_cert(management,
                                            options->management_certificate);
-        tls_ctx_load_cert_file(new_ctx, INLINE_FILE_TAG, cert);
+        tls_ctx_load_cert_file(new_ctx, cert, true);
         free(cert);
     }
 #endif
src/openvpn/ssl_backend.h +30 -25

117 lines not on this page · raw message

src/openvpn/ssl_common.h +1 -1
index 998ea3c4..f48990d6 100644
@@ -269,7 +269,7 @@ struct tls_options
     int verify_x509_type;
     const char *verify_x509_name;
     const char *crl_file;
-    const char *crl_file_inline;
+    bool crl_file_inline;
     int ns_cert_type;
     unsigned remote_cert_ku[MAX_PARMS];
     const char *remote_cert_eku;
src/openvpn/ssl_mbedtls.c +30 -31

158 lines not on this page · raw message

src/openvpn/ssl_openssl.c +42 -43

237 lines not on this page · raw message

src/openvpn/tls_crypt.c +4 -4
index e9f9cc2a..16424215 100644
@@ -79,7 +79,7 @@ tls_crypt_buf_overhead(void)
 
 void
 tls_crypt_init_key(struct key_ctx_bi *key, const char *key_file,
-                   const char *key_inline, bool tls_server)
+                   bool key_inline, bool tls_server)
 {
     const int key_direction = tls_server ?
                               KEY_DIRECTION_NORMAL : KEY_DIRECTION_INVERSE;
@@ -295,7 +295,7 @@ tls_crypt_v2_load_client_key(struct key_ctx_bi *key, const struct key2 *key2,
 
 void
 tls_crypt_v2_init_client_key(struct key_ctx_bi *key, struct buffer *wkc_buf,
-                             const char *key_file, const char *key_inline)
+                             const char *key_file, bool key_inline)
 {
     struct buffer client_key = alloc_buf(TLS_CRYPT_V2_CLIENT_KEY_LEN
                                          + TLS_CRYPT_V2_MAX_WKC_LEN);
@@ -320,7 +320,7 @@ tls_crypt_v2_init_client_key(struct key_ctx_bi *key, struct buffer *wkc_buf,
 
 void
 tls_crypt_v2_init_server_key(struct key_ctx *key_ctx, bool encrypt,
-                             const char *key_file, const char *key_inline)
+                             const char *key_file, bool key_inline)
 {
     struct key srv_key;
     struct buffer srv_key_buf;
@@ -638,7 +638,7 @@ void
 tls_crypt_v2_write_client_key_file(const char *filename,
                                    const char *b64_metadata,
                                    const char *server_key_file,
-                                   const char *server_key_inline)
+				   bool server_key_inline)
 {
     struct gc_arena gc = gc_new();
     struct key_ctx server_key = { 0 };
src/openvpn/tls_crypt.h +23 -20

76 lines not on this page · raw message

tests/unit_tests/openvpn/test_auth_token.c +3 -3
index 6b30dfbd..dbde8631 100644
@@ -341,7 +341,7 @@ auth_token_test_random_keys(void **state)
     ctx->multi.auth_token = strdup(random_token);
 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
-    auth_token_init_secret(&ctx->multi.opt.auth_token_key, INLINE_FILE_TAG, random_key);
+    auth_token_init_secret(&ctx->multi.opt.auth_token_key, random_key, true);
 
     /* Zero the hmac part to ensure we have a newly generated token */
     zerohmac(ctx->multi.auth_token);
@@ -361,12 +361,12 @@ auth_token_test_key_load(void **state)
     struct test_context *ctx = (struct test_context *) *state;
 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
-    auth_token_init_secret(&ctx->multi.opt.auth_token_key, INLINE_FILE_TAG, zeroinline);
+    auth_token_init_secret(&ctx->multi.opt.auth_token_key, zeroinline, true);
     strcpy(ctx->up.password, now0key0);
     assert_true(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session));
 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
-    auth_token_init_secret(&ctx->multi.opt.auth_token_key, INLINE_FILE_TAG, allx01inline);
+    auth_token_init_secret(&ctx->multi.opt.auth_token_key, allx01inline, true);
     assert_false(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session));
 }
 
tests/unit_tests/openvpn/test_tls_crypt.c +1 -2
index b9e3a7a6..c25513ec 100644
@@ -531,8 +531,7 @@ test_tls_crypt_v2_write_client_key_file(void **state) {
     expect_string(__wrap_buffer_read_from_file, filename, filename);
     will_return(__wrap_buffer_read_from_file, test_client_key);
 
-    tls_crypt_v2_write_client_key_file(filename, NULL, INLINE_FILE_TAG,
-                                       test_server_key);
+    tls_crypt_v2_write_client_key_file(filename, NULL, test_server_key, true);
 }
 
 int
Signature · 1 line
2.26.2
Acked-by on v9 1/1
#4 lore raw

Am 22.04.20 um 11:26 schrieb Antonio Quartulli:

Quoted 8 lines

Carrying around the INLINE_TAG is not really efficient, because it requires a strcmp() to be performed every time we want to understand if the data is stored inline or not.

Convert all the *_inline attributes to bool to make the logic easier and checks more efficient.

Look through it. It is only a minor improvement but Antonio has been persistent enough and spend enough time to make 9 versions of this so I took the time to look through it:

The patch is not uncrustify clean at the moment. So uncrustify should be run on commit or we need a v10 of this patch.

The patch does not apply cleanly anymore: The test_tls_crypt.c part should be now.

tests/unit_tests/openvpn/test_tls_crypt.c +1 -2
@@ -549,8 +549,7 @@ test_tls_crypt_v2_write_client_key_file(void **state) {
     expect_string(__wrap_buffer_read_from_file, filename, filename);
     will_return(__wrap_buffer_read_from_file, test_client_key);
 
-    tls_crypt_v2_write_client_key_file(filename, NULL, INLINE_FILE_TAG,
-                                      test_server_key);
+  tls_crypt_v2_write_client_key_file(filename, NULL, test_server_key,

true);
}

static void
@@ -567,8 +566,7 @@

test_tls_crypt_v2_write_client_key_file_metadata(void **state) {
     expect_string(__wrap_buffer_read_from_file, filename, filename);
     will_return(__wrap_buffer_read_from_file, test_client_key_metadata);

- tls_crypt_v2_write_client_key_file(filename, b64metadata,
INLINE_FILE_TAG,
- test_server_key);
+ tls_crypt_v2_write_client_key_file(filename, NULL, test_server_key,
true);
}

#5 lore raw

Hi,

Quoted 14 lines · Arne Schwabe

Am 22.04.20 um 11:26 schrieb Antonio Quartulli:

Quoted 8 lines

Carrying around the INLINE_TAG is not really efficient, because it requires a strcmp() to be performed every time we want to understand if the data is stored inline or not.

Convert all the *_inline attributes to bool to make the logic easier and checks more efficient.

Look through it. It is only a minor improvement but Antonio has been persistent enough and spend enough time to make 9 versions of this so I took the time to look through it:

Quoted 7 lines

The patch is not uncrustify clean at the moment. So uncrustify should be run on commit or we need a v10 of this patch.

The patch does not apply cleanly anymore: The test_tls_crypt.c part should be now.

I can send v10 with these adjustments.

Thanks for the review!

Signature · 1 line
Antonio Quartulli
#6 lore raw

PATCH v10 1/1 convert *_inline attributes to bool

Carrying around the INLINE_TAG is not really efficient, because it requires a strcmp() to be performed every time we want to understand if the data is stored inline or not.

Convert all the *_inline attributes to bool to make the logic easier and checks more efficient.

---

Changes from v9:
- rebased on top of latest master
- checked with uncrustify-0.69.0
- adapt test_tls_crypt.c to new inline'd logic

Changes from v8:
- rebased on top of latest master
- changed auth-token secret file inlining logic too (not present before)
- adapted test_tls_crypt.c
- adapted test_auth_token.c

Changes from v7:
- rebased on top of latest master (68e0b9db)

Changes from v6:
- rebased on top of latest master

Changes from v5:
- fix function invocation alignment in options.c:options_postprocess_filechecks()
- fix typ0 in function invocation in options.c:options_postprocess_filechecks()
- fix doxygen comment for function tls_ctx_reload_crl() in ssl.c

Changes from v4:
- remove newline accidentally added in v4

Changes from v3:
- some code style adjustment in options.c:check_inline_file()
- move print_if_inline() from misc.c to crypto.c and rename it to print_key_filename()
- make comment of check_file_access_inline() and check_file_access_chroot_inline() doxygen compliant
- remove *is_inline argument in check_inline_file() and use its return value instead
- move declarations of is_inline to narrower scope in options.c
- move return type of plugin_option_list_add() to its own line

Changes from v2:
- fix indentation in ssl_openssl.c
- do not attempt to push inline'd options
- do not attempt to parse inline'd plugin
- introduce check_file_access_inline() and check_file_access_chroot_inline()
- introduce OPT_P_INLINE to specify when an option is allowed to be inline. Options not having this permission will fail to be parsed if is_inline is true

Changes from v1:
- remove the INLINE_TAG from the options parsing logic at all. Now a boolean variable is passed around
- add print_if_inline() helper function (to misc.c/h) to make sure we never print the inline data, but only the INLINE tag. Such function checks also for NULL pointers
- make sure print_if_inline() is always used when printing possibly inline data
- remove the INLINE_TAG from the options parsing logic at all. Now a boolean variable is passed around
- fix alignment error in comment
- remove CHKACC_INLINE from check_file_access() logic: this function is now not invoked at all in case of inline data

 src/openvpn/auth_token.c                   |   2 +-
 src/openvpn/auth_token.h                   |   2 +-
 src/openvpn/crypto.c                       |  47 +--
 src/openvpn/crypto.h                       |  25 +-
 src/openvpn/init.c                         |   9 +-
 src/openvpn/misc.c                         |   6 +-
 src/openvpn/misc.h                         |   3 +-
 src/openvpn/options.c                      | 329 +++++++++++----------
 src/openvpn/options.h                      |  32 +-
 src/openvpn/plugin.c                       |   5 +-
 src/openvpn/plugin.h                       |   3 +-
 src/openvpn/push.c                         |   2 +-
 src/openvpn/push.h                         |   3 +-
 src/openvpn/ssl.c                          |   4 +-
 src/openvpn/ssl_backend.h                  |  55 ++--
 src/openvpn/ssl_common.h                   |   2 +-
 src/openvpn/ssl_mbedtls.c                  |  61 ++--
 src/openvpn/ssl_openssl.c                  |  85 +++---
 src/openvpn/tls_crypt.c                    |   8 +-
 src/openvpn/tls_crypt.h                    |  43 +--
 tests/unit_tests/openvpn/test_auth_token.c |   6 +-
 tests/unit_tests/openvpn/test_tls_crypt.c  |   3 +-
 22 files changed, 402 insertions(+), 333 deletions(-)
src/openvpn/auth_token.c +1 -1
index 6275299d..d9212ee7 100644
@@ -135,7 +135,7 @@ auth_token_write_server_key_file(const char *filename)
 
 void
 auth_token_init_secret(struct key_ctx *key_ctx, const char *key_file,
-                       const char *key_inline)
+                       bool key_inline)
 {
     struct key_type kt = auth_token_kt();
 
src/openvpn/auth_token.h +1 -1
index 4b014d44..6f34b760 100644
@@ -74,7 +74,7 @@ verify_auth_token(struct user_pass *up, struct tls_multi *multi,
  */
 void
 auth_token_init_secret(struct key_ctx *key_ctx, const char *key_file,
-                       const char *key_inline);
+                       bool key_inline);
 
 
 /**
src/openvpn/crypto.c +30 -17

118 lines not on this page · raw message

src/openvpn/crypto.h +20 -5
index dadf0a90..999f643e 100644
@@ -444,13 +444,15 @@ generate_ephemeral_key(struct buffer *key, const char *pem_name);
  * Read key material from a PEM encoded files into the key structure
  * @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
- * @param key_inline    a string holding the data in case of an inline key
+ * @param key_file      name of the file to read or the key itself if
+ *                      key_inline is true
+ * @param key_inline    True if key_file contains an inline key, False
+ *                      otherwise.
  * @return              true if reading into key was successful
  */
 bool
 read_pem_key_file(struct buffer *key, const char *pem_name,
-                  const char *key_file, const char *key_inline);
+                  const char *key_file, bool key_inline);
 
 /* Minimum length of the nonce used by the PRNG */
 #define NONCE_SECRET_LEN_MIN 16
@@ -517,8 +519,9 @@ void key2_print(const struct key2 *k,
                 const char *prefix1);
 
 void crypto_read_openvpn_key(const struct key_type *key_type,
-                             struct key_ctx_bi *ctx, const char *key_file, const char *key_inline,
-                             const int key_direction, const char *key_name, const char *opt_name);
+                             struct key_ctx_bi *ctx, const char *key_file,
+                             bool key_inline, const int key_direction,
+                             const char *key_name, const char *opt_name);
 
 /*
  * Inline functions
@@ -536,4 +539,16 @@ key_ctx_bi_defined(const struct key_ctx_bi *key)
     return key->encrypt.cipher || key->encrypt.hmac || key->decrypt.cipher || key->decrypt.hmac;
 }
 
+/**
+ * To be used when printing a string that may contain inline data.
+ *
+ * If "is_inline" is true, return the inline tag.
+ * If "is_inline" is false and "str" is not NULL, return "str".
+ * Return the constant string "[NULL]" otherwise.
+ *
+ * @param str       the original string to return when is_inline is false
+ * @param is_inline true when str contains an inline data of some sort
+ */
+const char *print_key_filename(const char *str, bool is_inline);
+
 #endif /* CRYPTO_H */
src/openvpn/init.c +5 -4
index b3096dc8..3cb73fd4 100644
@@ -1108,7 +1108,7 @@ do_genkey(const struct options *options)
 
         tls_crypt_v2_write_client_key_file(options->genkey_filename,
                                            options->genkey_extra_data, options->tls_crypt_v2_file,
-                                           options->tls_crypt_v2_inline);
+                                           options->tls_crypt_v2_file_inline);
         return true;
     }
     else if (options->genkey && options->genkey_type == GENKEY_AUTH_TOKEN)
@@ -2633,7 +2633,8 @@ do_init_tls_wrap_key(struct context *c)
     {
         tls_crypt_init_key(&c->c1.ks.tls_wrap_key,
                            options->ce.tls_crypt_file,
-                           options->ce.tls_crypt_inline, options->tls_server);
+                           options->ce.tls_crypt_file_inline,
+			   options->tls_server);
     }
 
     /* tls-crypt with client-specific keys (--tls-crypt-v2) */
@@ -2643,14 +2644,14 @@ do_init_tls_wrap_key(struct context *c)
         {
             tls_crypt_v2_init_server_key(&c->c1.ks.tls_crypt_v2_server_key,
                                          true, options->ce.tls_crypt_v2_file,
-                                         options->ce.tls_crypt_v2_inline);
+                                         options->ce.tls_crypt_v2_file_inline);
         }
         else
         {
             tls_crypt_v2_init_client_key(&c->c1.ks.tls_wrap_key,
                                          &c->c1.ks.tls_crypt_v2_wkc,
                                          options->ce.tls_crypt_v2_file,
-                                         options->ce.tls_crypt_v2_inline);
+                                         options->ce.tls_crypt_v2_file_inline);
         }
     }
 
src/openvpn/misc.c +3 -3
index 9d739d57..b4eff3b4 100644
@@ -611,12 +611,12 @@ make_arg_copy(char **p, struct gc_arena *gc)
 }
 
 const char **
-make_extended_arg_array(char **p, struct gc_arena *gc)
+make_extended_arg_array(char **p, bool is_inline, struct gc_arena *gc)
 {
     const int argc = string_array_len((const char **)p);
-    if (!strcmp(p[0], INLINE_FILE_TAG) && argc == 2)
+    if (is_inline)
     {
-        return make_inline_array(p[1], gc);
+        return make_inline_array(p[0], gc);
     }
     else if (argc == 0)
     {
src/openvpn/misc.h +2 -1
index 2605c6d2..a3095c1f 100644
@@ -50,7 +50,8 @@ void save_inetd_socket_descriptor(void);
 
 const char **make_arg_array(const char *first, const char *parms, struct gc_arena *gc);
 
-const char **make_extended_arg_array(char **p, struct gc_arena *gc);
+const char **make_extended_arg_array(char **p, bool is_inline,
+                                     struct gc_arena *gc);
 
 /* prepend a random prefix to hostname */
 const char *hostname_randomize(const char *hostname, struct gc_arena *gc);
src/openvpn/options.c +180 -149

609 lines not on this page · raw message

src/openvpn/options.h +16 -16
index 4c1737e1..96e302bf 100644
@@ -133,17 +133,17 @@ struct connection_entry
 
     /* Shared secret used for TLS control channel authentication */
     const char *tls_auth_file;
-    const char *tls_auth_file_inline;
+    bool 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;
+    bool tls_crypt_file_inline;
 
     /* Client-specific secret or server key used for TLS control channel
      * authenticated encryption v2 */
     const char *tls_crypt_v2_file;
-    const char *tls_crypt_v2_inline;
+    bool tls_crypt_v2_file_inline;
 };
 
 struct remote_entry
@@ -480,7 +480,7 @@ struct options
     bool auth_token_call_auth;
     int auth_token_lifetime;
     const char *auth_token_secret_file;
-    const char *auth_token_secret_file_inline;
+    bool auth_token_secret_file_inline;
 
 #if PORT_SHARE
     char *port_share_host;
@@ -505,7 +505,7 @@ struct options
 
     /* Cipher parms */
     const char *shared_secret_file;
-    const char *shared_secret_file_inline;
+    bool shared_secret_file_inline;
     int key_direction;
     const char *ciphername;
     bool ncp_enabled;
@@ -529,12 +529,18 @@ struct options
     bool tls_server;
     bool tls_client;
     const char *ca_file;
+    bool ca_file_inline;
     const char *ca_path;
     const char *dh_file;
+    bool dh_file_inline;
     const char *cert_file;
+    bool cert_file_inline;
     const char *extra_certs_file;
+    bool extra_certs_file_inline;
     const char *priv_key_file;
+    bool priv_key_file_inline;
     const char *pkcs12_file;
+    bool pkcs12_file_inline;
     const char *cipher_list;
     const char *cipher_list_tls13;
     const char *tls_cert_profile;
@@ -544,14 +550,7 @@ struct options
     const char *verify_x509_name;
     const char *tls_export_cert;
     const char *crl_file;
-
-    const char *ca_file_inline;
-    const char *cert_file_inline;
-    const char *extra_certs_file_inline;
-    const char *crl_file_inline;
-    char *priv_key_file_inline;
-    const char *dh_file_inline;
-    const char *pkcs12_file_inline; /* contains the base64 encoding of pkcs12 file */
+    bool crl_file_inline;
 
     int ns_cert_type; /* set to 0, NS_CERT_CHECK_SERVER, or NS_CERT_CHECK_CLIENT */
     unsigned remote_cert_ku[MAX_PARMS];
@@ -600,16 +599,16 @@ struct options
 
     /* Shared secret used for TLS control channel authentication */
     const char *tls_auth_file;
-    const char *tls_auth_file_inline;
+    bool tls_auth_file_inline;
 
     /* Shared secret used for TLS control channel authenticated encryption */
     const char *tls_crypt_file;
-    const char *tls_crypt_inline;
+    bool tls_crypt_file_inline;
 
     /* Client-specific secret or server key used for TLS control channel
      * authenticated encryption v2 */
     const char *tls_crypt_v2_file;
-    const char *tls_crypt_v2_inline;
+    bool tls_crypt_v2_file_inline;
 
     const char *tls_crypt_v2_metadata;
 
@@ -691,6 +690,7 @@ struct options
 #define OPT_P_SOCKFLAGS       (1<<26)
 #define OPT_P_CONNECTION      (1<<27)
 #define OPT_P_PEER_ID         (1<<28)
+#define OPT_P_INLINE          (1<<29)
 
 #define OPT_P_DEFAULT   (~(OPT_P_INSTANCE|OPT_P_PULL_MODE))
 
src/openvpn/plugin.c +3 -2
index dba9d335..4de1d6b7 100644
@@ -161,12 +161,13 @@ plugin_option_list_new(struct gc_arena *gc)
 }
 
 bool
-plugin_option_list_add(struct plugin_option_list *list, char **p, struct gc_arena *gc)
+plugin_option_list_add(struct plugin_option_list *list, char **p,
+                       struct gc_arena *gc)
 {
     if (list->n < MAX_PLUGINS)
     {
         struct plugin_option *o = &list->plugins[list->n++];
-        o->argv = make_extended_arg_array(p, gc);
+        o->argv = make_extended_arg_array(p, false, gc);
         if (o->argv[0])
         {
             o->so_pathname = o->argv[0];
src/openvpn/plugin.h +2 -1
index 791d476b..bf4d71bb 100644
@@ -106,7 +106,8 @@ struct plugin_return
 
 struct plugin_option_list *plugin_option_list_new(struct gc_arena *gc);
 
-bool plugin_option_list_add(struct plugin_option_list *list, char **p, struct gc_arena *gc);
+bool plugin_option_list_add(struct plugin_option_list *list, char **p,
+                            struct gc_arena *gc);
 
 #ifndef ENABLE_SMALL
 void plugin_option_list_print(const struct plugin_option_list *list, int msglevel);
src/openvpn/push.c +1 -1
index 39a906d4..ab0cdf6a 100644
@@ -650,7 +650,7 @@ clone_push_list(struct options *o)
 void
 push_options(struct options *o, char **p, int msglevel, struct gc_arena *gc)
 {
-    const char **argv = make_extended_arg_array(p, gc);
+    const char **argv = make_extended_arg_array(p, false, gc);
     char *opt = print_argv(argv, gc, 0);
     push_option(o, opt, msglevel);
 }
src/openvpn/push.h +2 -1
index 070782dd..6cabc44d 100644
@@ -57,7 +57,8 @@ void clone_push_list(struct options *o);
 
 void push_option(struct options *o, const char *opt, int msglevel);
 
-void push_options(struct options *o, char **p, int msglevel, struct gc_arena *gc);
+void push_options(struct options *o, char **p, int msglevel,
+                  struct gc_arena *gc);
 
 void push_reset(struct options *o);
 
src/openvpn/ssl.c +2 -2
index 80e0d5ac..88b4cc7d 100644
@@ -558,7 +558,7 @@ tls_version_parse(const char *vstr, const char *extra)
  */
 static void
 tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file,
-                   const char *crl_file_inline)
+                   bool crl_file_inline)
 {
     /* if something goes wrong with stat(), we'll store 0 as mtime */
     platform_stat_t crl_stat = {0};
@@ -665,7 +665,7 @@ init_ssl(const struct options *options, struct tls_root_ctx *new_ctx)
     {
         char *cert = management_query_cert(management,
                                            options->management_certificate);
-        tls_ctx_load_cert_file(new_ctx, INLINE_FILE_TAG, cert);
+        tls_ctx_load_cert_file(new_ctx, cert, true);
         free(cert);
     }
 #endif
src/openvpn/ssl_backend.h +30 -25

117 lines not on this page · raw message

src/openvpn/ssl_common.h +1 -1
index 998ea3c4..f48990d6 100644
@@ -269,7 +269,7 @@ struct tls_options
     int verify_x509_type;
     const char *verify_x509_name;
     const char *crl_file;
-    const char *crl_file_inline;
+    bool crl_file_inline;
     int ns_cert_type;
     unsigned remote_cert_ku[MAX_PARMS];
     const char *remote_cert_eku;
src/openvpn/ssl_mbedtls.c +30 -31

158 lines not on this page · raw message

src/openvpn/ssl_openssl.c +42 -43

237 lines not on this page · raw message

src/openvpn/tls_crypt.c +4 -4
index e9f9cc2a..16424215 100644
@@ -79,7 +79,7 @@ tls_crypt_buf_overhead(void)
 
 void
 tls_crypt_init_key(struct key_ctx_bi *key, const char *key_file,
-                   const char *key_inline, bool tls_server)
+                   bool key_inline, bool tls_server)
 {
     const int key_direction = tls_server ?
                               KEY_DIRECTION_NORMAL : KEY_DIRECTION_INVERSE;
@@ -295,7 +295,7 @@ tls_crypt_v2_load_client_key(struct key_ctx_bi *key, const struct key2 *key2,
 
 void
 tls_crypt_v2_init_client_key(struct key_ctx_bi *key, struct buffer *wkc_buf,
-                             const char *key_file, const char *key_inline)
+                             const char *key_file, bool key_inline)
 {
     struct buffer client_key = alloc_buf(TLS_CRYPT_V2_CLIENT_KEY_LEN
                                          + TLS_CRYPT_V2_MAX_WKC_LEN);
@@ -320,7 +320,7 @@ tls_crypt_v2_init_client_key(struct key_ctx_bi *key, struct buffer *wkc_buf,
 
 void
 tls_crypt_v2_init_server_key(struct key_ctx *key_ctx, bool encrypt,
-                             const char *key_file, const char *key_inline)
+                             const char *key_file, bool key_inline)
 {
     struct key srv_key;
     struct buffer srv_key_buf;
@@ -638,7 +638,7 @@ void
 tls_crypt_v2_write_client_key_file(const char *filename,
                                    const char *b64_metadata,
                                    const char *server_key_file,
-                                   const char *server_key_inline)
+				   bool server_key_inline)
 {
     struct gc_arena gc = gc_new();
     struct key_ctx server_key = { 0 };
src/openvpn/tls_crypt.h +23 -20

76 lines not on this page · raw message

tests/unit_tests/openvpn/test_auth_token.c +3 -3
index 6b30dfbd..dbde8631 100644
@@ -341,7 +341,7 @@ auth_token_test_random_keys(void **state)
     ctx->multi.auth_token = strdup(random_token);
 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
-    auth_token_init_secret(&ctx->multi.opt.auth_token_key, INLINE_FILE_TAG, random_key);
+    auth_token_init_secret(&ctx->multi.opt.auth_token_key, random_key, true);
 
     /* Zero the hmac part to ensure we have a newly generated token */
     zerohmac(ctx->multi.auth_token);
@@ -361,12 +361,12 @@ auth_token_test_key_load(void **state)
     struct test_context *ctx = (struct test_context *) *state;
 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
-    auth_token_init_secret(&ctx->multi.opt.auth_token_key, INLINE_FILE_TAG, zeroinline);
+    auth_token_init_secret(&ctx->multi.opt.auth_token_key, zeroinline, true);
     strcpy(ctx->up.password, now0key0);
     assert_true(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session));
 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
-    auth_token_init_secret(&ctx->multi.opt.auth_token_key, INLINE_FILE_TAG, allx01inline);
+    auth_token_init_secret(&ctx->multi.opt.auth_token_key, allx01inline, true);
     assert_false(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session));
 }
 
tests/unit_tests/openvpn/test_tls_crypt.c +1 -2
index 8406d89d..2dc30bfe 100644
@@ -568,8 +568,7 @@ test_tls_crypt_v2_write_client_key_file(void **state)
     expect_string(__wrap_buffer_read_from_file, filename, filename);
     will_return(__wrap_buffer_read_from_file, test_client_key);
 
-    tls_crypt_v2_write_client_key_file(filename, NULL, INLINE_FILE_TAG,
-                                       test_server_key);
+    tls_crypt_v2_write_client_key_file(filename, NULL, test_server_key, true);
 }
 
 static void
Signature · 1 line
2.26.2
#7 lore raw

PATCH v11 1/1 convert *_inline attributes to bool

Carrying around the INLINE_TAG is not really efficient, because it requires a strcmp() to be performed every time we want to understand if the data is stored inline or not.

Convert all the *_inline attributes to bool to make the logic easier and checks more efficient.

---

Changes from v10:
- adapt test_tls_crypt.c to new inline'd logic for real this time
- run uncrustify-0.69.0 for real

Changes from v9:
- rebased on top of latest master
- checked with uncrustify-0.69.0
- adapt test_tls_crypt.c to new inline'd logic

Changes from v8:
- rebased on top of latest master
- changed auth-token secret file inlining logic too (not present before)
- adapted test_tls_crypt.c
- adapted test_auth_token.c

Changes from v7:
- rebased on top of latest master (68e0b9db)

Changes from v6:
- rebased on top of latest master

Changes from v5:
- fix function invocation alignment in options.c:options_postprocess_filechecks()
- fix typ0 in function invocation in options.c:options_postprocess_filechecks()
- fix doxygen comment for function tls_ctx_reload_crl() in ssl.c

Changes from v4:
- remove newline accidentally added in v4

Changes from v3:
- some code style adjustment in options.c:check_inline_file()
- move print_if_inline() from misc.c to crypto.c and rename it to print_key_filename()
- make comment of check_file_access_inline() and check_file_access_chroot_inline() doxygen compliant
- remove *is_inline argument in check_inline_file() and use its return value instead
- move declarations of is_inline to narrower scope in options.c
- move return type of plugin_option_list_add() to its own line

Changes from v2:
- fix indentation in ssl_openssl.c
- do not attempt to push inline'd options
- do not attempt to parse inline'd plugin
- introduce check_file_access_inline() and check_file_access_chroot_inline()
- introduce OPT_P_INLINE to specify when an option is allowed to be inline. Options not having this permission will fail to be parsed if is_inline is true

Changes from v1:
- remove the INLINE_TAG from the options parsing logic at all. Now a boolean variable is passed around
- add print_if_inline() helper function (to misc.c/h) to make sure we never print the inline data, but only the INLINE tag. Such function checks also for NULL pointers
- make sure print_if_inline() is always used when printing possibly inline data
- remove the INLINE_TAG from the options parsing logic at all. Now a boolean variable is passed around
- fix alignment error in comment
- remove CHKACC_INLINE from check_file_access() logic: this function is now not invoked at all in case of inline data

 src/openvpn/auth_token.c                   |   2 +-
 src/openvpn/auth_token.h                   |   2 +-
 src/openvpn/crypto.c                       |  47 +--
 src/openvpn/crypto.h                       |  25 +-
 src/openvpn/init.c                         |   9 +-
 src/openvpn/misc.c                         |   6 +-
 src/openvpn/misc.h                         |   3 +-
 src/openvpn/options.c                      | 329 +++++++++++----------
 src/openvpn/options.h                      |  32 +-
 src/openvpn/plugin.c                       |   5 +-
 src/openvpn/plugin.h                       |   3 +-
 src/openvpn/push.c                         |   2 +-
 src/openvpn/push.h                         |   3 +-
 src/openvpn/ssl.c                          |   4 +-
 src/openvpn/ssl_backend.h                  |  55 ++--
 src/openvpn/ssl_common.h                   |   2 +-
 src/openvpn/ssl_mbedtls.c                  |  61 ++--
 src/openvpn/ssl_openssl.c                  |  85 +++---
 src/openvpn/tls_crypt.c                    |   8 +-
 src/openvpn/tls_crypt.h                    |  39 +--
 tests/unit_tests/openvpn/test_auth_token.c |   6 +-
 tests/unit_tests/openvpn/test_tls_crypt.c  |   7 +-
 22 files changed, 402 insertions(+), 333 deletions(-)
src/openvpn/auth_token.c +1 -1
index 6275299d..d9212ee7 100644
@@ -135,7 +135,7 @@ auth_token_write_server_key_file(const char *filename)
 
 void
 auth_token_init_secret(struct key_ctx *key_ctx, const char *key_file,
-                       const char *key_inline)
+                       bool key_inline)
 {
     struct key_type kt = auth_token_kt();
 
src/openvpn/auth_token.h +1 -1
index 4b014d44..6f34b760 100644
@@ -74,7 +74,7 @@ verify_auth_token(struct user_pass *up, struct tls_multi *multi,
  */
 void
 auth_token_init_secret(struct key_ctx *key_ctx, const char *key_file,
-                       const char *key_inline);
+                       bool key_inline);
 
 
 /**
src/openvpn/crypto.c +30 -17

118 lines not on this page · raw message

src/openvpn/crypto.h +20 -5
index dadf0a90..999f643e 100644
@@ -444,13 +444,15 @@ generate_ephemeral_key(struct buffer *key, const char *pem_name);
  * Read key material from a PEM encoded files into the key structure
  * @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
- * @param key_inline    a string holding the data in case of an inline key
+ * @param key_file      name of the file to read or the key itself if
+ *                      key_inline is true
+ * @param key_inline    True if key_file contains an inline key, False
+ *                      otherwise.
  * @return              true if reading into key was successful
  */
 bool
 read_pem_key_file(struct buffer *key, const char *pem_name,
-                  const char *key_file, const char *key_inline);
+                  const char *key_file, bool key_inline);
 
 /* Minimum length of the nonce used by the PRNG */
 #define NONCE_SECRET_LEN_MIN 16
@@ -517,8 +519,9 @@ void key2_print(const struct key2 *k,
                 const char *prefix1);
 
 void crypto_read_openvpn_key(const struct key_type *key_type,
-                             struct key_ctx_bi *ctx, const char *key_file, const char *key_inline,
-                             const int key_direction, const char *key_name, const char *opt_name);
+                             struct key_ctx_bi *ctx, const char *key_file,
+                             bool key_inline, const int key_direction,
+                             const char *key_name, const char *opt_name);
 
 /*
  * Inline functions
@@ -536,4 +539,16 @@ key_ctx_bi_defined(const struct key_ctx_bi *key)
     return key->encrypt.cipher || key->encrypt.hmac || key->decrypt.cipher || key->decrypt.hmac;
 }
 
+/**
+ * To be used when printing a string that may contain inline data.
+ *
+ * If "is_inline" is true, return the inline tag.
+ * If "is_inline" is false and "str" is not NULL, return "str".
+ * Return the constant string "[NULL]" otherwise.
+ *
+ * @param str       the original string to return when is_inline is false
+ * @param is_inline true when str contains an inline data of some sort
+ */
+const char *print_key_filename(const char *str, bool is_inline);
+
 #endif /* CRYPTO_H */
src/openvpn/init.c +5 -4
index b3096dc8..10aad9f4 100644
@@ -1108,7 +1108,7 @@ do_genkey(const struct options *options)
 
         tls_crypt_v2_write_client_key_file(options->genkey_filename,
                                            options->genkey_extra_data, options->tls_crypt_v2_file,
-                                           options->tls_crypt_v2_inline);
+                                           options->tls_crypt_v2_file_inline);
         return true;
     }
     else if (options->genkey && options->genkey_type == GENKEY_AUTH_TOKEN)
@@ -2633,7 +2633,8 @@ do_init_tls_wrap_key(struct context *c)
     {
         tls_crypt_init_key(&c->c1.ks.tls_wrap_key,
                            options->ce.tls_crypt_file,
-                           options->ce.tls_crypt_inline, options->tls_server);
+                           options->ce.tls_crypt_file_inline,
+                           options->tls_server);
     }
 
     /* tls-crypt with client-specific keys (--tls-crypt-v2) */
@@ -2643,14 +2644,14 @@ do_init_tls_wrap_key(struct context *c)
         {
             tls_crypt_v2_init_server_key(&c->c1.ks.tls_crypt_v2_server_key,
                                          true, options->ce.tls_crypt_v2_file,
-                                         options->ce.tls_crypt_v2_inline);
+                                         options->ce.tls_crypt_v2_file_inline);
         }
         else
         {
             tls_crypt_v2_init_client_key(&c->c1.ks.tls_wrap_key,
                                          &c->c1.ks.tls_crypt_v2_wkc,
                                          options->ce.tls_crypt_v2_file,
-                                         options->ce.tls_crypt_v2_inline);
+                                         options->ce.tls_crypt_v2_file_inline);
         }
     }
 
src/openvpn/misc.c +3 -3
index 9d739d57..b4eff3b4 100644
@@ -611,12 +611,12 @@ make_arg_copy(char **p, struct gc_arena *gc)
 }
 
 const char **
-make_extended_arg_array(char **p, struct gc_arena *gc)
+make_extended_arg_array(char **p, bool is_inline, struct gc_arena *gc)
 {
     const int argc = string_array_len((const char **)p);
-    if (!strcmp(p[0], INLINE_FILE_TAG) && argc == 2)
+    if (is_inline)
     {
-        return make_inline_array(p[1], gc);
+        return make_inline_array(p[0], gc);
     }
     else if (argc == 0)
     {
src/openvpn/misc.h +2 -1
index 2605c6d2..a3095c1f 100644
@@ -50,7 +50,8 @@ void save_inetd_socket_descriptor(void);
 
 const char **make_arg_array(const char *first, const char *parms, struct gc_arena *gc);
 
-const char **make_extended_arg_array(char **p, struct gc_arena *gc);
+const char **make_extended_arg_array(char **p, bool is_inline,
+                                     struct gc_arena *gc);
 
 /* prepend a random prefix to hostname */
 const char *hostname_randomize(const char *hostname, struct gc_arena *gc);
src/openvpn/options.c +180 -149

609 lines not on this page · raw message

src/openvpn/options.h +16 -16
index 4c1737e1..96e302bf 100644
@@ -133,17 +133,17 @@ struct connection_entry
 
     /* Shared secret used for TLS control channel authentication */
     const char *tls_auth_file;
-    const char *tls_auth_file_inline;
+    bool 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;
+    bool tls_crypt_file_inline;
 
     /* Client-specific secret or server key used for TLS control channel
      * authenticated encryption v2 */
     const char *tls_crypt_v2_file;
-    const char *tls_crypt_v2_inline;
+    bool tls_crypt_v2_file_inline;
 };
 
 struct remote_entry
@@ -480,7 +480,7 @@ struct options
     bool auth_token_call_auth;
     int auth_token_lifetime;
     const char *auth_token_secret_file;
-    const char *auth_token_secret_file_inline;
+    bool auth_token_secret_file_inline;
 
 #if PORT_SHARE
     char *port_share_host;
@@ -505,7 +505,7 @@ struct options
 
     /* Cipher parms */
     const char *shared_secret_file;
-    const char *shared_secret_file_inline;
+    bool shared_secret_file_inline;
     int key_direction;
     const char *ciphername;
     bool ncp_enabled;
@@ -529,12 +529,18 @@ struct options
     bool tls_server;
     bool tls_client;
     const char *ca_file;
+    bool ca_file_inline;
     const char *ca_path;
     const char *dh_file;
+    bool dh_file_inline;
     const char *cert_file;
+    bool cert_file_inline;
     const char *extra_certs_file;
+    bool extra_certs_file_inline;
     const char *priv_key_file;
+    bool priv_key_file_inline;
     const char *pkcs12_file;
+    bool pkcs12_file_inline;
     const char *cipher_list;
     const char *cipher_list_tls13;
     const char *tls_cert_profile;
@@ -544,14 +550,7 @@ struct options
     const char *verify_x509_name;
     const char *tls_export_cert;
     const char *crl_file;
-
-    const char *ca_file_inline;
-    const char *cert_file_inline;
-    const char *extra_certs_file_inline;
-    const char *crl_file_inline;
-    char *priv_key_file_inline;
-    const char *dh_file_inline;
-    const char *pkcs12_file_inline; /* contains the base64 encoding of pkcs12 file */
+    bool crl_file_inline;
 
     int ns_cert_type; /* set to 0, NS_CERT_CHECK_SERVER, or NS_CERT_CHECK_CLIENT */
     unsigned remote_cert_ku[MAX_PARMS];
@@ -600,16 +599,16 @@ struct options
 
     /* Shared secret used for TLS control channel authentication */
     const char *tls_auth_file;
-    const char *tls_auth_file_inline;
+    bool tls_auth_file_inline;
 
     /* Shared secret used for TLS control channel authenticated encryption */
     const char *tls_crypt_file;
-    const char *tls_crypt_inline;
+    bool tls_crypt_file_inline;
 
     /* Client-specific secret or server key used for TLS control channel
      * authenticated encryption v2 */
     const char *tls_crypt_v2_file;
-    const char *tls_crypt_v2_inline;
+    bool tls_crypt_v2_file_inline;
 
     const char *tls_crypt_v2_metadata;
 
@@ -691,6 +690,7 @@ struct options
 #define OPT_P_SOCKFLAGS       (1<<26)
 #define OPT_P_CONNECTION      (1<<27)
 #define OPT_P_PEER_ID         (1<<28)
+#define OPT_P_INLINE          (1<<29)
 
 #define OPT_P_DEFAULT   (~(OPT_P_INSTANCE|OPT_P_PULL_MODE))
 
src/openvpn/plugin.c +3 -2
index dba9d335..4de1d6b7 100644
@@ -161,12 +161,13 @@ plugin_option_list_new(struct gc_arena *gc)
 }
 
 bool
-plugin_option_list_add(struct plugin_option_list *list, char **p, struct gc_arena *gc)
+plugin_option_list_add(struct plugin_option_list *list, char **p,
+                       struct gc_arena *gc)
 {
     if (list->n < MAX_PLUGINS)
     {
         struct plugin_option *o = &list->plugins[list->n++];
-        o->argv = make_extended_arg_array(p, gc);
+        o->argv = make_extended_arg_array(p, false, gc);
         if (o->argv[0])
         {
             o->so_pathname = o->argv[0];
src/openvpn/plugin.h +2 -1
index 791d476b..bf4d71bb 100644
@@ -106,7 +106,8 @@ struct plugin_return
 
 struct plugin_option_list *plugin_option_list_new(struct gc_arena *gc);
 
-bool plugin_option_list_add(struct plugin_option_list *list, char **p, struct gc_arena *gc);
+bool plugin_option_list_add(struct plugin_option_list *list, char **p,
+                            struct gc_arena *gc);
 
 #ifndef ENABLE_SMALL
 void plugin_option_list_print(const struct plugin_option_list *list, int msglevel);
src/openvpn/push.c +1 -1
index 39a906d4..ab0cdf6a 100644
@@ -650,7 +650,7 @@ clone_push_list(struct options *o)
 void
 push_options(struct options *o, char **p, int msglevel, struct gc_arena *gc)
 {
-    const char **argv = make_extended_arg_array(p, gc);
+    const char **argv = make_extended_arg_array(p, false, gc);
     char *opt = print_argv(argv, gc, 0);
     push_option(o, opt, msglevel);
 }
src/openvpn/push.h +2 -1
index 070782dd..6cabc44d 100644
@@ -57,7 +57,8 @@ void clone_push_list(struct options *o);
 
 void push_option(struct options *o, const char *opt, int msglevel);
 
-void push_options(struct options *o, char **p, int msglevel, struct gc_arena *gc);
+void push_options(struct options *o, char **p, int msglevel,
+                  struct gc_arena *gc);
 
 void push_reset(struct options *o);
 
src/openvpn/ssl.c +2 -2
index 80e0d5ac..88b4cc7d 100644
@@ -558,7 +558,7 @@ tls_version_parse(const char *vstr, const char *extra)
  */
 static void
 tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file,
-                   const char *crl_file_inline)
+                   bool crl_file_inline)
 {
     /* if something goes wrong with stat(), we'll store 0 as mtime */
     platform_stat_t crl_stat = {0};
@@ -665,7 +665,7 @@ init_ssl(const struct options *options, struct tls_root_ctx *new_ctx)
     {
         char *cert = management_query_cert(management,
                                            options->management_certificate);
-        tls_ctx_load_cert_file(new_ctx, INLINE_FILE_TAG, cert);
+        tls_ctx_load_cert_file(new_ctx, cert, true);
         free(cert);
     }
 #endif
src/openvpn/ssl_backend.h +30 -25

117 lines not on this page · raw message

src/openvpn/ssl_common.h +1 -1
index 998ea3c4..f48990d6 100644
@@ -269,7 +269,7 @@ struct tls_options
     int verify_x509_type;
     const char *verify_x509_name;
     const char *crl_file;
-    const char *crl_file_inline;
+    bool crl_file_inline;
     int ns_cert_type;
     unsigned remote_cert_ku[MAX_PARMS];
     const char *remote_cert_eku;
src/openvpn/ssl_mbedtls.c +30 -31

158 lines not on this page · raw message

src/openvpn/ssl_openssl.c +42 -43

237 lines not on this page · raw message

src/openvpn/tls_crypt.c +4 -4
index e9f9cc2a..484d4d46 100644
@@ -79,7 +79,7 @@ tls_crypt_buf_overhead(void)
 
 void
 tls_crypt_init_key(struct key_ctx_bi *key, const char *key_file,
-                   const char *key_inline, bool tls_server)
+                   bool key_inline, bool tls_server)
 {
     const int key_direction = tls_server ?
                               KEY_DIRECTION_NORMAL : KEY_DIRECTION_INVERSE;
@@ -295,7 +295,7 @@ tls_crypt_v2_load_client_key(struct key_ctx_bi *key, const struct key2 *key2,
 
 void
 tls_crypt_v2_init_client_key(struct key_ctx_bi *key, struct buffer *wkc_buf,
-                             const char *key_file, const char *key_inline)
+                             const char *key_file, bool key_inline)
 {
     struct buffer client_key = alloc_buf(TLS_CRYPT_V2_CLIENT_KEY_LEN
                                          + TLS_CRYPT_V2_MAX_WKC_LEN);
@@ -320,7 +320,7 @@ tls_crypt_v2_init_client_key(struct key_ctx_bi *key, struct buffer *wkc_buf,
 
 void
 tls_crypt_v2_init_server_key(struct key_ctx *key_ctx, bool encrypt,
-                             const char *key_file, const char *key_inline)
+                             const char *key_file, bool key_inline)
 {
     struct key srv_key;
     struct buffer srv_key_buf;
@@ -638,7 +638,7 @@ void
 tls_crypt_v2_write_client_key_file(const char *filename,
                                    const char *b64_metadata,
                                    const char *server_key_file,
-                                   const char *server_key_inline)
+                                   bool server_key_inline)
 {
     struct gc_arena gc = gc_new();
     struct key_ctx server_key = { 0 };
src/openvpn/tls_crypt.h +21 -18
index 43bf8122..2e127f2f 100644
@@ -108,14 +108,14 @@
  * Initialize a key_ctx_bi structure for use with --tls-crypt.
  *
  * @param key           The key context to initialize
- * @param key_file      The file to read the key from (or the inline tag to
- *                      indicate and inline key).
- * @param key_inline    Array containing (zero-terminated) inline key, or NULL
- *                      if not used.
+ * @param key_file      The file to read the key from or the key itself if
+ *                      key_inline is true.
+ * @param key_inline    True if key_file contains an inline key, False
+ *                      otherwise.
  * @param tls_server    Must be set to true is this is a TLS server instance.
  */
 void tls_crypt_init_key(struct key_ctx_bi *key, const char *key_file,
-                        const char *key_inline, bool tls_server);
+                        bool key_inline, bool tls_server);
 
 /**
  * Returns the maximum overhead (in bytes) added to the destination buffer by
@@ -162,11 +162,14 @@ bool tls_crypt_unwrap(const struct buffer *src, struct buffer *dst,
  * @param key           Key structure to be initialized.  Must be non-NULL.
  * @parem encrypt       If true, initialize the key structure for encryption,
  *                      otherwise for decryption.
- * @param key_file      File path of the key file to load, or INLINE tag.
- * @param key_inline    Inline key file contents (or NULL if not inline).
+ * @param key_file      File path of the key file to load or the key itself if
+ *                      key_inline is true.
+ * @param key_inline    True if key_file contains an inline key, False
+ *                      otherwise.
+ *
  */
 void tls_crypt_v2_init_server_key(struct key_ctx *key_ctx, bool encrypt,
-                                  const char *key_file, const char *key_inline);
+                                  const char *key_file, bool key_inline);
 
 /**
  * Initialize a tls-crypt-v2 client key.
@@ -176,13 +179,14 @@ void tls_crypt_v2_init_server_key(struct key_ctx *key_ctx, bool encrypt,
  * @param wrapped_key_buf   Returns buffer containing the wrapped key that will
  *                          be sent to the server when connecting.  Caller must
  *                          free this buffer when no longer needed.
- * @param key_file          File path of the key file to load, or INLINE tag.
- * @param key_inline        Inline key file contents (or NULL if not inline).
+ * @param key_file          File path of the key file to load or the key itself
+ *                          if key_inline is true.
+ * @param key_inline        True if key_file contains an inline key, False
+ *                          otherwise.
  */
 void tls_crypt_v2_init_client_key(struct key_ctx_bi *key,
                                   struct buffer *wrapped_key_buf,
-                                  const char *key_file,
-                                  const char *key_inline);
+                                  const char *key_file, bool key_inline);
 
 /**
  * Extract a tls-crypt-v2 client key from a P_CONTROL_HARD_RESET_CLIENT_V3
@@ -210,15 +214,14 @@ void tls_crypt_v2_write_server_key_file(const char *filename);
  *
  * @param filename          Filename of the client key file to create.
  * @param b64_metadata      Base64 metadata to be included in the client key.
- * @param server_key_file   File path of the server key to use for wrapping the
- *                          client key, or INLINE tag.
- * @param server_key_inline Inline server key file contents (or NULL if not
- *                          inline).
+ * @param key_file          File path of the server key to use for wrapping the
+ *                          client key or the key itself if key_inline is true.
+ * @param key_inline        True if key_file contains an inline key, False
+ *                          otherwise.
  */
 void tls_crypt_v2_write_client_key_file(const char *filename,
                                         const char *b64_metadata,
-                                        const char *key_file,
-                                        const char *key_inline);
+                                        const char *key_file, bool key_inline);
 
 /** @} */
 
tests/unit_tests/openvpn/test_auth_token.c +3 -3
index 6b30dfbd..dbde8631 100644
@@ -341,7 +341,7 @@ auth_token_test_random_keys(void **state)
     ctx->multi.auth_token = strdup(random_token);
 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
-    auth_token_init_secret(&ctx->multi.opt.auth_token_key, INLINE_FILE_TAG, random_key);
+    auth_token_init_secret(&ctx->multi.opt.auth_token_key, random_key, true);
 
     /* Zero the hmac part to ensure we have a newly generated token */
     zerohmac(ctx->multi.auth_token);
@@ -361,12 +361,12 @@ auth_token_test_key_load(void **state)
     struct test_context *ctx = (struct test_context *) *state;
 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
-    auth_token_init_secret(&ctx->multi.opt.auth_token_key, INLINE_FILE_TAG, zeroinline);
+    auth_token_init_secret(&ctx->multi.opt.auth_token_key, zeroinline, true);
     strcpy(ctx->up.password, now0key0);
     assert_true(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session));
 
     free_key_ctx(&ctx->multi.opt.auth_token_key);
-    auth_token_init_secret(&ctx->multi.opt.auth_token_key, INLINE_FILE_TAG, allx01inline);
+    auth_token_init_secret(&ctx->multi.opt.auth_token_key, allx01inline, true);
     assert_false(verify_auth_token(&ctx->up, &ctx->multi, &ctx->session));
 }
 
tests/unit_tests/openvpn/test_tls_crypt.c +3 -4
index 8406d89d..2a146f84 100644
@@ -568,8 +568,7 @@ test_tls_crypt_v2_write_client_key_file(void **state)
     expect_string(__wrap_buffer_read_from_file, filename, filename);
     will_return(__wrap_buffer_read_from_file, test_client_key);
 
-    tls_crypt_v2_write_client_key_file(filename, NULL, INLINE_FILE_TAG,
-                                       test_server_key);
+    tls_crypt_v2_write_client_key_file(filename, NULL, test_server_key, true);
 }
 
 static void
@@ -587,8 +586,8 @@ test_tls_crypt_v2_write_client_key_file_metadata(void **state)
     expect_string(__wrap_buffer_read_from_file, filename, filename);
     will_return(__wrap_buffer_read_from_file, test_client_key_metadata);
 
-    tls_crypt_v2_write_client_key_file(filename, b64metadata, INLINE_FILE_TAG,
-                                       test_server_key);
+    tls_crypt_v2_write_client_key_file(filename, b64metadata, test_server_key,
+                                       true);
 }
 
 int
Signature · 1 line
2.26.2
#8 lore raw

I have diffed v9 and v11, and the only change is the unit test related stuff. Unit tests pass with this version, so I assume it's good :-)

I take the ACK from Arne for v9 (because the main code change is the same).

Client-tested on Linux and FreeBSD, though only with a very simple inline config (key/ca/cert only) :-) (trusting Arne's tests here).

Your patch has been applied to the master branch.

As a side note: this will be a major source of conflicts for everything related to option.c being backported to 2.4... so "shortly before 2.5" is actually a good point to merge this. To merge it to 2.4 is no-go, way too intrusive and really "cleanup and refactoring", not bugfix / long-term compat stuff.

commit cb2e9218f2bc73fa22da1f6e850e5fe97f689730

Author: Antonio Quartulli
Date:   Thu May 7 15:59:09 2020 +0200
     convert *_inline attributes to bool

     Signed-off-by: Antonio Quartulli <[email protected]...>
     Acked-by: Arne Schwabe <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg19854.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering