[Openvpn-devel,v3] crypto_epoch: Clean up type handling in ovpn_expand_label()

Message ID 20251004061545.7277-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v3] crypto_epoch: Clean up type handling in ovpn_expand_label() | expand

Commit Message

Gert Doering Oct. 4, 2025, 6:15 a.m. UTC
From: Frank Lichtenheld <frank@lichtenheld.com>

- Add explicit casts where we have checked the value and
  need to put it into a smaller type.
- Adapt some types to actual usage.

Change-Id: Iad717f0ff3c79ae199c8be5f93bc51bf258c68c3
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: MaxF <max@max-fillinger.net>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1218
---

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/+/1218
This mail reflects revision 3 of this Change.

Acked-by according to Gerrit (reflected above):
MaxF <max@max-fillinger.net>

Comments

Gert Doering Oct. 4, 2025, 3:36 p.m. UTC | #1
Looks reasonable.  Buildbot states that all tests (including t_client runs)
still pass. ACK from Max.  (After the last addition of ASSERT() I've become
a bit more careful and ran it on the server testbed which has epoch tests,
and that also works fine).

Your patch has been applied to the master branch.

commit cb8155711a18e2c6b4e437ab224a9eb5961dfeda
Author: Frank Lichtenheld
Date:   Sat Oct 4 08:15:38 2025 +0200

     crypto_epoch: Clean up type handling in ovpn_expand_label()

     Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
     Acked-by: MaxF <max@max-fillinger.net>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1218
     Message-Id: <20251004061545.7277-1-gert@greenie.muc.de>
     URL: https://sourceforge.net/p/openvpn/mailman/message/59242119/
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/crypto_epoch.c b/src/openvpn/crypto_epoch.c
index 7026ff8..f34dc8c 100644
--- a/src/openvpn/crypto_epoch.c
+++ b/src/openvpn/crypto_epoch.c
@@ -72,14 +72,9 @@ 
     hmac_ctx_free(hmac_ctx);
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 bool
 ovpn_expand_label(const uint8_t *secret, size_t secret_len, const uint8_t *label, size_t label_len,
-                  const uint8_t *context, size_t context_len, uint8_t *out, uint16_t out_len)
+                  const uint8_t *context, size_t context_len, uint8_t *out, int out_len)
 {
     if (secret_len != 32 || label_len > 250 || context_len > 255 || label_len < 1)
     {
@@ -89,22 +84,23 @@ 
          * need need to be in range */
         return false;
     }
+    ASSERT(out_len >= 0 && out_len <= UINT16_MAX);
 
     struct gc_arena gc = gc_new();
     /* 2 byte for the outlen encoded as uint16, 5 bytes for "ovpn ",
      * 1 byte for context len byte and 1 byte for label len byte */
     const uint8_t *label_prefix = (const uint8_t *)("ovpn ");
-    int prefix_len = 5;
+    uint8_t prefix_len = 5;
 
-    int hkdf_label_len = 2 + prefix_len + 1 + label_len + 1 + context_len;
+    size_t hkdf_label_len = 2 + prefix_len + 1 + label_len + 1 + context_len;
     struct buffer hkdf_label = alloc_buf_gc(hkdf_label_len, &gc);
 
-    buf_write_u16(&hkdf_label, out_len);
-    buf_write_u8(&hkdf_label, prefix_len + label_len);
+    buf_write_u16(&hkdf_label, (uint16_t)out_len);
+    buf_write_u8(&hkdf_label, prefix_len + (uint8_t)label_len);
     buf_write(&hkdf_label, label_prefix, prefix_len);
     buf_write(&hkdf_label, label, label_len);
 
-    buf_write_u8(&hkdf_label, context_len);
+    buf_write_u8(&hkdf_label, (uint8_t)context_len);
     if (context_len > 0)
     {
         buf_write(&hkdf_label, context, context_len);
@@ -168,10 +164,6 @@ 
     key->epoch = epoch_key->epoch;
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 static void
 epoch_init_send_key_ctx(struct crypto_options *co)
 {
diff --git a/src/openvpn/crypto_epoch.h b/src/openvpn/crypto_epoch.h
index 33ca741..a6fa116 100644
--- a/src/openvpn/crypto_epoch.h
+++ b/src/openvpn/crypto_epoch.h
@@ -60,7 +60,7 @@ 
  */
 bool ovpn_expand_label(const uint8_t *secret, size_t secret_len, const uint8_t *label,
                        size_t label_len, const uint8_t *context, size_t context_len, uint8_t *out,
-                       uint16_t out_len);
+                       int out_len);
 
 /**
  * Generate a data channel key pair from the epoch key