]> git.ipfire.org Git - thirdparty/openvpn.git/commitdiff
Remove key-method 1
authorArne Schwabe <arne@rfc2549.org>
Tue, 21 Jul 2020 10:01:28 +0000 (12:01 +0200)
committerGert Doering <gert@greenie.muc.de>
Tue, 21 Jul 2020 18:58:13 +0000 (20:58 +0200)
Key-method 1 is only needed to talk to pre OpenVPN 2.0 clients.

Patch V2: Fix style. Make V1 op codes illegal, remove all code handling
          v1 op codes and give a good warning message if we encounter
          them in the legal op codes pre-check.

Patch V3: Add a bit more comments in the existing methods.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20200721100128.9850-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20516.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
doc/doxygen/doc_control_processor.h
doc/doxygen/doc_key_generation.h
doc/doxygen/doc_protocol_overview.h
src/openvpn/forward.c
src/openvpn/helper.c
src/openvpn/init.c
src/openvpn/options.c
src/openvpn/options.h
src/openvpn/ssl.c
src/openvpn/ssl.h
src/openvpn/ssl_common.h

index f87324ccee835492ca20ffdb826d6acd6066df60..1bbf2d2dbd9a69e0226e17f2409886f38e5fabf9 100644 (file)
  *    appropriate messages to be sent.
  *
  * @par Functions which control data channel key generation
- *  - Key method 1 key exchange functions:
- *     - \c key_method_1_write(), generates and processes key material to
- *       be sent to the remote OpenVPN peer.
- *     - \c key_method_1_read(), processes key material received from the
- *       remote OpenVPN peer.
+ *  - Key method 1 key exchange functions were removed from OpenVPN 2.5
  *  - Key method 2 key exchange functions:
  *     - \c key_method_2_write(), generates and processes key material to
  *       be sent to the remote OpenVPN peer.
index efe61155d955a3755f8789e78ea06d3345404bc1..4bb9c708b5bcf64f1677766a4152f29bdd4e857f 100644 (file)
@@ -131,11 +131,7 @@ S_ACTIVE                                                            S_ACTIVE
  * control_processor Control Channel Processor module's\endlink \c
  * tls_process() function and control the %key generation and exchange
  * process as follows:
- * - %Key method 1:
- *   - \c key_method_1_write(): generate random material locally, and load
- *     as "sending" keys.
- *   - \c key_method_1_read(): read random material received from remote
- *     peer, and load as "receiving" keys.
+ * - %Key method 1 has been removed in OpenVPN 2.5
  * - %Key method 2:
  *   - \c key_method_2_write(): generate random material locally, and if
  *     in server mode generate %key expansion.
index 3f48b18a63fe112ec158a3ffdc86c30615832e0f..08212223ef60e2e250901ccd2be1f8d510ce535e 100644 (file)
  *
  * @subsection network_protocol_control_plaintext Structure of plaintext control channel messages
  *
- *  - %Key method 1:
+ *  - %Key method 1 (support removed in OpenVPN 2.5):
  *     - Cipher %key length in bytes (1 byte).
  *     - Cipher %key (n bytes).
  *     - HMAC %key length in bytes (1 byte).
index 5c4370a87d94a35c93a7d75b79b46fc5cb8a8578..698451d1288171b86ebc6532f95e7d9a59352333 100644 (file)
@@ -1100,7 +1100,7 @@ process_incoming_link_part1(struct context *c, struct link_socket_info *lsi, boo
                                 floated, &ad_start))
             {
                 /* Restore pre-NCP frame parameters */
-                if (is_hard_reset(opcode, c->options.key_method))
+                if (is_hard_reset_method2(opcode))
                 {
                     c->c2.frame = c->c2.frame_initial;
 #ifdef ENABLE_FRAGMENT
index 6e9cc63c742882a75690483cff348ed60c3c6bf5..a1d03070687cf4872d3fde32b1791dac1e374cc3 100644 (file)
@@ -490,11 +490,6 @@ helper_client_server(struct options *o)
      */
     if (o->client)
     {
-        if (o->key_method != 2)
-        {
-            msg(M_USAGE, "--client requires --key-method 2");
-        }
-
         o->pull = true;
         o->tls_client = true;
     }
index e9c016291d854e9f768083b1d5a7a21d0711c441..b96d1471a88ea4ee74a1eb46be8bc28c85c38fae 100644 (file)
@@ -2852,7 +2852,6 @@ do_init_crypto_tls(struct context *c, const unsigned int flags)
     to.ssl_ctx = c->c1.ks.ssl_ctx;
     to.key_type = c->c1.ks.key_type;
     to.server = options->tls_server;
-    to.key_method = options->key_method;
     to.replay = options->replay;
     to.replay_window = options->replay_window;
     to.replay_time = options->replay_time;
index b1962ca47c1c055b3979804c4783002906ff0568..e9100498fde18aba0011443dd11e56d392168ae1 100644 (file)
@@ -876,7 +876,6 @@ init_options(struct options *o, const bool init_gc)
 #ifdef ENABLE_PREDICTION_RESISTANCE
     o->use_prediction_resistance = false;
 #endif
-    o->key_method = 2;
     o->tls_timeout = 2;
     o->renegotiate_bytes = -1;
     o->renegotiate_seconds = 3600;
@@ -1714,7 +1713,6 @@ show_settings(const struct options *o)
 
     SHOW_BOOL(tls_server);
     SHOW_BOOL(tls_client);
-    SHOW_INT(key_method);
     SHOW_STR_INLINE(ca_file);
     SHOW_STR(ca_path);
     SHOW_STR(dh_file);
@@ -2375,10 +2373,6 @@ options_postprocess_verify_ce(const struct options *options, const struct connec
         {
             msg(M_USAGE, "--ccd-exclusive must be used with --client-config-dir");
         }
-        if (options->key_method != 2)
-        {
-            msg(M_USAGE, "--mode server requires --key-method 2");
-        }
         if (options->auth_token_generate && !options->renegotiate_seconds)
         {
             msg(M_USAGE, "--auth-gen-token needs a non-infinite "
@@ -2545,13 +2539,6 @@ options_postprocess_verify_ce(const struct options *options, const struct connec
             "may accept clients which do not present a certificate");
     }
 
-    if (options->key_method == 1)
-    {
-        msg(M_WARN, "WARNING: --key-method 1 is deprecated and will be removed "
-            "in OpenVPN 2.5.  By default --key-method 2 will be used if not set "
-            "in the configuration file, which is the recommended approach.");
-    }
-
     const int tls_version_max =
         (options->ssl_flags >> SSLF_TLS_VERSION_MAX_SHIFT)
         & SSLF_TLS_VERSION_MAX_MASK;
@@ -2793,7 +2780,6 @@ options_postprocess_verify_ce(const struct options *options, const struct connec
         MUST_BE_UNDEF(push_peer_info);
         MUST_BE_UNDEF(tls_exit);
         MUST_BE_UNDEF(crl_file);
-        MUST_BE_UNDEF(key_method);
         MUST_BE_UNDEF(ns_cert_type);
         MUST_BE_UNDEF(remote_cert_ku[0]);
         MUST_BE_UNDEF(remote_cert_eku);
@@ -3822,10 +3808,7 @@ options_string(const struct options *o,
              * tls-auth/tls-crypt does not match.  Removing tls-auth here would
              * break stuff, so leaving that in place. */
 
-            if (o->key_method > 1)
-            {
-                buf_printf(&out, ",key-method %d", o->key_method);
-            }
+            buf_printf(&out, ",key-method %d", KEY_METHOD_2);
         }
 
         if (remote)
@@ -8456,22 +8439,6 @@ add_option(struct options *options,
         VERIFY_PERMISSION(OPT_P_GENERAL);
         options->tls_crypt_v2_verify_script = p[1];
     }
-    else if (streq(p[0], "key-method") && p[1] && !p[2])
-    {
-        int key_method;
-
-        VERIFY_PERMISSION(OPT_P_GENERAL);
-        key_method = atoi(p[1]);
-        if (key_method < KEY_METHOD_MIN || key_method > KEY_METHOD_MAX)
-        {
-            msg(msglevel, "key_method parameter (%d) must be >= %d and <= %d",
-                key_method,
-                KEY_METHOD_MIN,
-                KEY_METHOD_MAX);
-            goto err;
-        }
-        options->key_method = key_method;
-    }
     else if (streq(p[0], "x509-track") && p[1] && !p[2])
     {
         VERIFY_PERMISSION(OPT_P_GENERAL);
index c37006d3c07bd921d0e42f7fbe8dd0340d1359e9..816655900a5985b8987a8cb0bf607626101204a7 100644 (file)
@@ -571,10 +571,6 @@ struct options
 #ifdef ENABLE_CRYPTOAPI
     const char *cryptoapi_cert;
 #endif
-
-    /* data channel key exchange method */
-    int key_method;
-
     /* Per-packet timeout on control channel */
     int tls_timeout;
 
index 54a2301133f187782f66865bd92636a2b1bdce0d..2ab3440e70879fb7ba92be970d9ecece630b1462 100644 (file)
@@ -855,23 +855,12 @@ print_key_id(struct tls_multi *multi, struct gc_arena *gc)
 }
 
 bool
-is_hard_reset(int op, int key_method)
+is_hard_reset_method2(int op)
 {
-    if (!key_method || key_method == 1)
+    if (op == P_CONTROL_HARD_RESET_CLIENT_V2 || op == P_CONTROL_HARD_RESET_SERVER_V2
+        || op == P_CONTROL_HARD_RESET_CLIENT_V3)
     {
-        if (op == P_CONTROL_HARD_RESET_CLIENT_V1 || op == P_CONTROL_HARD_RESET_SERVER_V1)
-        {
-            return true;
-        }
-    }
-
-    if (!key_method || key_method >= 2)
-    {
-        if (op == P_CONTROL_HARD_RESET_CLIENT_V2 || op == P_CONTROL_HARD_RESET_SERVER_V2
-            || op == P_CONTROL_HARD_RESET_CLIENT_V3)
-        {
-            return true;
-        }
+        return true;
     }
 
     return false;
@@ -1091,23 +1080,14 @@ tls_session_init(struct tls_multi *multi, struct tls_session *session)
     }
 
     /* Are we a TLS server or client? */
-    ASSERT(session->opt->key_method >= 1);
-    if (session->opt->key_method == 1)
+    if (session->opt->server)
     {
-        session->initial_opcode = session->opt->server ?
-                                  P_CONTROL_HARD_RESET_SERVER_V1 : P_CONTROL_HARD_RESET_CLIENT_V1;
+        session->initial_opcode = P_CONTROL_HARD_RESET_SERVER_V2;
     }
-    else /* session->opt->key_method >= 2 */
+    else
     {
-        if (session->opt->server)
-        {
-            session->initial_opcode = P_CONTROL_HARD_RESET_SERVER_V2;
-        }
-        else
-        {
-            session->initial_opcode = session->opt->tls_crypt_v2 ?
-                                      P_CONTROL_HARD_RESET_CLIENT_V3 : P_CONTROL_HARD_RESET_CLIENT_V2;
-        }
+        session->initial_opcode = session->opt->tls_crypt_v2 ?
+                                  P_CONTROL_HARD_RESET_CLIENT_V3 : P_CONTROL_HARD_RESET_CLIENT_V2;
     }
 
     /* Initialize control channel authentication parameters */
@@ -2219,52 +2199,6 @@ read_string_alloc(struct buffer *buf)
     return str;
 }
 
-/*
- * Handle the reading and writing of key data to and from
- * the TLS control channel (cleartext).
- */
-
-static bool
-key_method_1_write(struct buffer *buf, struct tls_session *session)
-{
-    struct key key;
-    struct key_state *ks = &session->key[KS_PRIMARY];      /* primary key */
-
-    ASSERT(session->opt->key_method == 1);
-    ASSERT(buf_init(buf, 0));
-
-    generate_key_random(&key, &session->opt->key_type);
-    if (!check_key(&key, &session->opt->key_type))
-    {
-        msg(D_TLS_ERRORS, "TLS Error: Bad encrypting key generated");
-        return false;
-    }
-
-    if (!write_key(&key, &session->opt->key_type, buf))
-    {
-        msg(D_TLS_ERRORS, "TLS Error: write_key failed");
-        return false;
-    }
-
-    init_key_ctx(&ks->crypto_options.key_ctx_bi.encrypt, &key,
-                 &session->opt->key_type, OPENVPN_OP_ENCRYPT,
-                 "Data Channel Encrypt");
-    secure_memzero(&key, sizeof(key));
-
-    /* send local options string */
-    {
-        const char *local_options = local_options_string(session);
-        const int optlen = strlen(local_options) + 1;
-        if (!buf_write(buf, local_options, optlen))
-        {
-            msg(D_TLS_ERRORS, "TLS Error: KM1 write options failed");
-            return false;
-        }
-    }
-
-    return true;
-}
-
 static bool
 push_peer_info(struct buffer *buf, struct tls_session *session)
 {
@@ -2372,12 +2306,15 @@ error:
     return ret;
 }
 
+/**
+ * Handle the writing of key data, peer-info, username/password, OCC
+ * to the TLS control channel (cleartext).
+ */
 static bool
 key_method_2_write(struct buffer *buf, struct tls_session *session)
 {
     struct key_state *ks = &session->key[KS_PRIMARY];      /* primary key */
 
-    ASSERT(session->opt->key_method == 2);
     ASSERT(buf_init(buf, 0));
 
     /* write a uint32 0 */
@@ -2387,7 +2324,7 @@ key_method_2_write(struct buffer *buf, struct tls_session *session)
     }
 
     /* write key_method + flags */
-    if (!buf_write_u8(buf, (session->opt->key_method & KEY_METHOD_MASK)))
+    if (!buf_write_u8(buf, KEY_METHOD_2))
     {
         goto error;
     }
@@ -2489,73 +2426,15 @@ error:
     return false;
 }
 
-static bool
-key_method_1_read(struct buffer *buf, struct tls_session *session)
-{
-    int status;
-    struct key key;
-    struct key_state *ks = &session->key[KS_PRIMARY];      /* primary key */
-
-    ASSERT(session->opt->key_method == 1);
-
-    if (!session->verified)
-    {
-        msg(D_TLS_ERRORS,
-            "TLS Error: Certificate verification failed (key-method 1)");
-        goto error;
-    }
-
-    status = read_key(&key, &session->opt->key_type, buf);
-    if (status != 1)
-    {
-        msg(D_TLS_ERRORS,
-            "TLS Error: Error reading data channel key from plaintext buffer");
-        goto error;
-    }
-
-    if (!check_key(&key, &session->opt->key_type))
-    {
-        msg(D_TLS_ERRORS, "TLS Error: Bad decrypting key received from peer");
-        goto error;
-    }
-
-    if (buf->len < 1)
-    {
-        msg(D_TLS_ERRORS, "TLS Error: Missing options string");
-        goto error;
-    }
-
-#ifdef ENABLE_OCC
-    /* compare received remote options string
-     * with our locally computed options string */
-    if (!session->opt->disable_occ
-        && !options_cmp_equal_safe((char *) BPTR(buf), session->opt->remote_options, buf->len))
-    {
-        options_warning_safe((char *) BPTR(buf), session->opt->remote_options, buf->len);
-    }
-#endif
-
-    buf_clear(buf);
-
-    init_key_ctx(&ks->crypto_options.key_ctx_bi.decrypt, &key,
-                 &session->opt->key_type, OPENVPN_OP_DECRYPT,
-                 "Data Channel Decrypt");
-    secure_memzero(&key, sizeof(key));
-    ks->authenticated = KS_AUTH_TRUE;
-    return true;
-
-error:
-    buf_clear(buf);
-    secure_memzero(&key, sizeof(key));
-    return false;
-}
-
+/**
+ * Handle reading key data, peer-info, username/password, OCC
+ * from the TLS control channel (cleartext).
+ */
 static bool
 key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct tls_session *session)
 {
     struct key_state *ks = &session->key[KS_PRIMARY];      /* primary key */
 
-    int key_method_flags;
     bool username_status, password_status;
 
     struct gc_arena gc = gc_new();
@@ -2565,8 +2444,6 @@ key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct tls_sessio
     /* allocate temporary objects */
     ALLOC_ARRAY_CLEAR_GC(options, char, TLS_OPTIONS_LEN, &gc);
 
-    ASSERT(session->opt->key_method == 2);
-
     /* discard leading uint32 */
     if (!buf_advance(buf, 4))
     {
@@ -2576,7 +2453,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct tls_sessio
     }
 
     /* get key method */
-    key_method_flags = buf_read_u8(buf);
+    int key_method_flags = buf_read_u8(buf);
     if ((key_method_flags & KEY_METHOD_MASK) != 2)
     {
         msg(D_TLS_ERRORS,
@@ -2986,23 +2863,9 @@ tls_process(struct tls_multi *multi,
         if (!buf->len && ((ks->state == S_START && !session->opt->server)
                           || (ks->state == S_GOT_KEY && session->opt->server)))
         {
-            if (session->opt->key_method == 1)
-            {
-                if (!key_method_1_write(buf, session))
-                {
-                    goto error;
-                }
-            }
-            else if (session->opt->key_method == 2)
-            {
-                if (!key_method_2_write(buf, session))
-                {
-                    goto error;
-                }
-            }
-            else
+            if (!key_method_2_write(buf, session))
             {
-                ASSERT(0);
+                goto error;
             }
 
             state_change = true;
@@ -3016,23 +2879,9 @@ tls_process(struct tls_multi *multi,
             && ((ks->state == S_SENT_KEY && !session->opt->server)
                 || (ks->state == S_START && session->opt->server)))
         {
-            if (session->opt->key_method == 1)
-            {
-                if (!key_method_1_read(buf, session))
-                {
-                    goto error;
-                }
-            }
-            else if (session->opt->key_method == 2)
-            {
-                if (!key_method_2_read(buf, multi, session))
-                {
-                    goto error;
-                }
-            }
-            else
+            if (!key_method_2_read(buf, multi, session))
             {
-                ASSERT(0);
+                goto error;
             }
 
             state_change = true;
@@ -3446,6 +3295,11 @@ tls_pre_decrypt(struct tls_multi *multi,
             /* verify legal opcode */
             if (op < P_FIRST_OPCODE || op > P_LAST_OPCODE)
             {
+                if (op == P_CONTROL_HARD_RESET_CLIENT_V1
+                    || op == P_CONTROL_HARD_RESET_SERVER_V1)
+                {
+                    msg(D_TLS_ERRORS, "Peer tried unsupported key-method 1");
+                }
                 msg(D_TLS_ERRORS,
                     "TLS Error: unknown opcode received from %s op=%d",
                     print_link_socket_actual(from, &gc), op);
@@ -3453,14 +3307,12 @@ tls_pre_decrypt(struct tls_multi *multi,
             }
 
             /* hard reset ? */
-            if (is_hard_reset(op, 0))
+            if (is_hard_reset_method2(op))
             {
                 /* verify client -> server or server -> client connection */
-                if (((op == P_CONTROL_HARD_RESET_CLIENT_V1
-                      || op == P_CONTROL_HARD_RESET_CLIENT_V2
+                if (((op == P_CONTROL_HARD_RESET_CLIENT_V2
                       || op == P_CONTROL_HARD_RESET_CLIENT_V3) && !multi->opt.server)
-                    || ((op == P_CONTROL_HARD_RESET_SERVER_V1
-                         || op == P_CONTROL_HARD_RESET_SERVER_V2) && multi->opt.server))
+                    || ((op == P_CONTROL_HARD_RESET_SERVER_V2) && multi->opt.server))
                 {
                     msg(D_TLS_ERRORS,
                         "TLS Error: client->client or server->server connection attempted from %s",
@@ -3522,22 +3374,14 @@ tls_pre_decrypt(struct tls_multi *multi,
             }
 
             /*
-             * Initial packet received.
+             * Hard reset and session id does not match any session in
+             * multi->session: Possible initial packet
              */
-
-            if (i == TM_SIZE && is_hard_reset(op, 0))
+            if (i == TM_SIZE && is_hard_reset_method2(op))
             {
                 struct tls_session *session = &multi->session[TM_ACTIVE];
                 struct key_state *ks = &session->key[KS_PRIMARY];
 
-                if (!is_hard_reset(op, multi->opt.key_method))
-                {
-                    msg(D_TLS_ERRORS, "TLS ERROR: initial packet local/remote key_method mismatch, local key_method=%d, op=%s",
-                        multi->opt.key_method,
-                        packet_opcode_name(op));
-                    goto error;
-                }
-
                 /*
                  * If we have no session currently in progress, the initial packet will
                  * open a new session in TM_ACTIVE rather than TM_UNTRUSTED.
@@ -3577,7 +3421,11 @@ tls_pre_decrypt(struct tls_multi *multi,
                 }
             }
 
-            if (i == TM_SIZE && is_hard_reset(op, 0))
+            /*
+             * If we detected new session in the last if block, i has
+             * changed to TM_ACTIVE, so check the condition again.
+             */
+            if (i == TM_SIZE && is_hard_reset_method2(op))
             {
                 /*
                  * No match with existing sessions,
@@ -3597,14 +3445,6 @@ tls_pre_decrypt(struct tls_multi *multi,
                     goto error;
                 }
 
-                if (!is_hard_reset(op, multi->opt.key_method))
-                {
-                    msg(D_TLS_ERRORS, "TLS ERROR: new session local/remote key_method mismatch, local key_method=%d, op=%s",
-                        multi->opt.key_method,
-                        packet_opcode_name(op));
-                    goto error;
-                }
-
                 if (!read_control_auth(buf, &session->tls_wrap, from,
                                        session->opt))
                 {
index 58a9b0d45c211bafbf2f2b54527687eb8277f6a1..e21eee0ce318e2fdd49538dd00c48e5841ef7512 100644 (file)
 /* indicates key_method >= 2 and client-specific tls-crypt key */
 #define P_CONTROL_HARD_RESET_CLIENT_V3 10    /* initial key from client, forget previous state */
 
-/* define the range of legal opcodes */
-#define P_FIRST_OPCODE                 1
+/* define the range of legal opcodes
+ * Since we do no longer support key-method 1 we consider
+ * the v1 op codes invalid */
+#define P_FIRST_OPCODE                 3
 #define P_LAST_OPCODE                  10
 
 /*
 /* Default field in X509 to be username */
 #define X509_USERNAME_FIELD_DEFAULT "CN"
 
-/*
- * Range of key exchange methods
- */
-#define KEY_METHOD_MIN 1
-#define KEY_METHOD_MAX 2
+#define KEY_METHOD_2  2
 
 /* key method taken from lower 4 bits */
 #define KEY_METHOD_MASK 0x0F
@@ -578,12 +576,11 @@ void show_tls_performance_stats(void);
 void extract_x509_field_test(void);
 
 /**
- * Given a key_method, return true if opcode represents the required form of
- * hard_reset.
+ * Given a key_method, return true if opcode represents the one of the
+ * hard_reset op codes for key-method 2
  *
- * If key_method == 0, return true if any form of hard reset is used.
  */
-bool is_hard_reset(int op, int key_method);
+bool is_hard_reset_method2(int op);
 
 void delayed_auth_pass_purge(void);
 
index e0b3ee56c63aa251fe486553cbd951d259e784a3..d904c31f2af612703cb199966919c5c74bd0d470 100644 (file)
@@ -262,7 +262,6 @@ struct tls_options
 #endif
 
     /* from command line */
-    int key_method;
     bool replay;
     bool single_session;
 #ifdef ENABLE_OCC