diff mbox series

[Openvpn-devel,v5] crypto*: Avoid sign conversion warnings by using defines directly

Message ID 20261111183144.18676-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v5] crypto*: Avoid sign conversion warnings by using defines directly |

Commit Message

Gert Doering Nov. 11, 2026, 6:31 p.m. UTC 
  From: Frank Lichtenheld <frank@lichtenheld.com>

Using the defines directly avoids the issue with sign
conversion by letting the compiler deduce the types of
the literals.

I do not think that using an intermediate variable
increases the readability.

Change-Id: I3bcb4cb8e17ff80a5cb8acdaf18d1072159184e5
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/2158
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/2158
This mail reflects revision 5 of this Change.

Acked-by according to Gerrit (reflected above):
Razvan Cojocaru <razvanc@mailbox.org>
diff mbox series

Patch

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index a4c2683..6955760 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -81,7 +81,6 @@ 
 
     const struct key_ctx *ctx = &opt->key_ctx_bi.encrypt;
     uint8_t *mac_out = NULL;
-    const int mac_len = OPENVPN_AEAD_TAG_LENGTH;
 
     /* IV, packet-ID and implicit IV required for this mode. */
     ASSERT(ctx->cipher);
@@ -142,7 +141,7 @@ 
     dmsg(D_PACKET_CONTENT, "ENCRYPT FROM: %s", format_hex(BPTR(buf), BLENZ(buf), 80, &gc));
 
     /* Buffer overflow check */
-    if (!buf_safe(&work, buf->len + mac_len + cipher_ctx_block_size(ctx->cipher)))
+    if (!buf_safe(&work, BLENZ(buf) + OPENVPN_AEAD_TAG_LENGTH + cipher_ctx_block_size(ctx->cipher)))
     {
         msg(D_CRYPT_ERRORS, "ENCRYPT: buffer size error, bc=%d bo=%d bl=%d wc=%d wo=%d wl=%d",
             buf->capacity, buf->offset, buf->len, work.capacity, work.offset, work.len);
@@ -156,7 +155,7 @@ 
     if (!use_epoch_data_format)
     {
         /* Reserve space for authentication tag */
-        mac_out = buf_write_alloc(&work, mac_len);
+        mac_out = buf_write_alloc(&work, OPENVPN_AEAD_TAG_LENGTH);
         ASSERT(mac_out);
     }
 
@@ -177,12 +176,12 @@ 
     if (use_epoch_data_format)
     {
         /* Reserve space for authentication tag */
-        mac_out = buf_write_alloc(&work, mac_len);
+        mac_out = buf_write_alloc(&work, OPENVPN_AEAD_TAG_LENGTH);
         ASSERT(mac_out);
     }
 
     /* Write authentication tag */
-    ASSERT(cipher_ctx_get_tag(ctx->cipher, mac_out, mac_len));
+    ASSERT(cipher_ctx_get_tag(ctx->cipher, mac_out, OPENVPN_AEAD_TAG_LENGTH));
 
     *buf = work;
 
@@ -453,9 +452,6 @@ 
         CRYPT_DROP("Decryption failed verification limit reached.");
     }
 
-    const int tag_size = OPENVPN_AEAD_TAG_LENGTH;
-
-
     ASSERT(frame);
     ASSERT(buf->len > 0);
     ASSERT(ctx->cipher);
@@ -470,14 +466,14 @@ 
     ASSERT(packet_id_initialized(&opt->packet_id));
 
     /* Ensure that the packet size is long enough */
-    int min_packet_len = packet_id_size(false) + tag_size + 1;
+    size_t min_packet_len = packet_id_size(false) + OPENVPN_AEAD_TAG_LENGTH + 1;
 
     if (use_epoch_data_format)
     {
         min_packet_len += sizeof(uint32_t);
     }
 
-    if (buf->len < min_packet_len)
+    if (BLENZ(buf) < min_packet_len)
     {
         CRYPT_ERROR("missing IV info, missing tag or no payload");
     }
@@ -540,24 +536,24 @@ 
         }
     }
 
-    const int ad_size = (int)(BPTR(buf) - ad_start);
+    const uintptr_t ad_size = (uintptr_t)(BPTR(buf) - ad_start);
 
     uint8_t *tag_ptr = NULL;
     int data_len = 0;
 
     if (use_epoch_data_format)
     {
-        data_len = BLEN(buf) - tag_size;
+        data_len = BLEN(buf) - OPENVPN_AEAD_TAG_LENGTH;
         tag_ptr = BPTR(buf) + data_len;
     }
     else
     {
         tag_ptr = BPTR(buf);
-        ASSERT(buf_advance(buf, tag_size));
+        ASSERT(buf_advance(buf, OPENVPN_AEAD_TAG_LENGTH));
         data_len = BLEN(buf);
     }
 
-    dmsg(D_PACKET_CONTENT, "DECRYPT MAC: %s", format_hex(tag_ptr, tag_size, 0, &gc));
+    dmsg(D_PACKET_CONTENT, "DECRYPT MAC: %s", format_hex(tag_ptr, OPENVPN_AEAD_TAG_LENGTH, 0, &gc));
     dmsg(D_PACKET_CONTENT, "DECRYPT FROM: %s", format_hex(BPTR(buf), BLENZ(buf), 0, &gc));
 
     /* Buffer overflow check (should never fail) */
@@ -567,7 +563,7 @@ 
     }
 
     /* feed in tag and the authenticated data */
-    ASSERT(cipher_ctx_update_ad(ctx->cipher, ad_start, ad_size));
+    ASSERT(cipher_ctx_update_ad(ctx->cipher, ad_start, (int)ad_size));
     dmsg(D_PACKET_CONTENT, "DECRYPT AD: %s", format_hex(ad_start, ad_size, 0, &gc));
 
     /* Decrypt and authenticate packet */
@@ -578,7 +574,7 @@ 
     }
 
     ASSERT(buf_inc_len(&work, outlen));
-    if (!cipher_ctx_final_check_tag(ctx->cipher, BPTR(&work) + outlen, &outlen, tag_ptr, tag_size))
+    if (!cipher_ctx_final_check_tag(ctx->cipher, BPTR(&work) + outlen, &outlen, tag_ptr, OPENVPN_AEAD_TAG_LENGTH))
     {
         ctx->failed_verifications++;
         CRYPT_DROP("packet tag authentication failed");
@@ -1118,20 +1114,19 @@ 
 static void
 generate_key_random(struct key *key)
 {
-    int cipher_len = MAX_CIPHER_KEY_LENGTH;
-    int hmac_len = MAX_HMAC_KEY_LENGTH;
-
     struct gc_arena gc = gc_new();
 
     CLEAR(*key);
-    if (!rand_bytes(key->cipher, cipher_len) || !rand_bytes(key->hmac, hmac_len))
+    if (!rand_bytes(key->cipher, MAX_CIPHER_KEY_LENGTH)
+        || !rand_bytes(key->hmac, MAX_HMAC_KEY_LENGTH))
     {
         msg(M_FATAL, "ERROR: Random number generator cannot obtain entropy for key generation");
     }
 
     dmsg(D_SHOW_KEY_SOURCE, "Cipher source entropy: %s",
-         format_hex(key->cipher, cipher_len, 0, &gc));
-    dmsg(D_SHOW_KEY_SOURCE, "HMAC source entropy: %s", format_hex(key->hmac, hmac_len, 0, &gc));
+         format_hex(key->cipher, MAX_CIPHER_KEY_LENGTH, 0, &gc));
+    dmsg(D_SHOW_KEY_SOURCE, "HMAC source entropy: %s",
+         format_hex(key->hmac, MAX_HMAC_KEY_LENGTH, 0, &gc));
 
     gc_free(&gc);
 }
diff --git a/src/openvpn/crypto_epoch.c b/src/openvpn/crypto_epoch.c
index 243b5a2..40e8cec 100644
--- a/src/openvpn/crypto_epoch.c
+++ b/src/openvpn/crypto_epoch.c
@@ -38,11 +38,6 @@ 
 #include "buffer.h"
 #include "integer.h"
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wsign-conversion"
-#endif
-
 void
 ovpn_hkdf_expand(const uint8_t *secret, const uint8_t *info, size_t info_len,
                  uint8_t *out, size_t out_len)
@@ -51,26 +46,23 @@ 
     hmac_ctx_init(hmac_ctx, secret, "SHA256");
 
     ASSERT(info_len <= INT_MAX);
-    const unsigned int digest_size = SHA256_DIGEST_LENGTH;
 
     /* T(0) = empty string */
     uint8_t t_prev[SHA256_DIGEST_LENGTH];
-    unsigned int t_prev_len = 0;
 
-    for (uint8_t block = 1; (block - 1) * digest_size < out_len; block++)
+    for (uint8_t block = 1; (block - 1u) * SHA256_DIGEST_LENGTH < out_len; block++)
     {
         hmac_ctx_reset(hmac_ctx);
 
         /* calculate T(block) */
-        hmac_ctx_update(hmac_ctx, t_prev, t_prev_len);
+        hmac_ctx_update(hmac_ctx, t_prev, block == 1 ? 0 : SHA256_DIGEST_LENGTH);
         hmac_ctx_update(hmac_ctx, info, (int)info_len);
         hmac_ctx_update(hmac_ctx, &block, 1);
         hmac_ctx_final(hmac_ctx, t_prev);
-        t_prev_len = digest_size;
 
         /* Copy a full hmac output or remaining bytes */
-        size_t out_offset = (block - 1) * digest_size;
-        size_t copylen = min_size(digest_size, out_len - out_offset);
+        size_t out_offset = (block - 1u) * SHA256_DIGEST_LENGTH;
+        size_t copylen = min_size(SHA256_DIGEST_LENGTH, out_len - out_offset);
 
         memcpy(out + out_offset, t_prev, copylen);
     }
@@ -463,7 +455,3 @@ 
         epoch_iterate_send_key(opt);
     }
 }
-
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
diff --git a/src/openvpn/packet_id.h b/src/openvpn/packet_id.h
index 48752d3..782dd58 100644
--- a/src/openvpn/packet_id.h
+++ b/src/openvpn/packet_id.h
@@ -312,7 +312,7 @@ 
 const char *packet_id_net_print(const struct packet_id_net *pin, bool print_timestamp,
                                 struct gc_arena *gc);
 
-static inline int
+static inline unsigned int
 packet_id_size(bool long_form)
 {
     return sizeof(packet_id_type) + (long_form ? sizeof(net_time_t) : 0);
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 2dadc4f..7eb83ce 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -52,7 +52,7 @@ 
     return create_kt("AES-256-CTR", "SHA256", "tls-crypt");
 }
 
-int
+unsigned int
 tls_crypt_buf_overhead(void)
 {
     return packet_id_size(true) + TLS_CRYPT_TAG_SIZE + TLS_CRYPT_BLOCK_SIZE;
diff --git a/src/openvpn/tls_crypt.h b/src/openvpn/tls_crypt.h
index d179cbb..8e5567f 100644
--- a/src/openvpn/tls_crypt.h
+++ b/src/openvpn/tls_crypt.h
@@ -130,7 +130,7 @@ 
  * Returns the maximum overhead (in bytes) added to the destination buffer by
  * tls_crypt_wrap().
  */
-int tls_crypt_buf_overhead(void);
+unsigned int tls_crypt_buf_overhead(void);
 
 /**
  * Wrap a control channel packet (both authenticates and encrypts the data).
diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c b/tests/unit_tests/openvpn/test_tls_crypt.c
index 2d31c00..00de73a 100644
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
@@ -232,11 +232,6 @@ 
     }
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wsign-conversion"
-#endif
-
 /**
  * Check that short messages are successfully wrapped-and-unwrapped.
  */
@@ -778,10 +773,6 @@ 
     tls_wrap_free(&wrap_ctx);
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 static void
 test_tls_crypt_v2_write_server_key_file(void **state)
 {