Remove key_type->cipher_length field

This field is only set once to cipher_kt_key_size(kt.cipher) at the same
time that kt.cipher is set and therefore completely redundant.

This field was useful in the past when we supported cipher with variable
key length as this field would then store the key length that we would use.
Now that we do not support this anymore, we can simplify the code.

Patch v2: correct print message that would print bytes instead bits.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20211206010151.3072787-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23304.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 0d577624e0031db2b5e0755a57a2a5817034ad9f..4d089fcc39206a5557f351c74a1be08c9888bedb 100644 (file)
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -744,8 +744,6 @@ init_key_type(struct key_type *kt, const char *ciphername,
             msg(M_FATAL, "Cipher %s not supported", ciphername);
         }
 
-        kt->cipher_length = cipher_kt_key_size(kt->cipher);
-
         /* check legal cipher mode */
         aead_cipher = cipher_kt_mode_aead(kt->cipher);
         if (!(cipher_kt_mode_cbc(kt->cipher)
@@ -811,21 +809,18 @@ init_key_ctx(struct key_ctx *ctx, const struct key *key,
 {
     struct gc_arena gc = gc_new();
     CLEAR(*ctx);
-    if (kt->cipher && kt->cipher_length > 0)
+    if (kt->cipher)
     {
 
         ctx->cipher = cipher_ctx_new();
-        cipher_ctx_init(ctx->cipher, key->cipher, kt->cipher_length,
-                        kt->cipher, enc);
+        cipher_ctx_init(ctx->cipher, key->cipher, kt->cipher, enc);
 
         const char *ciphername = cipher_kt_name(kt->cipher);
         msg(D_HANDSHAKE, "%s: Cipher '%s' initialized with %d bit key",
-            prefix,
-            ciphername,
-            kt->cipher_length *8);
+            prefix, ciphername, cipher_kt_key_size(kt->cipher) * 8);
 
         dmsg(D_SHOW_KEYS, "%s: CIPHER KEY: %s", prefix,
-             format_hex(key->cipher, kt->cipher_length, 0, &gc));
+             format_hex(key->cipher, cipher_kt_key_size(kt->cipher), 0, &gc));
         dmsg(D_CRYPTO_DEBUG, "%s: CIPHER block_size=%d iv_size=%d",
              prefix, cipher_kt_block_size(kt->cipher),
              cipher_kt_iv_size(kt->cipher));
@@ -899,8 +894,8 @@ free_key_ctx_bi(struct key_ctx_bi *ctx)
 static bool
 key_is_zero(struct key *key, const struct key_type *kt)
 {
-    int i;
-    for (i = 0; i < kt->cipher_length; ++i)
+    int cipher_length = cipher_kt_key_size(kt->cipher);
+    for (int i = 0; i < cipher_length; ++i)
     {
         if (key->cipher[i])
         {
@@ -959,10 +954,7 @@ generate_key_random(struct key *key, const struct key_type *kt)
         CLEAR(*key);
         if (kt)
         {
-            if (kt->cipher && kt->cipher_length > 0 && kt->cipher_length <= cipher_len)
-            {
-                cipher_len = kt->cipher_length;
-            }
+            cipher_len = cipher_kt_key_size(kt->cipher);
 
             if (kt->digest && kt->hmac_length > 0 && kt->hmac_length <= hmac_len)
             {
@@ -996,13 +988,13 @@ key2_print(const struct key2 *k,
     ASSERT(k->n == 2);
     dmsg(D_SHOW_KEY_SOURCE, "%s (cipher): %s",
          prefix0,
-         format_hex(k->keys[0].cipher, kt->cipher_length, 0, &gc));
+         format_hex(k->keys[0].cipher, cipher_kt_key_size(kt->cipher), 0, &gc));
     dmsg(D_SHOW_KEY_SOURCE, "%s (hmac): %s",
          prefix0,
          format_hex(k->keys[0].hmac, kt->hmac_length, 0, &gc));
     dmsg(D_SHOW_KEY_SOURCE, "%s (cipher): %s",
          prefix1,
-         format_hex(k->keys[1].cipher, kt->cipher_length, 0, &gc));
+         format_hex(k->keys[1].cipher, cipher_kt_key_size(kt->cipher), 0, &gc));
     dmsg(D_SHOW_KEY_SOURCE, "%s (hmac): %s",
          prefix1,
          format_hex(k->keys[1].hmac, kt->hmac_length, 0, &gc));
@@ -1532,10 +1524,11 @@ bool
 write_key(const struct key *key, const struct key_type *kt,
           struct buffer *buf)
 {
-    ASSERT(kt->cipher_length <= MAX_CIPHER_KEY_LENGTH
+    ASSERT(cipher_kt_key_size(kt->cipher) <= MAX_CIPHER_KEY_LENGTH
            && kt->hmac_length <= MAX_HMAC_KEY_LENGTH);
 
-    if (!buf_write(buf, &kt->cipher_length, 1))
+    const uint8_t cipher_length = cipher_kt_key_size(kt->cipher);
+    if (!buf_write(buf, &cipher_length, 1))
     {
         return false;
     }
@@ -1543,7 +1536,7 @@ write_key(const struct key *key, const struct key_type *kt,
     {
         return false;
     }
-    if (!buf_write(buf, key->cipher, kt->cipher_length))
+    if (!buf_write(buf, key->cipher, cipher_kt_key_size(kt->cipher)))
     {
         return false;
     }
@@ -1577,7 +1570,7 @@ read_key(struct key *key, const struct key_type *kt, struct buffer *buf)
         goto read_err;
     }
 
-    if (cipher_length != kt->cipher_length || hmac_length != kt->hmac_length)
+    if (cipher_length != cipher_kt_key_size(kt->cipher) || hmac_length != kt->hmac_length)
     {
         goto key_len_err;
     }
@@ -1600,7 +1593,7 @@ read_err:
 key_len_err:
     msg(D_TLS_ERRORS,
         "TLS Error: key length mismatch, local cipher/hmac %d/%d, remote cipher/hmac %d/%d",
-        kt->cipher_length, kt->hmac_length, cipher_length, hmac_length);
+        cipher_kt_key_size(kt->cipher), kt->hmac_length, cipher_length, hmac_length);
     return 0;
 }
 

diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index f1af8df848a3a58d6d3b5da77dd888c75fca0017..8998a74f961ab1956ab0418f01385e2ea1a422b2 100644 (file)
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -138,7 +138,6 @@ struct sha256_digest {
  */
 struct key_type
 {
-    uint8_t cipher_length;      /**< Cipher length, in bytes */
     uint8_t hmac_length;        /**< HMAC length, in bytes */
     const cipher_kt_t *cipher;  /**< Cipher static parameters */
     const md_kt_t *digest;      /**< Message digest static parameters */

diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h
index 925d1db375af879f5a656d601997072429dcec0b..d4dd93c3abf405644ca3e98fbb66276033d8bd0d 100644 (file)
--- a/src/openvpn/crypto_backend.h
+++ b/src/openvpn/crypto_backend.h
@@ -323,12 +323,11 @@ void cipher_ctx_free(cipher_ctx_t *ctx);
  *
  * @param ctx           Cipher context. May not be NULL
  * @param key           Buffer containing the key to use
- * @param key_len       Length of the key, in bytes
  * @param kt            Static cipher parameters to use
  * @param enc           Whether to encrypt or decrypt (either
  *                      \c MBEDTLS_OP_ENCRYPT or \c MBEDTLS_OP_DECRYPT).
  */
-void cipher_ctx_init(cipher_ctx_t *ctx, const uint8_t *key, int key_len,
+void cipher_ctx_init(cipher_ctx_t *ctx, const uint8_t *key,
                      const cipher_kt_t *kt, int enc);
 
 /**

diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c
index 566baadde06306c2802b6f003ecebaf94abf8770..feb97bc948fc1f1aa4c98bcac1db53dd4e503061 100644 (file)
--- a/src/openvpn/crypto_mbedtls.c
+++ b/src/openvpn/crypto_mbedtls.c
@@ -534,10 +534,11 @@ cipher_ctx_free(mbedtls_cipher_context_t *ctx)
 }
 
 void
-cipher_ctx_init(mbedtls_cipher_context_t *ctx, const uint8_t *key, int key_len,
+cipher_ctx_init(mbedtls_cipher_context_t *ctx, const uint8_t *key,
                 const mbedtls_cipher_info_t *kt, const mbedtls_operation_t operation)
 {
     ASSERT(NULL != kt && NULL != ctx);
+    int key_len = cipher_kt_key_size(kt);
 
     CLEAR(*ctx);
 

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 9d6c7c80792357525735aa15fa0e986aabce1379..8b53b2ce8979f66580aff95f203d53f381208914 100644 (file)
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -754,7 +754,7 @@ cipher_ctx_free(EVP_CIPHER_CTX *ctx)
 }
 
 void
-cipher_ctx_init(EVP_CIPHER_CTX *ctx, const uint8_t *key, int key_len,
+cipher_ctx_init(EVP_CIPHER_CTX *ctx, const uint8_t *key,
                 const EVP_CIPHER *kt, int enc)
 {
     ASSERT(NULL != kt && NULL != ctx);
@@ -770,7 +770,7 @@ cipher_ctx_init(EVP_CIPHER_CTX *ctx, const uint8_t *key, int key_len,
     }
 
     /* make sure we used a big enough key */
-    ASSERT(EVP_CIPHER_CTX_key_length(ctx) <= key_len);
+    ASSERT(EVP_CIPHER_CTX_key_length(ctx) <= EVP_CIPHER_key_length(kt));
 }
 
 int

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index cc3d9fa0712780f0870932a50d0a1548bd553d5e..928f7e8a3b2d6a3e1664b7b3b3a5d1624213dad9 100644 (file)
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3988,7 +3988,7 @@ options_string(const struct options *o,
         {
             init_key_type(&kt, o->ciphername, o->authname, true, false);
             ciphername = cipher_kt_name(kt.cipher);
-            keysize = kt.cipher_length * 8;
+            keysize = cipher_kt_key_size(kt.cipher) * 8;
         }
         /* Only announce the cipher to our peer if we are willing to
          * support it */

diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 663f5e169a5adb122d371a39cfff67f482234dd0..8403363e2b48ef60f2d45e450a76242f37265a80 100644 (file)
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -65,7 +65,6 @@ tls_crypt_kt(void)
         return (struct key_type) { 0 };
     }
 
-    kt.cipher_length = cipher_kt_key_size(kt.cipher);
     kt.hmac_length = md_kt_size(kt.digest);
 
     return kt;