[Openvpn-devel,09/25] dco: configure keys in DCO right after generating them

Message ID 20220624083809.23487-10-a@unstable.cc
State Changes Requested
Headers show
Series ovpn-dco: introduce data-channel offload support | expand

Commit Message

Antonio Quartulli June 24, 2022, 8:37 a.m. UTC
The ovpn-dco kernel module needs to be informed about the keys to be
used to encrypt/decrypt data traffic to/from a peer.

Configure keys in DCO right afte they are generated by the SSL code, to
avoid keeping them in memory longer than needed.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
---
 src/openvpn/dco.c   | 57 ++++++++++++++++++++++++++++++++++
 src/openvpn/dco.h   | 27 +++++++++++++++++
 src/openvpn/init.c  | 11 ++++---
 src/openvpn/multi.c |  2 +-
 src/openvpn/ssl.c   | 74 +++++++++++++++++++++++++++++++++------------
 src/openvpn/ssl.h   |  7 +++--
 6 files changed, 152 insertions(+), 26 deletions(-)

Comments

Arne Schwabe June 27, 2022, 12:42 p.m. UTC | #1
Am 24.06.22 um 10:37 schrieb Antonio Quartulli:
>   
> +/**
> + * Install the key material in DCO for the specified peer, at the specified slot
> + *
> + * @param multi     the TLS context of the current instance
> + * @param ks        the state of the key being installed
> + * @param key2      the container for the raw key material
> + * @param key_direction the key direction to be used to extract the material
> + * @param ciphername    the name of the cipher to use the key with
> + * @param server    whether we are running on a server instance or not
> + *
> + * @return          0 on success or a negative error code otherwise
> + */
> +int init_key_dco_bi(struct tls_multi *multi, struct key_state *ks,
> +                    const struct key2 *key2, int key_direction,
> +                    const char *ciphername, bool server);

I think here the description might be outdated. Your method does not 
have a specified slot anymore. It would be good to document that this
method instead has a hidden that it install the primary key on the first 
call and otherwards installs/overwrites the secondary key.

> +    if (dco_disabled)
> +    {
> +        init_key_ctx_bi(key, key2, key_direction, key_type, "Data Channel");
> +        /* Initialize implicit IVs */
> +        key_ctx_update_implicit_iv(&key->encrypt, key2->keys[(int)server].hmac,
> +                                   MAX_HMAC_KEY_LENGTH);
> +        key_ctx_update_implicit_iv(&key->decrypt,
> +                                   key2->keys[1 - (int)server].hmac,
> +                                   MAX_HMAC_KEY_LENGTH);
> +    }
> +
> +    if (!dco_disabled)
> +    {

I think an else here would be better.

> +        /* encrypt/decrypt context are unused with DCO */

Do we have actually checked this? IIrc the generation of the explicit 
exit notification might still try to generate a data channel key and use 
these contexts.

Arne

Patch

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 1e45130a..e38614fa 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -33,7 +33,64 @@ 
 #if defined(ENABLE_DCO)
 
 #include "syshead.h"
+#include "crypto.h"
 #include "dco.h"
+#include "errlevel.h"
+#include "openvpn.h"
+#include "ssl_common.h"
+#include "ssl_ncp.h"
+#include "tun.h"
+
+static int
+dco_install_key(struct tls_multi *multi, struct key_state *ks,
+                const uint8_t *encrypt_key, const uint8_t *encrypt_iv,
+                const uint8_t *decrypt_key, const uint8_t *decrypt_iv,
+                const char *ciphername)
+
+{
+    msg(D_DCO_DEBUG, "%s: peer_id=%d keyid=%d", __func__, multi->peer_id,
+        ks->key_id);
+
+    /* Install a key in the PRIMARY slot only when no other key exist.
+     * From that moment on, any new key will be installed in the SECONDARY
+     * slot and will be promoted to PRIMARY when userspace says so (a swap
+     * will be performed in that case)
+     */
+    dco_key_slot_t slot = OVPN_KEY_SLOT_PRIMARY;
+    if (multi->dco_keys_installed > 0)
+    {
+        slot = OVPN_KEY_SLOT_SECONDARY;
+    }
+
+    int ret = dco_new_key(multi->dco, multi->peer_id, ks->key_id, slot,
+                          encrypt_key, encrypt_iv,
+                          decrypt_key, decrypt_iv,
+                          ciphername);
+    if ((ret == 0) && (multi->dco_keys_installed < 2))
+    {
+        multi->dco_keys_installed++;
+        ks->dco_status = (slot == OVPN_KEY_SLOT_PRIMARY) ? DCO_INSTALLED_PRIMARY :
+                         DCO_INSTALLED_SECONDARY;
+    }
+
+    return ret;
+}
+
+int
+init_key_dco_bi(struct tls_multi *multi, struct key_state *ks,
+                const struct key2 *key2, int key_direction,
+                const char *ciphername, bool server)
+{
+    struct key_direction_state kds;
+    key_direction_state_init(&kds, key_direction);
+
+    return dco_install_key(multi, ks,
+                           key2->keys[kds.out_key].cipher,
+                           key2->keys[(int)server].hmac,
+                           key2->keys[kds.in_key].cipher,
+                           key2->keys[1 - (int)server].hmac,
+                           ciphername);
+}
 
 static bool
 dco_check_option_conflict_ce(const struct connection_entry *ce, int msglevel)
diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index 063e5028..b081c6fa 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -35,7 +35,10 @@ 
  * order problems)
  */
 struct event_set;
+struct key2;
+struct key_state;
 struct options;
+struct tls_multi;
 struct tuntap;
 
 #define DCO_DEFAULT_METRIC  200
@@ -111,6 +114,22 @@  int dco_do_write(dco_context_t *dco, int peer_id, struct buffer *buf);
  */
 void dco_event_set(dco_context_t *dco, struct event_set *es, void *arg);
 
+/**
+ * Install the key material in DCO for the specified peer, at the specified slot
+ *
+ * @param multi     the TLS context of the current instance
+ * @param ks        the state of the key being installed
+ * @param key2      the container for the raw key material
+ * @param key_direction the key direction to be used to extract the material
+ * @param ciphername    the name of the cipher to use the key with
+ * @param server    whether we are running on a server instance or not
+ *
+ * @return          0 on success or a negative error code otherwise
+ */
+int init_key_dco_bi(struct tls_multi *multi, struct key_state *ks,
+                    const struct key2 *key2, int key_direction,
+                    const char *ciphername, bool server);
+
 #else /* if defined(ENABLE_DCO) */
 
 typedef void *dco_context_t;
@@ -163,5 +182,13 @@  dco_event_set(dco_context_t *dco, struct event_set *es, void *arg)
 {
 }
 
+static inline int
+init_key_dco_bi(struct tls_multi *multi, struct key_state *ks,
+                const struct key2 *key2, int key_direction,
+                const char *ciphername, bool server)
+{
+    return 0;
+}
+
 #endif /* defined(ENABLE_DCO) */
 #endif /* ifndef DCO_H */
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 7ab2c9a2..06911cd0 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2201,8 +2201,9 @@  do_deferred_p2p_ncp(struct context *c)
     }
 #endif
 
-    if (!tls_session_update_crypto_params(session, &c->options, &c->c2.frame,
-                                          frame_fragment, get_link_socket_info(c)))
+    if (!tls_session_update_crypto_params(c->c2.tls_multi, session, &c->options,
+                                          &c->c2.frame, frame_fragment,
+                                          get_link_socket_info(c)))
     {
         msg(D_TLS_ERRORS, "ERROR: failed to set crypto cipher");
         return false;
@@ -2315,8 +2316,10 @@  do_deferred_options(struct context *c, const unsigned int found)
 #endif
 
         struct tls_session *session = &c->c2.tls_multi->session[TM_ACTIVE];
-        if (!tls_session_update_crypto_params(session, &c->options, &c->c2.frame,
-                                              frame_fragment, get_link_socket_info(c)))
+        if (!tls_session_update_crypto_params(c->c2.tls_multi, session,
+                                              &c->options, &c->c2.frame,
+                                              frame_fragment,
+                                              get_link_socket_info(c)))
         {
             msg(D_TLS_ERRORS, "OPTIONS ERROR: failed to import crypto options");
             return false;
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index ba2f6d58..c72575ae 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2286,7 +2286,7 @@  multi_client_generate_tls_keys(struct context *c)
     }
 #endif
     struct tls_session *session = &c->c2.tls_multi->session[TM_ACTIVE];
-    if (!tls_session_update_crypto_params(session, &c->options,
+    if (!tls_session_update_crypto_params(c->c2.tls_multi, session, &c->options,
                                           &c->c2.frame, frame_fragment,
                                           get_link_socket_info(c)))
     {
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 61dea996..9b16d6a3 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -63,6 +63,7 @@ 
 #include "ssl_util.h"
 #include "auth_token.h"
 #include "mss.h"
+#include "dco.h"
 
 #include "memdbg.h"
 
@@ -1429,21 +1430,49 @@  openvpn_PRF(const uint8_t *secret,
 }
 
 static void
-init_key_contexts(struct key_ctx_bi *key,
+init_key_contexts(struct key_state *ks,
+                  struct tls_multi *multi,
                   const struct key_type *key_type,
                   bool server,
-                  struct key2 *key2)
+                  struct key2 *key2,
+                  bool dco_disabled)
 {
+    struct key_ctx_bi *key = &ks->crypto_options.key_ctx_bi;
+
     /* Initialize key contexts */
     int key_direction = server ? KEY_DIRECTION_INVERSE : KEY_DIRECTION_NORMAL;
-    init_key_ctx_bi(key, key2, key_direction, key_type, "Data Channel");
 
-    /* Initialize implicit IVs */
-    key_ctx_update_implicit_iv(&key->encrypt, key2->keys[(int)server].hmac,
-                               MAX_HMAC_KEY_LENGTH);
-    key_ctx_update_implicit_iv(&key->decrypt, key2->keys[1 - (int)server].hmac,
-                               MAX_HMAC_KEY_LENGTH);
+    if (dco_disabled)
+    {
+        init_key_ctx_bi(key, key2, key_direction, key_type, "Data Channel");
+        /* Initialize implicit IVs */
+        key_ctx_update_implicit_iv(&key->encrypt, key2->keys[(int)server].hmac,
+                                   MAX_HMAC_KEY_LENGTH);
+        key_ctx_update_implicit_iv(&key->decrypt,
+                                   key2->keys[1 - (int)server].hmac,
+                                   MAX_HMAC_KEY_LENGTH);
+    }
+
+    if (!dco_disabled)
+    {
+        if (key->encrypt.hmac)
+        {
+            msg(M_FATAL, "FATAL: DCO does not support --auth");
+        }
+
+        int ret = init_key_dco_bi(multi, ks, key2, key_direction,
+                                  key_type->cipher, server);
+        if (ret < 0)
+        {
+            msg(M_FATAL, "Impossible to install key material in DCO: %s",
+                strerror(-ret));
+        }
 
+        /* encrypt/decrypt context are unused with DCO */
+        CLEAR(key->encrypt);
+        CLEAR(key->decrypt);
+        key->initialized = true;
+    }
 }
 
 static bool
@@ -1519,9 +1548,10 @@  generate_key_expansion_openvpn_prf(const struct tls_session *session, struct key
  * master key.
  */
 static bool
-generate_key_expansion(struct key_ctx_bi *key,
+generate_key_expansion(struct tls_multi *multi, struct key_state *ks,
                        struct tls_session *session)
 {
+    struct key_ctx_bi *key = &ks->crypto_options.key_ctx_bi;
     bool ret = false;
     struct key2 key2;
 
@@ -1562,7 +1592,9 @@  generate_key_expansion(struct key_ctx_bi *key,
             goto exit;
         }
     }
-    init_key_contexts(key, &session->opt->key_type, server, &key2);
+
+    init_key_contexts(ks, multi, &session->opt->key_type, server, &key2,
+                      session->opt->disable_dco);
     ret = true;
 
 exit:
@@ -1594,7 +1626,8 @@  key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t *key, size_t key_len)
  * can thus be called only once per session.
  */
 bool
-tls_session_generate_data_channel_keys(struct tls_session *session)
+tls_session_generate_data_channel_keys(struct tls_multi *multi,
+                                       struct tls_session *session)
 {
     bool ret = false;
     struct key_state *ks = &session->key[KS_PRIMARY];   /* primary key */
@@ -1607,7 +1640,7 @@  tls_session_generate_data_channel_keys(struct tls_session *session)
 
     ks->crypto_options.flags = session->opt->crypto_flags;
 
-    if (!generate_key_expansion(&ks->crypto_options.key_ctx_bi, session))
+    if (!generate_key_expansion(multi, ks, session))
     {
         msg(D_TLS_ERRORS, "TLS Error: generate_key_expansion failed");
         goto cleanup;
@@ -1625,8 +1658,10 @@  cleanup:
 }
 
 bool
-tls_session_update_crypto_params_do_work(struct tls_session *session,
-                                         struct options *options, struct frame *frame,
+tls_session_update_crypto_params_do_work(struct tls_multi *multi,
+                                         struct tls_session *session,
+                                         struct options *options,
+                                         struct frame *frame,
                                          struct frame *frame_fragment,
                                          struct link_socket_info *lsi)
 {
@@ -1669,11 +1704,12 @@  tls_session_update_crypto_params_do_work(struct tls_session *session,
         frame_print(frame_fragment, D_MTU_INFO, "Fragmentation MTU parms");
     }
 
-    return tls_session_generate_data_channel_keys(session);
+    return tls_session_generate_data_channel_keys(multi, session);
 }
 
 bool
-tls_session_update_crypto_params(struct tls_session *session,
+tls_session_update_crypto_params(struct tls_multi *multi,
+                                 struct tls_session *session,
                                  struct options *options, struct frame *frame,
                                  struct frame *frame_fragment,
                                  struct link_socket_info *lsi)
@@ -1695,8 +1731,8 @@  tls_session_update_crypto_params(struct tls_session *session,
     /* Import crypto settings that might be set by pull/push */
     session->opt->crypto_flags |= options->data_channel_crypto_flags;
 
-    return tls_session_update_crypto_params_do_work(session, options, frame,
-                                                    frame_fragment, lsi);
+    return tls_session_update_crypto_params_do_work(multi, session, options,
+                                                    frame, frame_fragment, lsi);
 }
 
 
@@ -3089,7 +3125,7 @@  tls_multi_process(struct tls_multi *multi,
                 /* Session is now fully authenticated.
                 * tls_session_generate_data_channel_keys will move ks->state
                 * from S_ACTIVE to S_GENERATED_KEYS */
-                if (!tls_session_generate_data_channel_keys(session))
+                if (!tls_session_generate_data_channel_keys(multi, session))
                 {
                     msg(D_TLS_ERRORS, "TLS Error: generate_key_expansion failed");
                     ks->authenticated = KS_AUTH_FALSE;
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 0ba86d3e..ba271971 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -423,6 +423,7 @@  void tls_update_remote_addr(struct tls_multi *multi,
  * channel keys based on the supplied options. Does nothing if keys are already
  * generated.
  *
+ * @param multi           The TLS object for this instance.
  * @param session         The TLS session to update.
  * @param options         The options to use when updating session.
  * @param frame           The frame options for this session (frame overhead is
@@ -433,7 +434,8 @@  void tls_update_remote_addr(struct tls_multi *multi,
  *
  * @return true if updating succeeded or keys are already generated, false otherwise.
  */
-bool tls_session_update_crypto_params(struct tls_session *session,
+bool tls_session_update_crypto_params(struct tls_multi *multi,
+                                      struct tls_session *session,
                                       struct options *options,
                                       struct frame *frame,
                                       struct frame *frame_fragment,
@@ -548,7 +550,8 @@  show_available_tls_ciphers(const char *cipher_list,
  * can thus be called only once per session.
  */
 bool
-tls_session_generate_data_channel_keys(struct tls_session *session);
+tls_session_generate_data_channel_keys(struct tls_multi *multi,
+                                       struct tls_session *session);
 
 /**
  * Load ovpn.xkey provider used for external key signing