openvpn-devel — OpenVPN developers list

[Openvpn-devel] [XS] Change in openvpn[master]: ssl: Fix -Wconversion warnings in pem_password_callback

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1137?usp=email

to review the following change.


Change subject: ssl: Fix -Wconversion warnings in pem_password_callback
......................................................................

ssl: Fix -Wconversion warnings in pem_password_callback

The OpenSSL API is how it is, so adapt with casts.

Change-Id: I053ddbb71cc5b9ae16c5a49be833035d943d7eba
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/ssl.c
1 file changed, 2 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/37/1137/1

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 17065aa..d586ba1 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -264,10 +264,10 @@
         /* prompt for password even if --askpass wasn't specified */
         pem_password_setup(NULL);
         ASSERT(!passbuf.protected);
-        strncpynt(buf, passbuf.password, size);
+        strncpynt(buf, passbuf.password, (size_t)size);
         purge_user_pass(&passbuf, false);
 
-        return strlen(buf);
+        return (int)strlen(buf);
     }
     return 0;
 }

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1137?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I053ddbb71cc5b9ae16c5a49be833035d943d7eba
Gerrit-Change-Number: 1137
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange

[Openvpn-devel] [S] Change in openvpn[master]: openvpn_PRF: Change API to use size_t for lenghts

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1135?usp=email

to review the following change.


Change subject: openvpn_PRF: Change API to use size_t for lenghts
......................................................................

openvpn_PRF: Change API to use size_t for lenghts

Basically all users already wanted that anyway. And most
of the library functions also take size_t nowadays.

Change-Id: Ic88cd6e143bc48cab3c9ebb7c7007513803bd199
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/crypto.c
M src/openvpn/crypto_backend.h
M src/openvpn/crypto_mbedtls.c
M src/openvpn/crypto_openssl.c
M src/openvpn/ssl.c
M tests/unit_tests/openvpn/test_crypto.c
6 files changed, 22 insertions(+), 22 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/35/1135/1

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 1fa08fd..e128bb8 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -1903,8 +1903,8 @@
     uint8_t out[8];
     uint8_t expected_out[] = { 'q', 'D', '\xfe', '%', '@', 's', 'u', '\x95' };
 
-    int ret = ssl_tls1_PRF((uint8_t *)seed, (int)strlen(seed), (uint8_t *)secret,
-                           (int)strlen(secret), out, sizeof(out));
+    int ret = ssl_tls1_PRF((uint8_t *)seed, strlen(seed), (uint8_t *)secret,
+                           strlen(secret), out, sizeof(out));
 
     return (ret && memcmp(out, expected_out, sizeof(out)) == 0);
 }
diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h
index 59418f6..b74cb7f 100644
--- a/src/openvpn/crypto_backend.h
+++ b/src/openvpn/crypto_backend.h
@@ -716,7 +716,7 @@
  *
  * @return              true if successful, false on any error
  */
-bool ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret, int secret_len,
-                  uint8_t *output, int output_len);
+bool ssl_tls1_PRF(const uint8_t *seed, size_t seed_len, const uint8_t *secret, size_t secret_len,
+                  uint8_t *output, size_t output_len);
 
 #endif /* CRYPTO_BACKEND_H_ */
diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c
index 86317dd..87a2d12 100644
--- a/src/openvpn/crypto_mbedtls.c
+++ b/src/openvpn/crypto_mbedtls.c
@@ -983,8 +983,8 @@
  * from recent versions, so we use our own implementation if necessary. */
 #if defined(HAVE_MBEDTLS_SSL_TLS_PRF) && defined(MBEDTLS_SSL_TLS_PRF_TLS1)
 bool
-ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret, int secret_len,
-             uint8_t *output, int output_len)
+ssl_tls1_PRF(const uint8_t *seed, size_t seed_len, const uint8_t *secret, size_t secret_len,
+             uint8_t *output, size_t output_len)
 {
     return mbed_ok(mbedtls_ssl_tls_prf(MBEDTLS_SSL_TLS_PRF_TLS1, secret, secret_len, "", seed,
                                        seed_len, output, output_len));
@@ -1002,8 +1002,8 @@
  * @param olen          Length of the output buffer
  */
 static void
-tls1_P_hash(const mbedtls_md_info_t *md_kt, const uint8_t *sec, int sec_len, const uint8_t *seed,
-            int seed_len, uint8_t *out, int olen)
+tls1_P_hash(const mbedtls_md_info_t *md_kt, const uint8_t *sec, size_t sec_len, const uint8_t *seed,
+            size_t seed_len, uint8_t *out, size_t olen)
 {
     struct gc_arena gc = gc_new();
     uint8_t A1[MAX_HMAC_KEY_LENGTH];
@@ -1089,8 +1089,8 @@
  * (2) The pre-master secret is generated by the client.
  */
 bool
-ssl_tls1_PRF(const uint8_t *label, int label_len, const uint8_t *sec, int slen, uint8_t *out1,
-             int olen)
+ssl_tls1_PRF(const uint8_t *label, size_t label_len, const uint8_t *sec, size_t slen, uint8_t *out1,
+             size_t olen)
 {
     struct gc_arena gc = gc_new();
     const md_kt_t *md5 = md_get("MD5");
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 2351bfd..dfc2627 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -1341,8 +1341,8 @@
 }
 #if (OPENSSL_VERSION_NUMBER >= 0x30000000L) && !defined(LIBRESSL_VERSION_NUMBER)
 bool
-ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret, int secret_len,
-             uint8_t *output, int output_len)
+ssl_tls1_PRF(const uint8_t *seed, size_t seed_len, const uint8_t *secret, size_t secret_len,
+             uint8_t *output, size_t output_len)
 {
     bool ret = true;
     EVP_KDF_CTX *kctx = NULL;
@@ -1368,9 +1368,9 @@
     params[0] =
         OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_DIGEST, SN_md5_sha1, strlen(SN_md5_sha1));
     params[1] = OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_SECRET, (uint8_t *)secret,
-                                                  (size_t)secret_len);
+                                                  secret_len);
     params[2] =
-        OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_SEED, (uint8_t *)seed, (size_t)seed_len);
+        OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_SEED, (uint8_t *)seed, seed_len);
     params[3] = OSSL_PARAM_construct_end();
 
     if (EVP_KDF_derive(kctx, output, output_len, params) <= 0)
@@ -1392,15 +1392,15 @@
 }
 #elif defined(OPENSSL_IS_AWSLC)
 bool
-ssl_tls1_PRF(const uint8_t *label, int label_len, const uint8_t *sec, int slen, uint8_t *out1,
-             int olen)
+ssl_tls1_PRF(const uint8_t *label, size_t label_len, const uint8_t *sec, size_t slen, uint8_t *out1,
+             size_t olen)
 {
     CRYPTO_tls1_prf(EVP_md5_sha1(), out1, olen, sec, slen, label, label_len, NULL, 0, NULL, 0);
 }
 #elif !defined(LIBRESSL_VERSION_NUMBER) && !defined(ENABLE_CRYPTO_WOLFSSL)
 bool
-ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret, int secret_len,
-             uint8_t *output, int output_len)
+ssl_tls1_PRF(const uint8_t *seed, size_t seed_len, const uint8_t *secret, size_t secret_len,
+             uint8_t *output, size_t output_len)
 {
     EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_TLS1_PRF, NULL);
     if (!pctx)
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 84ef4fb..17065aa 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1294,10 +1294,10 @@
 }
 
 static bool
-openvpn_PRF(const uint8_t *secret, int secret_len, const char *label, const uint8_t *client_seed,
-            int client_seed_len, const uint8_t *server_seed, int server_seed_len,
+openvpn_PRF(const uint8_t *secret, size_t secret_len, const char *label, const uint8_t *client_seed,
+            size_t client_seed_len, const uint8_t *server_seed, size_t server_seed_len,
             const struct session_id *client_sid, const struct session_id *server_sid,
-            uint8_t *output, int output_len)
+            uint8_t *output, size_t output_len)
 {
     /* concatenate seed components */
 
diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c
index de8f9fe..77834e8 100644
--- a/tests/unit_tests/openvpn/test_crypto.c
+++ b/tests/unit_tests/openvpn/test_crypto.c
@@ -161,7 +161,7 @@
 
 
     uint8_t out[32];
-    bool ret = ssl_tls1_PRF(seed, (int)seed_len, secret, (int)secret_len, out, sizeof(out));
+    bool ret = ssl_tls1_PRF(seed, seed_len, secret, secret_len, out, sizeof(out));
 
 #if defined(LIBRESSL_VERSION_NUMBER) || defined(ENABLE_CRYPTO_WOLFSSL)
     /* No TLS1 PRF support in these libraries */

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1135?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic88cd6e143bc48cab3c9ebb7c7007513803bd199
Gerrit-Change-Number: 1135
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange

[Openvpn-devel] [XS] Change in openvpn[master]: buffer: remove unused function buf_write_alloc_prepend

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1134?usp=email

to review the following change.


Change subject: buffer: remove unused function buf_write_alloc_prepend
......................................................................

buffer: remove unused function buf_write_alloc_prepend

Change-Id: I71981e39932cafe3fd68b475fdb81a8f20a3a547
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/buffer.h
1 file changed, 0 insertions(+), 6 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/34/1134/1

diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h
index 8d6bb64..6f15115 100644
--- a/src/openvpn/buffer.h
+++ b/src/openvpn/buffer.h
@@ -644,12 +644,6 @@
 }
 
 static inline uint8_t *
-buf_write_alloc_prepend(struct buffer *buf, int size, bool prepend)
-{
-    return prepend ? buf_prepend(buf, size) : buf_write_alloc(buf, size);
-}
-
-static inline uint8_t *
 buf_read_alloc(struct buffer *buf, int size)
 {
     uint8_t *ret;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1134?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I71981e39932cafe3fd68b475fdb81a8f20a3a547
Gerrit-Change-Number: 1134
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange

[Openvpn-devel] [S] Change in openvpn[master]: socket: Change resolve flags to unsigned int

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1131?usp=email

to review the following change.


Change subject: socket: Change resolve flags to unsigned int
......................................................................

socket: Change resolve flags to unsigned int

And use them consistently so to avoid conversion
warnings.

Change-Id: I5ef21e425786a49c90d4b7305c3fb174ab6ddf92
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/socket.c
M src/openvpn/socket.h
2 files changed, 18 insertions(+), 18 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/31/1131/1

diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index b0a158b..4019c1e 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -246,10 +246,10 @@
  */
 static int
 get_cached_dns_entry(struct cached_dns_entry *dns_cache, const char *hostname, const char *servname,
-                     int ai_family, int resolve_flags, struct addrinfo **ai)
+                     int ai_family, unsigned int resolve_flags, struct addrinfo **ai)
 {
     struct cached_dns_entry *ph;
-    int flags;
+    unsigned int flags;
 
     /* Only use flags that are relevant for the structure */
     flags = resolve_flags & GETADDR_CACHE_MASK;
@@ -269,7 +269,7 @@
 
 static int
 do_preresolve_host(struct context *c, const char *hostname, const char *servname, const int af,
-                   const int flags)
+                   const unsigned int flags)
 {
     struct addrinfo *ai;
     int status;
@@ -323,7 +323,7 @@
     {
         int status;
         const char *remote;
-        int flags = preresolve_flags;
+        unsigned int flags = preresolve_flags;
 
         struct connection_entry *ce = l->array[i];
 
@@ -1608,7 +1608,7 @@
     /* resolve local address if undefined */
     if (!sock->info.lsa->bind_local)
     {
-        int flags = GETADDR_RESOLVE | GETADDR_WARN_ON_SIGNAL | GETADDR_FATAL | GETADDR_PASSIVE;
+        unsigned int flags = GETADDR_RESOLVE | GETADDR_WARN_ON_SIGNAL | GETADDR_FATAL | GETADDR_PASSIVE;
         int status;
 
         if (proto_is_dgram(sock->info.proto))
diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
index c34c65a..3b82dac 100644
--- a/src/openvpn/socket.h
+++ b/src/openvpn/socket.h
@@ -78,7 +78,7 @@
     const char *hostname;
     const char *servname;
     int ai_family;
-    int flags;
+    unsigned int flags;
     struct addrinfo *ai;
     struct cached_dns_entry *next;
 };
@@ -476,18 +476,18 @@
  * DNS resolution
  */
 
-#define GETADDR_RESOLVE                 (1 << 0)
-#define GETADDR_FATAL                   (1 << 1)
-#define GETADDR_HOST_ORDER              (1 << 2)
-#define GETADDR_MENTION_RESOLVE_RETRY   (1 << 3)
-#define GETADDR_FATAL_ON_SIGNAL         (1 << 4)
-#define GETADDR_WARN_ON_SIGNAL          (1 << 5)
-#define GETADDR_MSG_VIRT_OUT            (1 << 6)
-#define GETADDR_TRY_ONCE                (1 << 7)
-#define GETADDR_UPDATE_MANAGEMENT_STATE (1 << 8)
-#define GETADDR_RANDOMIZE               (1 << 9)
-#define GETADDR_PASSIVE                 (1 << 10)
-#define GETADDR_DATAGRAM                (1 << 11)
+#define GETADDR_RESOLVE                 (1u << 0)
+#define GETADDR_FATAL                   (1u << 1)
+#define GETADDR_HOST_ORDER              (1u << 2)
+#define GETADDR_MENTION_RESOLVE_RETRY   (1u << 3)
+#define GETADDR_FATAL_ON_SIGNAL         (1u << 4)
+#define GETADDR_WARN_ON_SIGNAL          (1u << 5)
+#define GETADDR_MSG_VIRT_OUT            (1u << 6)
+#define GETADDR_TRY_ONCE                (1u << 7)
+#define GETADDR_UPDATE_MANAGEMENT_STATE (1u << 8)
+#define GETADDR_RANDOMIZE               (1u << 9)
+#define GETADDR_PASSIVE                 (1u << 10)
+#define GETADDR_DATAGRAM                (1u << 11)
 
 #define GETADDR_CACHE_MASK (GETADDR_DATAGRAM | GETADDR_PASSIVE)
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1131?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I5ef21e425786a49c90d4b7305c3fb174ab6ddf92
Gerrit-Change-Number: 1131
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange

[Openvpn-devel] [S] Change in openvpn[master]: manage: Change command_line_* API to use size_t for lengths

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1136?usp=email

to review the following change.


Change subject: manage: Change command_line_* API to use size_t for lengths
......................................................................

manage: Change command_line_* API to use size_t for lengths

The used functions already expect this.

Change-Id: Ifc183e42b190e19e1d8c351d1cd460a038626e63
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/manage.c
M src/openvpn/manage.h
2 files changed, 7 insertions(+), 9 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/36/1136/1

diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 16b1b73..9750d58 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -2247,7 +2247,7 @@
         bool processed_command = false;
 
         ASSERT(len <= (int)sizeof(buf));
-        command_line_add(man->connection.in, buf, len);
+        command_line_add(man->connection.in, buf, (size_t)len);
 
         /*
          * Reset output object
@@ -3786,7 +3786,7 @@
  */
 
 struct command_line *
-command_line_new(const int buf_len)
+command_line_new(const size_t buf_len)
 {
     struct command_line *cl;
     ALLOC_OBJ_CLEAR(cl, struct command_line);
@@ -3816,10 +3816,9 @@
 }
 
 void
-command_line_add(struct command_line *cl, const unsigned char *buf, const int len)
+command_line_add(struct command_line *cl, const unsigned char *buf, const size_t len)
 {
-    int i;
-    for (i = 0; i < len; ++i)
+    for (size_t i = 0; i < len; ++i)
     {
         if (buf[i] && char_class(buf[i], (CC_PRINT | CC_NEWLINE)))
         {
@@ -3834,10 +3833,9 @@
 const char *
 command_line_get(struct command_line *cl)
 {
-    int i;
     const char *ret = NULL;
 
-    i = buf_substring_len(&cl->buf, '\n');
+    int i = buf_substring_len(&cl->buf, '\n');
     if (i >= 0)
     {
         buf_copy_excess(&cl->residual, &cl->buf, i);
diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h
index 01d180a..e234df7 100644
--- a/src/openvpn/manage.h
+++ b/src/openvpn/manage.h
@@ -83,11 +83,11 @@
     struct buffer residual;
 };
 
-struct command_line *command_line_new(const int buf_len);
+struct command_line *command_line_new(const size_t buf_len);
 
 void command_line_free(struct command_line *cl);
 
-void command_line_add(struct command_line *cl, const unsigned char *buf, const int len);
+void command_line_add(struct command_line *cl, const unsigned char *buf, const size_t len);
 
 const char *command_line_get(struct command_line *cl);
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1136?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ifc183e42b190e19e1d8c351d1cd460a038626e63
Gerrit-Change-Number: 1136
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange

[Openvpn-devel] [M] Change in openvpn[master]: buffer: Add BLENZ macro that returns size_t and use it where required

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1132?usp=email

to review the following change.


Change subject: buffer: Add BLENZ macro that returns size_t and use it where required
......................................................................

buffer: Add BLENZ macro that returns size_t and use it where required

The big int-vs-size_t length confusion in buffer and its
users can't be solved easily or quickly. So as a first step
document which users of BLEN actually already want a size_t
return. This is better than adding manual size_t casts since
it should be easier to change the API later.

Still reduces the -Wconversion noice considerably.

Change-Id: I4e75ba1dbc6d9a0f75298bc900f713b67e60d096
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/buffer.c
M src/openvpn/buffer.h
M src/openvpn/comp-lz4.c
M src/openvpn/crypto.c
M src/openvpn/dhcp.c
M src/openvpn/lzo.c
M src/openvpn/manage.c
M src/openvpn/misc.c
M src/openvpn/mroute.c
M src/openvpn/mss.c
M src/openvpn/pkcs11.c
M src/openvpn/proto.c
M src/openvpn/ps.c
M src/openvpn/push.c
M src/openvpn/socket.c
M src/openvpn/socket.h
M src/openvpn/ssl.c
M src/openvpn/ssl_openssl.c
M src/openvpn/ssl_pkt.c
M src/openvpn/ssl_verify.c
M src/openvpn/tls_crypt.c
M src/openvpn/vlan.c
M tests/unit_tests/openvpn/test_buffer.c
M tests/unit_tests/openvpn/test_crypto.c
M tests/unit_tests/openvpn/test_pkt.c
M tests/unit_tests/openvpn/test_ssl.c
M tests/unit_tests/openvpn/test_tls_crypt.c
27 files changed, 81 insertions(+), 80 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/32/1132/1

diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index b6d6669..3dd8b31 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -126,7 +126,7 @@
     ret.data = (uint8_t *)malloc(buf->capacity);
 #endif
     check_malloc_return(ret.data);
-    memcpy(BPTR(&ret), BPTR(buf), BLEN(buf));
+    memcpy(BPTR(&ret), BPTR(buf), BLENZ(buf));
     return ret;
 }
 
@@ -177,7 +177,7 @@
     {
         return false;
     }
-    return buf_write(dest, BPTR(src), BLEN(src));
+    return buf_write(dest, BPTR(src), BLENZ(src));
 }
 
 void
@@ -308,7 +308,7 @@
         return false;
     }
 
-    const ssize_t size = write(fd, BPTR(buf), BLEN(buf));
+    const ssize_t size = write(fd, BPTR(buf), BLENZ(buf));
     if (size != BLEN(buf))
     {
         msg(M_ERRNO, "Write error on file '%s'", filename);
@@ -1246,9 +1246,9 @@
     struct buffer_entry *more = bl->head;
     size_t size = 0;
     int count = 0;
-    for (count = 0; more; ++count)
+    for (; more; ++count)
     {
-        size_t extra_len = BLEN(&more->buf) + sep_len;
+        size_t extra_len = BLENZ(&more->buf) + sep_len;
         if (size + extra_len > max_len)
         {
             break;
diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h
index ae783c6..8d6bb64 100644
--- a/src/openvpn/buffer.h
+++ b/src/openvpn/buffer.h
@@ -124,6 +124,7 @@
 #define BEND(buf)  (buf_bend(buf))
 #define BLAST(buf) (buf_blast(buf))
 #define BLEN(buf)  (buf_len(buf))
+#define BLENZ(buf) ((size_t)buf_len(buf))
 #define BDEF(buf)  (buf_defined(buf))
 #define BSTR(buf)  (buf_str(buf))
 #define BCAP(buf)  (buf_forward_capacity(buf))
@@ -709,7 +710,7 @@
 static inline bool
 buf_copy(struct buffer *dest, const struct buffer *src)
 {
-    return buf_write(dest, BPTR(src), BLEN(src));
+    return buf_write(dest, BPTR(src), BLENZ(src));
 }
 
 static inline bool
@@ -826,7 +827,7 @@
 static inline bool
 buf_equal(const struct buffer *a, const struct buffer *b)
 {
-    return BLEN(a) == BLEN(b) && 0 == memcmp(BPTR(a), BPTR(b), BLEN(a));
+    return BLEN(a) == BLEN(b) && 0 == memcmp(BPTR(a), BPTR(b), BLENZ(a));
 }
 
 /**
diff --git a/src/openvpn/comp-lz4.c b/src/openvpn/comp-lz4.c
index 6736469..366cc1a 100644
--- a/src/openvpn/comp-lz4.c
+++ b/src/openvpn/comp-lz4.c
@@ -94,7 +94,7 @@
 {
     int uncomp_len;
     ASSERT(buf_safe(work, zlen_max));
-    uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(work), (size_t)BLEN(buf),
+    uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(work), BLENZ(buf),
                                      zlen_max);
     if (uncomp_len <= 0)
     {
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 6882ec3..1fa08fd 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -112,7 +112,7 @@
             }
         }
         /* Write packet id part of IV to work buffer */
-        ASSERT(buf_write(&work, iv, buf_len(&iv_buffer)));
+        ASSERT(buf_write(&work, iv, BLENZ(&iv_buffer)));
 
         /* This generates the IV by XORing the implicit part of the IV
          * with the packet id already written to the iv buffer */
@@ -1237,9 +1237,9 @@
 
         /* copy source to input buf */
         buf = work;
-        buf_p = buf_write_alloc(&buf, BLEN(&src));
+        buf_p = buf_write_alloc(&buf, BLENZ(&src));
         ASSERT(buf_p);
-        memcpy(buf_p, BPTR(&src), BLEN(&src));
+        memcpy(buf_p, BPTR(&src), BLENZ(&src));
 
         /* initialize work buffer with buf.headroom bytes of prepend capacity */
         ASSERT(buf_init(&encrypt_workspace, frame->buf.headroom));
diff --git a/src/openvpn/dhcp.c b/src/openvpn/dhcp.c
index 0a7689f..f0e0d73f 100644
--- a/src/openvpn/dhcp.c
+++ b/src/openvpn/dhcp.c
@@ -150,7 +150,7 @@
     struct dhcp_full *df = (struct dhcp_full *)BPTR(ipbuf);
     const int optlen =
         BLEN(ipbuf)
-        - (sizeof(struct openvpn_iphdr) + sizeof(struct openvpn_udphdr) + sizeof(struct dhcp));
+        - (int)(sizeof(struct openvpn_iphdr) + sizeof(struct openvpn_udphdr) + sizeof(struct dhcp));
 
     if (optlen >= 0 && df->ip.protocol == OPENVPN_IPPROTO_UDP
         && df->udp.source == htons(BOOTPS_PORT) && df->udp.dest == htons(BOOTPC_PORT)
diff --git a/src/openvpn/lzo.c b/src/openvpn/lzo.c
index 3a73d5f..a86d7a5 100644
--- a/src/openvpn/lzo.c
+++ b/src/openvpn/lzo.c
@@ -77,7 +77,6 @@
                const struct frame *frame)
 {
     lzo_uint zlen = frame->buf.payload_size;
-    int err;
     uint8_t c; /* flag indicating whether or not our peer compressed */
 
     if (buf->len <= 0)
@@ -93,7 +92,7 @@
     if (c == LZO_COMPRESS_BYTE) /* packet was compressed */
     {
         ASSERT(buf_safe(&work, zlen));
-        err = LZO_DECOMPRESS(BPTR(buf), BLEN(buf), BPTR(&work), &zlen, compctx->wu.lzo.wmem);
+        int err = LZO_DECOMPRESS(BPTR(buf), BLENZ(buf), BPTR(&work), &zlen, compctx->wu.lzo.wmem);
         if (err != LZO_E_OK)
         {
             dmsg(D_COMP_ERRORS, "LZO decompression error: %d", err);
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index aed04f5..16b1b73 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -3634,9 +3634,9 @@
         buf = buffer_list_peek(*input);
         if (buf && BLEN(buf) > 0)
         {
-            result = (char *)malloc(BLEN(buf) + 1);
+            result = (char *)malloc(BLENZ(buf) + 1);
             check_malloc_return(result);
-            memcpy(result, buf->data, BLEN(buf));
+            memcpy(result, buf->data, BLENZ(buf));
             result[BLEN(buf)] = '\0';
         }
     }
@@ -3663,9 +3663,9 @@
         buf = buffer_list_peek(*input);
         if (buf && BLEN(buf) > 0)
         {
-            result = (char *)malloc(BLEN(buf) + 1);
+            result = (char *)malloc(BLENZ(buf) + 1);
             check_malloc_return(result);
-            memcpy(result, buf->data, BLEN(buf));
+            memcpy(result, buf->data, BLENZ(buf));
             result[BLEN(buf)] = '\0';
         }
     }
diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index 17f7706..6fb16f0 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -251,7 +251,7 @@
                 struct buffer user_prompt = alloc_buf_gc(128, &gc);
 
                 buf_printf(&user_prompt, "NEED-OK|%s|%s:", prefix, up->username);
-                if (!query_user_SINGLE(BSTR(&user_prompt), BLEN(&user_prompt), up->password,
+                if (!query_user_SINGLE(BSTR(&user_prompt), BLENZ(&user_prompt), up->password,
                                        USER_PASS_LEN, false))
                 {
                     msg(M_FATAL, "ERROR: could not read %s ok-confirmation from stdin", prefix);
@@ -374,7 +374,7 @@
                     buf_printf(&challenge, "CHALLENGE: %s", ac->challenge_text);
                     buf_set_write(&packed_resp, (uint8_t *)up->password, USER_PASS_LEN);
 
-                    if (!query_user_SINGLE(BSTR(&challenge), BLEN(&challenge), response,
+                    if (!query_user_SINGLE(BSTR(&challenge), BLENZ(&challenge), response,
                                            USER_PASS_LEN, BOOL_CAST(ac->flags & CR_ECHO)))
                     {
                         msg(M_FATAL, "ERROR: could not read challenge response from stdin");
@@ -399,13 +399,13 @@
 
                 if (username_from_stdin && !(flags & GET_USER_PASS_PASSWORD_ONLY))
                 {
-                    query_user_add(BSTR(&user_prompt), BLEN(&user_prompt), up->username,
+                    query_user_add(BSTR(&user_prompt), BLENZ(&user_prompt), up->username,
                                    USER_PASS_LEN, true);
                 }
 
                 if (password_from_stdin)
                 {
-                    query_user_add(BSTR(&pass_prompt), BLEN(&pass_prompt), up->password,
+                    query_user_add(BSTR(&pass_prompt), BLENZ(&pass_prompt), up->password,
                                    USER_PASS_LEN, false);
                 }
 
@@ -433,7 +433,7 @@
                     challenge = alloc_buf_gc(14 + strlen(auth_challenge), &gc);
                     buf_printf(&challenge, "CHALLENGE: %s", auth_challenge);
 
-                    if (!query_user_SINGLE(BSTR(&challenge), BLEN(&challenge), response,
+                    if (!query_user_SINGLE(BSTR(&challenge), BLENZ(&challenge), response,
                                            USER_PASS_LEN,
                                            BOOL_CAST(flags & GET_USER_PASS_STATIC_CHALLENGE_ECHO)))
                     {
diff --git a/src/openvpn/mroute.c b/src/openvpn/mroute.c
index a598037..62335c4 100644
--- a/src/openvpn/mroute.c
+++ b/src/openvpn/mroute.c
@@ -152,7 +152,7 @@
         switch (OPENVPN_IPH_GET_VER(*BPTR(buf)))
         {
             case 4:
-                if (BLEN(buf) >= (int)sizeof(struct openvpn_iphdr))
+                if (BLENZ(buf) >= sizeof(struct openvpn_iphdr))
                 {
                     const struct openvpn_iphdr *ip = (const struct openvpn_iphdr *)BPTR(buf);
 
@@ -176,7 +176,7 @@
                 break;
 
             case 6:
-                if (BLEN(buf) >= (int)sizeof(struct openvpn_ipv6hdr))
+                if (BLENZ(buf) >= sizeof(struct openvpn_ipv6hdr))
                 {
                     const struct openvpn_ipv6hdr *ipv6 = (const struct openvpn_ipv6hdr *)BPTR(buf);
 #if 0 /* very basic debug */
diff --git a/src/openvpn/mss.c b/src/openvpn/mss.c
index 32cd3f8..bf18202 100644
--- a/src/openvpn/mss.c
+++ b/src/openvpn/mss.c
@@ -48,7 +48,7 @@
     const struct openvpn_iphdr *pip;
     int hlen;
 
-    if (BLEN(buf) < (int)sizeof(struct openvpn_iphdr))
+    if (BLENZ(buf) < sizeof(struct openvpn_iphdr))
     {
         return;
     }
@@ -85,7 +85,7 @@
     const struct openvpn_ipv6hdr *pip6;
     struct buffer newbuf;
 
-    if (BLEN(buf) < (int)sizeof(struct openvpn_ipv6hdr))
+    if (BLENZ(buf) < sizeof(struct openvpn_ipv6hdr))
     {
         return;
     }
@@ -96,7 +96,7 @@
     /* do we have the full IPv6 packet?
      * "payload_len" does not include IPv6 header (+40 bytes)
      */
-    if (BLEN(buf) != (int)ntohs(pip6->payload_len) + 40)
+    if (BLENZ(buf) != ntohs(pip6->payload_len) + 40)
     {
         return;
     }
@@ -120,7 +120,7 @@
      * verify remainder is large enough to contain a full TCP header
      */
     newbuf = *buf;
-    if (buf_advance(&newbuf, 40) && BLEN(&newbuf) >= (int)sizeof(struct openvpn_tcphdr))
+    if (buf_advance(&newbuf, 40) && BLENZ(&newbuf) >= sizeof(struct openvpn_tcphdr))
     {
         struct openvpn_tcphdr *tc = (struct openvpn_tcphdr *)BPTR(&newbuf);
         if (tc->flags & OPENVPN_TCPH_SYN_MASK)
@@ -144,7 +144,7 @@
     int accumulate;
     struct openvpn_tcphdr *tc;
 
-    if (BLEN(buf) < (int)sizeof(struct openvpn_tcphdr))
+    if (BLENZ(buf) < sizeof(struct openvpn_tcphdr))
     {
         return;
     }
diff --git a/src/openvpn/pkcs11.c b/src/openvpn/pkcs11.c
index dfc87f6..d9429f9 100644
--- a/src/openvpn/pkcs11.c
+++ b/src/openvpn/pkcs11.c
@@ -662,7 +662,7 @@
     ASSERT(token != NULL);
 
     buf_printf(&pass_prompt, "Please enter '%s' token PIN or 'cancel': ", token->display);
-    if (!query_user_SINGLE(BSTR(&pass_prompt), BLEN(&pass_prompt), pin, pin_max, false))
+    if (!query_user_SINGLE(BSTR(&pass_prompt), BLENZ(&pass_prompt), pin, pin_max, false))
     {
         msg(M_FATAL, "Could not retrieve the PIN");
     }
diff --git a/src/openvpn/proto.c b/src/openvpn/proto.c
index 34b3378..9adc623 100644
--- a/src/openvpn/proto.c
+++ b/src/openvpn/proto.c
@@ -45,7 +45,7 @@
     verify_align_4(buf);
     if (tunnel_type == DEV_TYPE_TUN)
     {
-        if (BLEN(buf) < sizeof(struct openvpn_iphdr))
+        if (BLENZ(buf) < sizeof(struct openvpn_iphdr))
         {
             return false;
         }
@@ -54,7 +54,7 @@
     else if (tunnel_type == DEV_TYPE_TAP)
     {
         const struct openvpn_ethhdr *eh;
-        if (BLEN(buf) < (sizeof(struct openvpn_ethhdr) + sizeof(struct openvpn_iphdr)))
+        if (BLENZ(buf) < (sizeof(struct openvpn_ethhdr) + sizeof(struct openvpn_iphdr)))
         {
             return false;
         }
@@ -70,7 +70,7 @@
         if (proto == htons(OPENVPN_ETH_P_8021Q))
         {
             const struct openvpn_8021qhdr *evh;
-            if (BLEN(buf) < (sizeof(struct openvpn_ethhdr) + sizeof(struct openvpn_iphdr)))
+            if (BLENZ(buf) < (sizeof(struct openvpn_ethhdr) + sizeof(struct openvpn_iphdr)))
             {
                 return false;
             }
@@ -185,7 +185,7 @@
             const char *msgstr = "PACKET SIZE INFO";
             unsigned int msglevel = D_PACKET_TRUNC_DEBUG;
 
-            if (BLEN(&buf) < (int)sizeof(struct openvpn_iphdr))
+            if (BLENZ(&buf) < sizeof(struct openvpn_iphdr))
             {
                 return;
             }
diff --git a/src/openvpn/ps.c b/src/openvpn/ps.c
index b4199c3..a04398e 100644
--- a/src/openvpn/ps.c
+++ b/src/openvpn/ps.c
@@ -209,7 +209,7 @@
         if (head)
         {
             iov[1].iov_base = BPTR(head);
-            iov[1].iov_len = BLEN(head);
+            iov[1].iov_len = BLENZ(head);
             mesg.msg_iovlen = 2;
         }
 
@@ -586,7 +586,7 @@
 proxy_connection_io_send(struct proxy_connection *pc, int *bytes_sent)
 {
     const socket_descriptor_t sd = pc->counterpart->sd;
-    const int status = send(sd, BPTR(&pc->buf), BLEN(&pc->buf), MSG_NOSIGNAL);
+    const int status = send(sd, BPTR(&pc->buf), BLENZ(&pc->buf), MSG_NOSIGNAL);
 
     if (status < 0)
     {
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index b0be70d..750204e 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -817,7 +817,7 @@
         buf_printf(&buf, ",push-continuation 1");
     }
 
-    if (BLEN(&buf) > sizeof(push_reply_cmd) - 1)
+    if (BLENZ(&buf) > sizeof(push_reply_cmd) - 1)
     {
         const bool status = send_control_channel_string(c, BSTR(&buf), D_PUSH);
         if (!status)
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 4019c1e..0558db7 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -3174,7 +3174,7 @@
 #else
         struct buffer frag;
         stream_buf_get_next(&sock->stream_buf, &frag);
-        len = recv(sock->sd, BPTR(&frag), BLEN(&frag), MSG_NOSIGNAL);
+        len = recv(sock->sd, BPTR(&frag), BLENZ(&frag), MSG_NOSIGNAL);
 #endif
 
         if (!len)
@@ -3320,8 +3320,8 @@
 ssize_t
 link_socket_write_tcp(struct link_socket *sock, struct buffer *buf, struct link_socket_actual *to)
 {
-    packet_size_type len = BLEN(buf);
-    dmsg(D_STREAM_DEBUG, "STREAM: WRITE %d offset=%d", (int)len, buf->offset);
+    packet_size_type len = (packet_size_type)BLENZ(buf);
+    dmsg(D_STREAM_DEBUG, "STREAM: WRITE %u offset=%d", len, buf->offset);
     ASSERT(len <= sock->stream_buf.maxlen);
     len = htonps(len);
     ASSERT(buf_write_prepend(buf, &len, sizeof(len)));
@@ -3344,7 +3344,7 @@
     uint8_t pktinfo_buf[PKTINFO_BUF_SIZE];
 
     iov.iov_base = BPTR(buf);
-    iov.iov_len = BLEN(buf);
+    iov.iov_len = BLENZ(buf);
     mesg.msg_iov = &iov;
     mesg.msg_iovlen = 1;
     switch (to->dest.addr.sa.sa_family)
diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
index 3b82dac..51d6813 100644
--- a/src/openvpn/socket.h
+++ b/src/openvpn/socket.h
@@ -1111,14 +1111,14 @@
     }
     else
 #endif
-        return sendto(sock->sd, BPTR(buf), BLEN(buf), 0, (struct sockaddr *)&to->dest.addr.sa,
+        return sendto(sock->sd, BPTR(buf), BLENZ(buf), 0, (struct sockaddr *)&to->dest.addr.sa,
                       (socklen_t)af_addr_size(to->dest.addr.sa.sa_family));
 }
 
 static inline ssize_t
 link_socket_write_tcp_posix(struct link_socket *sock, struct buffer *buf)
 {
-    return send(sock->sd, BPTR(buf), BLEN(buf), MSG_NOSIGNAL);
+    return send(sock->sd, BPTR(buf), BLENZ(buf), MSG_NOSIGNAL);
 }
 
 #endif /* ifdef _WIN32 */
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index b7db1e7..84ef4fb 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1318,7 +1318,7 @@
     }
 
     /* compute PRF */
-    bool ret = ssl_tls1_PRF(BPTR(&seed), BLEN(&seed), secret, secret_len, output, output_len);
+    bool ret = ssl_tls1_PRF(BPTR(&seed), BLENZ(&seed), secret, secret_len, output, output_len);
 
     buf_clear(&seed);
     free_buf(&seed);
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index aa1ac11..b2fa972 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -2083,9 +2083,10 @@
 static void
 bio_write_post(const int status, struct buffer *buf)
 {
-    if (status == 1)                     /* success status return from bio_write? */
+    /* success status return from bio_write? */
+    if (status == 1)
     {
-        memset(BPTR(buf), 0, BLEN(buf)); /* erase data just written */
+        memset(BPTR(buf), 0, BLENZ(buf)); /* erase data just written */
         buf->len = 0;
     }
 }
diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
index b901f87..d34036f 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -531,7 +531,7 @@
 {
     /* commands on the control channel are seperated by 0x00 bytes.
      * cmdlen does not include the 0 byte of the string */
-    int cmdlen = (int)strnlen(BSTR(buf), BLEN(buf));
+    int cmdlen = (int)strnlen(BSTR(buf), BLENZ(buf));
 
     if (cmdlen >= BLEN(buf))
     {
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 6f85dca..5179d33 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -692,7 +692,7 @@
 
         while (current_hash)
         {
-            if (memcmp_constant_time(BPTR(&cert_fp), current_hash->hash, BLEN(&cert_fp)) == 0)
+            if (memcmp_constant_time(BPTR(&cert_fp), current_hash->hash, BLENZ(&cert_fp)) == 0)
             {
                 break;
             }
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 2892199..f579c61 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -158,7 +158,7 @@
     dmsg(D_PACKET_CONTENT, "TLS-CRYPT WRAP AD: %s", format_hex(BPTR(dst), BLEN(dst), 0, &gc));
 
     /* Buffer overflow check */
-    if (!buf_safe(dst, BLEN(src) + TLS_CRYPT_BLOCK_SIZE + TLS_CRYPT_TAG_SIZE))
+    if (!buf_safe(dst, BLENZ(src) + TLS_CRYPT_BLOCK_SIZE + TLS_CRYPT_TAG_SIZE))
     {
         msg(D_CRYPT_ERRORS,
             "TLS-CRYPT WRAP: buffer size error, "
@@ -231,7 +231,7 @@
         int outlen = 0;
 
         /* Buffer overflow check (should never fail) */
-        if (!buf_safe(dst, BLEN(src) - TLS_CRYPT_OFF_CT + TLS_CRYPT_BLOCK_SIZE))
+        if (!buf_safe(dst, BLENZ(src) - TLS_CRYPT_OFF_CT + TLS_CRYPT_BLOCK_SIZE))
         {
             CRYPT_ERROR("potential buffer overflow");
         }
@@ -241,7 +241,7 @@
             CRYPT_ERROR("cipher reset failed");
         }
         if (!cipher_ctx_update(ctx->cipher, BPTR(dst), &outlen, BPTR(src) + TLS_CRYPT_OFF_CT,
-                               BLEN(src) - TLS_CRYPT_OFF_CT))
+                               (int)(BLENZ(src) - TLS_CRYPT_OFF_CT)))
         {
             CRYPT_ERROR("cipher update failed");
         }
@@ -376,8 +376,8 @@
         msg(M_WARN, "ERROR: could not write tag");
         return false;
     }
-    uint16_t net_len = htons(sizeof(src_key->keys) + BLEN(src_metadata) + TLS_CRYPT_V2_TAG_SIZE
-                             + sizeof(uint16_t));
+    uint16_t net_len = htons((uint16_t)(sizeof(src_key->keys) + BLENZ(src_metadata)
+                                        + TLS_CRYPT_V2_TAG_SIZE + sizeof(uint16_t)));
     hmac_ctx_t *hmac_ctx = server_key->hmac;
     hmac_ctx_reset(hmac_ctx);
     hmac_ctx_update(hmac_ctx, (void *)&net_len, sizeof(net_len));
@@ -391,7 +391,7 @@
     ASSERT(cipher_ctx_reset(cipher_ctx, tag));
 
     /* Overflow check (OpenSSL requires an extra block in the dst buffer) */
-    if (buf_forward_capacity(&work) < (sizeof(src_key->keys) + BLEN(src_metadata) + sizeof(net_len)
+    if (buf_forward_capacity(&work) < (sizeof(src_key->keys) + BLENZ(src_metadata) + sizeof(net_len)
                                        + cipher_ctx_block_size(cipher_ctx)))
     {
         msg(M_WARN, "ERROR: could not crypt: insufficient space in dst");
@@ -439,7 +439,7 @@
     uint16_t net_len = 0;
     const uint8_t *tag = BPTR(&wrapped_client_key);
 
-    if (BLEN(&wrapped_client_key) < sizeof(net_len))
+    if (BLENZ(&wrapped_client_key) < sizeof(net_len))
     {
         CRYPT_ERROR("failed to read length");
     }
@@ -589,7 +589,7 @@
     struct buffer wrapped_client_key = *buf;
     uint16_t net_len = 0;
 
-    if (BLEN(&wrapped_client_key) < sizeof(net_len))
+    if (BLENZ(&wrapped_client_key) < sizeof(net_len))
     {
         msg(D_TLS_ERRORS, "Can not read tls-crypt-v2 client key length");
         return false;
diff --git a/src/openvpn/vlan.c b/src/openvpn/vlan.c
index a6a6e93..d8f49c6 100644
--- a/src/openvpn/vlan.c
+++ b/src/openvpn/vlan.c
@@ -85,7 +85,7 @@
     uint16_t vid;
 
     /* assume untagged frame */
-    if (BLEN(buf) < sizeof(*ethhdr))
+    if (BLENZ(buf) < sizeof(*ethhdr))
     {
         goto drop;
     }
@@ -109,7 +109,7 @@
     }
 
     /* tagged frame */
-    if (BLEN(buf) < sizeof(*vlanhdr))
+    if (BLENZ(buf) < sizeof(*vlanhdr))
     {
         goto drop;
     }
@@ -184,7 +184,7 @@
     const struct openvpn_ethhdr *ethhdr;
     struct openvpn_8021qhdr *vlanhdr;
 
-    if (BLEN(buf) < sizeof(*ethhdr))
+    if (BLENZ(buf) < sizeof(*ethhdr))
     {
         goto drop;
     }
@@ -197,7 +197,7 @@
          */
 
         /* Frame too small for header type? */
-        if (BLEN(buf) < sizeof(*vlanhdr))
+        if (BLENZ(buf) < sizeof(*vlanhdr))
         {
             goto drop;
         }
@@ -263,7 +263,7 @@
     const struct openvpn_8021qhdr *vlanhdr;
     uint16_t vid;
 
-    if (BLEN(buf) < sizeof(struct openvpn_8021qhdr))
+    if (BLENZ(buf) < sizeof(struct openvpn_8021qhdr))
     {
         /* frame too small to be VLAN-tagged */
         return false;
diff --git a/tests/unit_tests/openvpn/test_buffer.c b/tests/unit_tests/openvpn/test_buffer.c
index 0cfb918..ab53131 100644
--- a/tests/unit_tests/openvpn/test_buffer.c
+++ b/tests/unit_tests/openvpn/test_buffer.c
@@ -49,9 +49,9 @@
 #define teststr2  "two"
 #define teststr3  "three"
 
-#define assert_buf_equals_str(buf, str)       \
-    assert_int_equal(BLEN(buf), strlen(str)); \
-    assert_memory_equal(BPTR(buf), str, BLEN(buf));
+#define assert_buf_equals_str(buf, str)        \
+    assert_int_equal(BLENZ(buf), strlen(str)); \
+    assert_memory_equal(BPTR(buf), str, BLENZ(buf));
 
 static void
 test_buffer_printf_catrunc(void **state)
diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c
index 12ddaba..de8f9fe 100644
--- a/tests/unit_tests/openvpn/test_crypto.c
+++ b/tests/unit_tests/openvpn/test_crypto.c
@@ -70,7 +70,7 @@
 
     assert_true(crypto_pem_decode("TESTKEYNAME", &dec_buf, &pem_buf));
     assert_int_equal(BLEN(&src_buf), BLEN(&dec_buf));
-    assert_memory_equal(BPTR(&src_buf), BPTR(&dec_buf), BLEN(&src_buf));
+    assert_memory_equal(BPTR(&src_buf), BPTR(&dec_buf), BLENZ(&src_buf));
 
     gc_free(&gc);
 }
diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c
index b08e4c2..5bf1562 100644
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ b/tests/unit_tests/openvpn/test_pkt.c
@@ -549,7 +549,7 @@
     struct buffer buf2 =
         tls_reset_standalone(&tas.tls_wrap, &tas, &client_id, &server_id, header, false);
     assert_int_equal(BLEN(&buf), BLEN(&buf2));
-    assert_memory_equal(BPTR(&buf), BPTR(&buf2), BLEN(&buf));
+    assert_memory_equal(BPTR(&buf), BPTR(&buf2), BLENZ(&buf));
 
     free_tls_pre_decrypt_state(&state);
     free_buf(&tas.workbuf);
@@ -586,7 +586,7 @@
     struct buffer buf2 = tls_reset_standalone(&tas_client.tls_wrap, &tas_client, &client_id,
                                               &server_id, header, false);
     assert_int_equal(BLEN(&buf), BLEN(&buf2));
-    assert_memory_equal(BPTR(&buf), BPTR(&buf2), BLEN(&buf));
+    assert_memory_equal(BPTR(&buf), BPTR(&buf2), BLENZ(&buf));
 
     free_tls_pre_decrypt_state(&state);
 
diff --git a/tests/unit_tests/openvpn/test_ssl.c b/tests/unit_tests/openvpn/test_ssl.c
index 7bf5396..cfa30a8 100644
--- a/tests/unit_tests/openvpn/test_ssl.c
+++ b/tests/unit_tests/openvpn/test_ssl.c
@@ -344,9 +344,9 @@
 
         /* copy source to input buf */
         buf = work;
-        buf_p = buf_write_alloc(&buf, BLEN(&src));
+        buf_p = buf_write_alloc(&buf, BLENZ(&src));
         ASSERT(buf_p);
-        memcpy(buf_p, BPTR(&src), BLEN(&src));
+        memcpy(buf_p, BPTR(&src), BLENZ(&src));
 
         /* initialize work buffer with buf.headroom bytes of prepend capacity */
         ASSERT(buf_init(&encrypt_workspace, frame.buf.headroom));
@@ -390,9 +390,9 @@
 
     /* copy source to input buf */
     buf = work;
-    buf_p = buf_write_alloc(&buf, BLEN(&src));
+    buf_p = buf_write_alloc(&buf, BLENZ(&src));
     ASSERT(buf_p);
-    memcpy(buf_p, BPTR(&src), BLEN(&src));
+    memcpy(buf_p, BPTR(&src), BLENZ(&src));
 
     ASSERT(buf_init(&encrypt_workspace, frame.buf.headroom));
     openvpn_encrypt(&buf, encrypt_workspace, co);
@@ -688,9 +688,9 @@
 
     /* copy source to input buf */
     buf = work;
-    buf_p = buf_write_alloc(&buf, BLEN(&src));
+    buf_p = buf_write_alloc(&buf, BLENZ(&src));
     ASSERT(buf_p);
-    memcpy(buf_p, BPTR(&src), BLEN(&src));
+    memcpy(buf_p, BPTR(&src), BLENZ(&src));
 
     /* initialize work buffer with buf.headroom bytes of prepend capacity */
     ASSERT(buf_init(&encrypt_workspace, frame.buf.headroom));
diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c b/tests/unit_tests/openvpn/test_tls_crypt.c
index e2b2e38..596f0e0 100644
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
@@ -225,7 +225,7 @@
     assert_true(BLEN(&ctx->source) < BLEN(&ctx->ciphertext));
     assert_true(tls_crypt_unwrap(&ctx->ciphertext, &ctx->unwrapped, &ctx->co));
     assert_int_equal(BLEN(&ctx->source), BLEN(&ctx->unwrapped));
-    assert_memory_equal(BPTR(&ctx->source), BPTR(&ctx->unwrapped), BLEN(&ctx->source));
+    assert_memory_equal(BPTR(&ctx->source), BPTR(&ctx->unwrapped), BLENZ(&ctx->source));
 }
 
 
@@ -259,7 +259,7 @@
                                       0x33, 0x7b, 0x9c, 0xfb, 0x56, 0xe1, 0xf1, 0x3a, 0x87, 0x0e,
                                       0x66, 0x47, 0xdf, 0xa1, 0x95, 0xc9, 0x2c, 0x17, 0xa0, 0x15,
                                       0xba, 0x49, 0x67, 0xa1, 0x1d, 0x55, 0xea, 0x1a, 0x06, 0xa7 };
-    assert_memory_equal(BPTR(&rctx->work), expected_ciphertext, buf_len(&rctx->work));
+    assert_memory_equal(BPTR(&rctx->work), expected_ciphertext, BLENZ(&rctx->work));
     tls_wrap_free(&session.tls_wrap_reneg);
 
     /* Use previous tls-crypt key as 0x00, with xor we should have the same key
@@ -273,7 +273,7 @@
     tls_crypt_wrap(&ctx->source, &rctx->work, &rctx->opt);
     assert_int_equal(buf_len(&ctx->source) + 40, buf_len(&rctx->work));
 
-    assert_memory_equal(BPTR(&rctx->work), expected_ciphertext, buf_len(&rctx->work));
+    assert_memory_equal(BPTR(&rctx->work), expected_ciphertext, BLENZ(&rctx->work));
     tls_wrap_free(&session.tls_wrap_reneg);
 
     /* XOR should not force a different key */
@@ -289,7 +289,7 @@
 
     /* Skip packet id */
     buf_advance(&rctx->work, 8);
-    assert_memory_not_equal(BPTR(&rctx->work), expected_ciphertext, buf_len(&rctx->work));
+    assert_memory_not_equal(BPTR(&rctx->work), expected_ciphertext, BLENZ(&rctx->work));
     tls_wrap_free(&session.tls_wrap_reneg);
 
 
@@ -312,7 +312,7 @@
     assert_true(BLEN(&ctx->source) < BLEN(&ctx->ciphertext));
     assert_true(tls_crypt_unwrap(&ctx->ciphertext, &ctx->unwrapped, &ctx->co));
     assert_int_equal(BLEN(&ctx->source), BLEN(&ctx->unwrapped));
-    assert_memory_equal(BPTR(&ctx->source), BPTR(&ctx->unwrapped), BLEN(&ctx->source));
+    assert_memory_equal(BPTR(&ctx->source), BPTR(&ctx->unwrapped), BLENZ(&ctx->source));
 }
 
 /**
@@ -333,7 +333,7 @@
     assert_true(BLEN(&ctx->source) < BLEN(&ctx->ciphertext));
     assert_true(tls_crypt_unwrap(&ctx->ciphertext, &ctx->unwrapped, &ctx->co));
     assert_int_equal(BLEN(&ctx->source), BLEN(&ctx->unwrapped));
-    assert_memory_equal(BPTR(&ctx->source), BPTR(&ctx->unwrapped), BLEN(&ctx->source));
+    assert_memory_equal(BPTR(&ctx->source), BPTR(&ctx->unwrapped), BLENZ(&ctx->source));
 }
 
 /**

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1132?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I4e75ba1dbc6d9a0f75298bc900f713b67e60d096
Gerrit-Change-Number: 1132
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange

[Openvpn-devel] [S] Change in openvpn[master]: Handle return type of EVP_MD_size

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1133?usp=email

to review the following change.


Change subject: Handle return type of EVP_MD_size
......................................................................

Handle return type of EVP_MD_size

Return type is int, but we often use it in contexts
where we expect size_t. So just cast it. Nothing else
to do really.

Change-Id: I22b93c807f1be99fab450708f686fce4aa6d5cef
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/crypto_openssl.c
M src/openvpn/ssl_verify_openssl.c
M src/openvpn/xkey_helper.c
3 files changed, 5 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/33/1133/1

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 4fb6393..2351bfd 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -1273,7 +1273,7 @@
 
     /* We need to make a copy of the key since the OSSL parameters
      * only reference it */
-    memcpy(ctx->key, key, EVP_MD_size(kt));
+    memcpy(ctx->key, key, (size_t)EVP_MD_size(kt));
 
     /* Lookup/setting of parameters in OpenSSL 3.0 are string based
      *
@@ -1282,7 +1282,7 @@
      * the constness away here.
      */
     ctx->params[0] = OSSL_PARAM_construct_utf8_string("digest", (char *)EVP_MD_get0_name(kt), 0);
-    ctx->params[1] = OSSL_PARAM_construct_octet_string("key", ctx->key, EVP_MD_size(kt));
+    ctx->params[1] = OSSL_PARAM_construct_octet_string("key", ctx->key, (size_t)EVP_MD_size(kt));
     ctx->params[2] = OSSL_PARAM_construct_end();
 
     if (!EVP_MAC_init(ctx->ctx, NULL, 0, ctx->params))
diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c
index 1d83dfe..22659aa 100644
--- a/src/openvpn/ssl_verify_openssl.c
+++ b/src/openvpn/ssl_verify_openssl.c
@@ -341,7 +341,7 @@
 x509_get_sha1_fingerprint(X509 *cert, struct gc_arena *gc)
 {
     const EVP_MD *sha1 = EVP_sha1();
-    struct buffer hash = alloc_buf_gc(EVP_MD_size(sha1), gc);
+    struct buffer hash = alloc_buf_gc((size_t)EVP_MD_size(sha1), gc);
     X509_digest(cert, EVP_sha1(), BPTR(&hash), NULL);
     ASSERT(buf_inc_len(&hash, EVP_MD_size(sha1)));
     return hash;
@@ -351,7 +351,7 @@
 x509_get_sha256_fingerprint(X509 *cert, struct gc_arena *gc)
 {
     const EVP_MD *sha256 = EVP_sha256();
-    struct buffer hash = alloc_buf_gc(EVP_MD_size(sha256), gc);
+    struct buffer hash = alloc_buf_gc((size_t)EVP_MD_size(sha256), gc);
     X509_digest(cert, EVP_sha256(), BPTR(&hash), NULL);
     ASSERT(buf_inc_len(&hash, EVP_MD_size(sha256)));
     return hash;
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
index 3820808..9541a7c 100644
--- a/src/openvpn/xkey_helper.c
+++ b/src/openvpn/xkey_helper.c
@@ -351,7 +351,7 @@
         }
     }
 
-    if (tbslen != EVP_MD_size(EVP_get_digestbyname(mdname)))
+    if (tbslen != (size_t)EVP_MD_size(EVP_get_digestbyname(mdname)))
     {
         msg(M_WARN, "Error: encode_pkcs11: invalid input length <%zu>", tbslen);
         goto done;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1133?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I22b93c807f1be99fab450708f686fce4aa6d5cef
Gerrit-Change-Number: 1133
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newchange

[Openvpn-devel] [L] Change in openvpn[master]: PUSH_UPDATE message sender: enabling the server to send PUSH_UPDATE c...

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, mrbff, plaisthos, stipa.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/869?usp=email )

Change subject: PUSH_UPDATE message sender: enabling the server to send PUSH_UPDATE control messages
......................................................................


Patch Set 21: Code-Review-1

(3 comments)

Patchset:

PS21: 
getting there


File doc/management-notes.txt:

http://gerrit.openvpn.net/c/openvpn/+/869/comment/6ddb1ed1_95abe02b :
PS16, Line 1075: 
> i mostly followed the `kill` command, but yes it is the second one.
So we discussed this at the community meeting today, and we should go for "do only what people really need" (= testing, maintenance) - so, `push-update-cid` and `push-update-broad` should stay, and `cn` and `addr` are left out.


File src/openvpn/push.h:

http://gerrit.openvpn.net/c/openvpn/+/869/comment/8416a3dd_3e281a0d :
PS16, Line 54: 
> you can get the cid only from management, but theoretically the rest could be used by other function […]
As we have no other communication channels in OpenVPN 2.x today that can asynchronously decide "we need new options at a client now!" (plugins or scripts are only called on connect, and not "while a client is connected") this first patch should be "management only".  So please make the `#ifdef ENABLE_MANAGEMENT` thing wider, and include all of the new code and data structures.



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/869?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ie82bcc7a8e583de9156b185d71d1a323ed8df3fc
Gerrit-Change-Number: 869
Gerrit-PatchSet: 21
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: mrbff <ma...@ma...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-Comment-Date: Wed, 06 Aug 2025 14:09:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: cron2 <ge...@gr...>
Comment-In-Reply-To: mrbff <ma...@ma...>
Gerrit-MessageType: comment

[Openvpn-devel] [M] Change in openvpn[master]: Check message id/acked ids too when doing sessionid cookie checks

[plaisthos (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld.

plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1067?usp=email )

Change subject: Check message id/acked ids too when doing sessionid cookie checks
......................................................................


Patch Set 5:

(1 comment)

File tests/unit_tests/openvpn/test_pkt.c:

http://gerrit.openvpn.net/c/openvpn/+/1067/comment/95234b5f_2bb8c6f6 :
PS3, Line 533: 
> Looking at the LeakSanitizer failure and the other test cases I think there is a `free_tls_pre_decry […]
Done



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1067?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6752dcd5aff3e5cea2b439366479e86751a1c403
Gerrit-Change-Number: 1067
Gerrit-PatchSet: 5
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Wed, 06 Aug 2025 12:25:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld <fr...@li...>
Gerrit-MessageType: comment

[Openvpn-devel] [M] Change in openvpn[master]: Check message id/acked ids too when doing sessionid cookie checks

[plaisthos (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

Hello flichtenheld, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1067?usp=email

to look at the new patch set (#5).


Change subject: Check message id/acked ids too when doing sessionid cookie checks
......................................................................

Check message id/acked ids too when doing sessionid cookie checks

This fixes that control packets on a floating client can trigger
creating a new session in special circumstances:

To trigger this circumstance a connection needs to

- starts on IP A
- successfully floats to IP B by data packet
- then has a control packet from IP A before any
  data packet can trigger the float back to IP A

and all of this needs to happen in the 60s time
that hmac cookie is valid in the default
configuration.

In this scenario we would trigger a new connection as the HMAC
session id would be valid.

This patch adds checking also of the message-id and acked ids to
discern packet from the initial three-way handshake where these
ids 0 or 1 from any later packet.

This will now trigger (at verb 4 or higher) a messaged like:

   Packet (P_ACK_V1) with invalid or missing SID

instead.

Also remove a few duplicated free_tls_pre_decrypt_state in test_ssl.

Reported-By: Walter Doekes <wal...@wj...>
Tested-By: Walter Doekes <wal...@wj...>

Change-Id: I6752dcd5aff3e5cea2b439366479e86751a1c403
Signed-off-by: Arne Schwabe <ar...@rf...>
---
M src/openvpn/mudp.c
M src/openvpn/ssl_pkt.c
M src/openvpn/ssl_pkt.h
M tests/unit_tests/openvpn/test_pkt.c
4 files changed, 126 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/67/1067/5

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 7259a4b..0f22821 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -151,7 +151,8 @@
          * need to contain the peer id */
         struct gc_arena gc = gc_new();
 
-        bool ret = check_session_id_hmac(state, from, hmac, handwindow);
+        bool pkt_is_ack = (verdict == VERDICT_VALID_ACK_V1);
+        bool ret = check_session_id_hmac(state, from, hmac, handwindow, pkt_is_ack);
 
         const char *peer = print_link_socket_actual(&m->top.c2.from, &gc);
         uint8_t pkt_firstbyte = *BPTR(&m->top.c2.buf);
diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
index b901f87..ddb3a15 100644
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -496,8 +496,11 @@
 }
 
 bool
-check_session_id_hmac(struct tls_pre_decrypt_state *state, const struct openvpn_sockaddr *from,
-                      hmac_ctx_t *hmac, int handwindow)
+check_session_id_hmac(struct tls_pre_decrypt_state *state,
+                      const struct openvpn_sockaddr *from,
+                      hmac_ctx_t *hmac,
+                      int handwindow,
+                      bool pkt_is_ack)
 {
     if (!from)
     {
@@ -512,6 +515,36 @@
         return false;
     }
 
+    /* Check if the packet ID of the packet or ACKED packet  is <= 1 */
+    for (int i = 0; i < ack.len; i++)
+    {
+        /* This packet ACKs a packet that has a higher packet id than the
+         * ones expected in the three-way handshake, consider it as invalid
+         * for the session */
+        if (ack.packet_id[i] > 1)
+        {
+            return false;
+        }
+    }
+
+    if (!pkt_is_ack)
+    {
+        packet_id_type message_id;
+        /* Extract the packet ID from the packet */
+        if (!reliable_ack_read_packet_id(&buf, &message_id))
+        {
+            return false;
+        }
+
+        /* similar check. Anything larger than 1 is not considered part of the
+         * three-way handshake */
+        if (message_id > 1)
+        {
+            return false;
+        }
+    }
+
+
     /* check adjacent timestamps too */
     for (int offset = -2; offset <= 1; offset++)
     {
diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h
index 8fe4880..2933109 100644
--- a/src/openvpn/ssl_pkt.h
+++ b/src/openvpn/ssl_pkt.h
@@ -178,14 +178,20 @@
 /**
  * Checks if a control packet has a correct HMAC server session id
  *
+ * This will also consider packets that have a packet id higher
+ * than 1 or ack packets higher than 1 to be invalid as they are
+ * not part of the initial three way handshake of OpenVPN and should
+ * not create a new connection.
+ *
  * @param state         session information
  * @param from          link_socket from the client
  * @param hmac          the hmac context to use for the calculation
  * @param handwindow    the quantisation of the current time
+ * @param pkt_is_ack    the packet being checked is a P_ACK_V1
  * @return              the expected server session id
  */
 bool check_session_id_hmac(struct tls_pre_decrypt_state *state, const struct openvpn_sockaddr *from,
-                           hmac_ctx_t *hmac, int handwindow);
+                           hmac_ctx_t *hmac, int handwindow, bool pkt_is_ack);
 
 /*
  * Write a control channel authentication record.
diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c
index 65b31e7..a58e121 100644
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ b/tests/unit_tests/openvpn/test_pkt.c
@@ -139,6 +139,27 @@
                                               0xc8, 0x01, 0x00, 0x00, 0x00, 0x00, 0xdd, 0x85,
                                               0xdb, 0x53, 0x56, 0x23, 0xb0, 0x2e };
 
+/* no tls-auth, P_ACK_V1, acks 0,1, and 2 */
+const uint8_t client_ack_123_none_random_id[] = {
+    0x28,
+    0xae, 0xb9, 0xaf, 0xe1, 0xf0, 0x1d, 0x79, 0xc8,
+    0x03,
+    0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x01,
+    0x00, 0x00, 0x00, 0x02,
+    0xdd, 0x85, 0xdb, 0x53, 0x56, 0x23, 0xb0, 0x2e
+};
+
+/* no tls-auth, P_CONTROL_V1, acks 0, msg-id 2 */
+const uint8_t client_control_none_random_id[] = {
+    0x20,
+    0xae, 0xb9, 0xaf, 0xe1, 0xf0, 0x1d, 0x79, 0xc8,
+    0x01,
+    0x00, 0x00, 0x00, 0x00,
+    0x02
+};
+
+
 struct tls_auth_standalone
 init_tas_auth(int key_direction)
 {
@@ -256,12 +277,10 @@
     assert_int_equal(verdict, VERDICT_VALID_RESET_V2);
     free_tls_pre_decrypt_state(&state);
 
-    free_tls_pre_decrypt_state(&state);
     /* The pre decrypt function should not modify the buffer, so calling it
      * again should have the same result */
     verdict = tls_pre_decrypt_lite(&tas, &state, &from, &buf);
     assert_int_equal(verdict, VERDICT_VALID_RESET_V2);
-    free_tls_pre_decrypt_state(&state);
 
     /* and buf memory should be equal */
     assert_memory_equal(BPTR(&buf), client_reset_v2_tls_auth, sizeof(client_reset_v2_tls_auth));
@@ -279,7 +298,6 @@
     assert_int_equal(verdict, VERDICT_INVALID);
     free_tls_pre_decrypt_state(&state);
 
-    free_tls_pre_decrypt_state(&state);
     /* Wrong key direction gives a wrong hmac key and should not validate */
     free_key_ctx_bi(&tas.tls_wrap.opt.key_ctx_bi);
     free_tas(&tas);
@@ -319,15 +337,12 @@
     assert_int_equal(verdict, VERDICT_VALID_RESET_V2);
     free_tls_pre_decrypt_state(&state);
 
-    free_tls_pre_decrypt_state(&state);
     buf_reset_len(&buf);
     buf_write(&buf, client_reset_v2_tls_crypt, sizeof(client_reset_v2_none));
     verdict = tls_pre_decrypt_lite(&tas, &state, &from, &buf);
     assert_int_equal(verdict, VERDICT_VALID_RESET_V2);
     free_tls_pre_decrypt_state(&state);
 
-    free_tls_pre_decrypt_state(&state);
-
     /* This is not a reset packet and should trigger the other response */
     buf_reset_len(&buf);
     buf_write(&buf, client_ack_tls_auth_randomid, sizeof(client_ack_tls_auth_randomid));
@@ -405,7 +420,7 @@
     assert_int_equal(verdict, VERDICT_VALID_CONTROL_V1);
 
     /* This is a valid packet but containing a random id instead of an HMAC id*/
-    bool valid = check_session_id_hmac(&state, &from.dest, hmac, 30);
+    bool valid = check_session_id_hmac(&state, &from.dest, hmac, 30, false);
     assert_false(valid);
 
     free_tls_pre_decrypt_state(&state);
@@ -436,7 +451,7 @@
     verdict = tls_pre_decrypt_lite(&tas, &state, &from, &buf);
     assert_int_equal(verdict, VERDICT_VALID_ACK_V1);
 
-    bool valid = check_session_id_hmac(&state, &from.dest, hmac, 30);
+    bool valid = check_session_id_hmac(&state, &from.dest, hmac, 30, true);
     assert_true(valid);
 
     free_tls_pre_decrypt_state(&state);
@@ -445,6 +460,51 @@
     hmac_ctx_free(hmac);
 }
 
+static void
+test_verify_hmac_none_out_of_range_ack(void **ut_state)
+{
+    hmac_ctx_t *hmac = session_id_hmac_init();
+
+    struct link_socket_actual from = { 0 };
+    from.dest.addr.sa.sa_family = AF_INET;
+
+    struct tls_auth_standalone tas = { 0 };
+    struct tls_pre_decrypt_state state = { 0 };
+
+    struct buffer buf = alloc_buf(1024);
+    enum first_packet_verdict verdict;
+
+    tas.tls_wrap.mode = TLS_WRAP_NONE;
+
+    buf_reset_len(&buf);
+    buf_write(&buf, client_ack_123_none_random_id, sizeof(client_ack_123_none_random_id));
+
+
+    verdict = tls_pre_decrypt_lite(&tas, &state, &from, &buf);
+    assert_int_equal(verdict, VERDICT_VALID_ACK_V1);
+
+    /* should fail because it acks 2 */
+    bool valid = check_session_id_hmac(&state, &from.dest, hmac, 30, true);
+    assert_false(valid);
+    free_tls_pre_decrypt_state(&state);
+
+    /* Try test with the control with a too high message id now */
+    buf_reset_len(&buf);
+    buf_write(&buf, client_control_none_random_id, sizeof(client_control_none_random_id));
+
+    verdict = tls_pre_decrypt_lite(&tas, &state, &from, &buf);
+    assert_int_equal(verdict, VERDICT_VALID_CONTROL_V1);
+
+    /* should fail because it has message id 2 */
+    valid = check_session_id_hmac(&state, &from.dest, hmac, 30, true);
+    assert_false(valid);
+
+    free_tls_pre_decrypt_state(&state);
+    free_buf(&buf);
+    hmac_ctx_cleanup(hmac);
+    hmac_ctx_free(hmac);
+}
+
 static hmac_ctx_t *
 init_static_hmac(void)
 {
@@ -634,16 +694,20 @@
 main(void)
 {
     openvpn_unit_test_setup();
-    const struct CMUnitTest tests[] = { cmocka_unit_test(test_tls_decrypt_lite_none),
-                                        cmocka_unit_test(test_tls_decrypt_lite_auth),
-                                        cmocka_unit_test(test_tls_decrypt_lite_crypt),
-                                        cmocka_unit_test(test_parse_ack),
-                                        cmocka_unit_test(test_calc_session_id_hmac_static),
-                                        cmocka_unit_test(test_verify_hmac_none),
-                                        cmocka_unit_test(test_verify_hmac_tls_auth),
-                                        cmocka_unit_test(test_generate_reset_packet_plain),
-                                        cmocka_unit_test(test_generate_reset_packet_tls_auth),
-                                        cmocka_unit_test(test_extract_control_message) };
+
+    const struct CMUnitTest tests[] = {
+        cmocka_unit_test(test_tls_decrypt_lite_none),
+        cmocka_unit_test(test_tls_decrypt_lite_auth),
+        cmocka_unit_test(test_tls_decrypt_lite_crypt),
+        cmocka_unit_test(test_parse_ack),
+        cmocka_unit_test(test_calc_session_id_hmac_static),
+        cmocka_unit_test(test_verify_hmac_none),
+        cmocka_unit_test(test_verify_hmac_tls_auth),
+        cmocka_unit_test(test_verify_hmac_none_out_of_range_ack),
+        cmocka_unit_test(test_generate_reset_packet_plain),
+        cmocka_unit_test(test_generate_reset_packet_tls_auth),
+        cmocka_unit_test(test_extract_control_message)
+    };
 
 #if defined(ENABLE_CRYPTO_OPENSSL)
     OpenSSL_add_all_algorithms();

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1067?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6752dcd5aff3e5cea2b439366479e86751a1c403
Gerrit-Change-Number: 1067
Gerrit-PatchSet: 5
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newpatchset

Re: [Openvpn-devel] Modifying OpenVPN Source Code to Allow for Bulk-Reads, Max-MTU, and Jumbo-TCP

[Gert D. <ge...@gr...>]

Hi,

On Wed, Aug 06, 2025 at 07:46:53AM -0400, Jon Chiappetta via Openvpn-devel wrote:
> With my modifications made, I can specifically and directly target an exact
> 1500 byte MTU VPN link so that it correctly matches the LAN and WAN side of
> the networks which is really important network wise and I can also let TCP
> at the kernel level beautifully handle huge streams of bulk data and
> ordering and delivery on my behalf as efficiently as possible. This would
> be a pretty nice modification if I could make it through to the end.

This is *exactly* what OpenVPN will do out of the box, as of today - do
an interface MTU of 1500 (unless told otherwise), and if you do TCP, 
the TCP layer will handle the necessary segmentation.  If you do UDP,
you get outside fragmentation.

Here's a few tcpdumps in different config scenarios, demonstrating
the 1500 bytes "inside" just fine

  https://community.openvpn.net/MTU%20and%20Fragments

gert

-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
                             Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany                             ge...@gr...

Re: [Openvpn-devel] Modifying OpenVPN Source Code to Allow for Bulk-Reads, Max-MTU, and Jumbo-TCP

[Jon C. <ro...@fo...>]

Hi Gert,

Thank you for the documentation link, I will try to take a look at it when
I get some free time from work and life. I think my specific use case is a
bit different than other peoples. I think most people run VPN clients
directly on their client host machines which means that their host
operating system and network stack will honor a lower sized MTU directly
before sending network data outbound.

In my use case, I am piping my entire home WiFi network traffic through a
dedicated Linux VM running on a Mac Mini which can result in some serious
issues if I'm not careful enough. The reason why MTU size (1500 bytes) is
important to me is because all of my network clients will auto assume the
standard 1500 bytes once they join a network. This means that if my middle
network VPN link MTU is <1500, it will likely cause fragmentation issues
for my LAN clients which I really don't want performance wise. If the VPN
link MTU is >1500, then my ISP will fragment my packets at the router WAN
side as that is the network standard there also. In addition, the code base
is only performing singular read/write MTU-sized calls and sending them out
right away which also greatly impacts potential performance as well.

With my modifications made, I can specifically and directly target an exact
1500 byte MTU VPN link so that it correctly matches the LAN and WAN side of
the networks which is really important network wise and I can also let TCP
at the kernel level beautifully handle huge streams of bulk data and
ordering and delivery on my behalf as efficiently as possible. This would
be a pretty nice modification if I could make it through to the end.

This is one of the reasons why I dislike WireGuard personally. It's at the
kernel level instead of application level, it enforces a very small sized
MTUs potentially causing fragmentation and performance issues and its UDP
based only. :/

Thanks for your time on this,
Jon Chiappetta








On Wed, Aug 6, 2025 at 7:22 AM Gert Doering <ge...@gr...> wrote:

> Hi,
>
> On Wed, Aug 06, 2025 at 07:03:51AM -0400, Jon Chiappetta via Openvpn-devel
> wrote:
> > Is there any development setup guide I could follow in the future which
> > would allow me to submit a pull request to the OpenVPN codebase in an
> > official manner?
>
> https://community.openvpn.net/Development/DeveloperDocumentation
>
> which is outdated in a few places, but the most important thing is
> still correct: we don't do Github PRs, we do "patch on list" or "push
> to gerrit".
>
> Before you spend too much time on actual code, it makes sense to see if
> there is general agreement on the problem statement, and the direction
> to fix it.  For problems that we cannot understand, it's slightly harder
> to convince us to accept the code (and the maintenance and testing
> obligation that comes with each new line of code).
>
> So far I have heard "MTU mumble mumble" and we do have lots of MTUs of
> all sorts already :-)
>
> gert
> --
> "If was one thing all people took for granted, was conviction that if you
>  feed honest figures into a computer, honest figures come out. Never
> doubted
>  it myself till I met a computer with a sense of humor."
>                              Robert A. Heinlein, The Moon is a Harsh
> Mistress
>
> Gert Doering - Munich, Germany
> ge...@gr...
>

Re: [Openvpn-devel] Modifying OpenVPN Source Code to Allow for Bulk-Reads, Max-MTU, and Jumbo-TCP

[Gert D. <ge...@gr...>]

Hi,

On Wed, Aug 06, 2025 at 07:03:51AM -0400, Jon Chiappetta via Openvpn-devel wrote:
> Is there any development setup guide I could follow in the future which
> would allow me to submit a pull request to the OpenVPN codebase in an
> official manner?

https://community.openvpn.net/Development/DeveloperDocumentation

which is outdated in a few places, but the most important thing is
still correct: we don't do Github PRs, we do "patch on list" or "push
to gerrit".

Before you spend too much time on actual code, it makes sense to see if
there is general agreement on the problem statement, and the direction 
to fix it.  For problems that we cannot understand, it's slightly harder
to convince us to accept the code (and the maintenance and testing
obligation that comes with each new line of code).

So far I have heard "MTU mumble mumble" and we do have lots of MTUs of
all sorts already :-)

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
                             Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany                             ge...@gr...

Re: [Openvpn-devel] Modifying OpenVPN Source Code to Allow for Bulk-Reads, Max-MTU, and Jumbo-TCP

[Jon C. <ro...@fo...>]

Thank you for your reply Arne.

I didn't want to interrupt the regular OpenVPN development workflow process
here with some theoretical development questions. I still need to clean up
the code so that it would set and respect a user's option to turn this
feature on (1 client-side setting, multi server-side settings) and to also
not automatically override important variables/values/methods/functions
within the rest of the code base either (first time going through the
code). I also need to see if any better code optimizations could be made as
I'm not an expert on this code base like the others here may be.

Is there any development setup guide I could follow in the future which
would allow me to submit a pull request to the OpenVPN codebase in an
official manner?

Thanks again for your work and time,
Jon Chiappetta



On Wed, Aug 6, 2025 at 1:34 AM Arne Schwabe <ar...@rf...> wrote:

> Am 06.08.25 um 05:46 schrieb Jon Chiappetta via Openvpn-devel:
> > Hi,
> >
> > This is my first time posting to a mailing list. I worked on a possible
> > performance proof-of-concept modification to the OpenVPN source code.
> > The code is in rough shape as I just wrote it this long weekend but I
> > wanted to see if anyone had any interesting feedback for such work.
> >
> > Blog Post: https://fossjon.com/2025/08/05/modifying-openvpn-source-code-
> > to-allow-for-bulk-reads-max-mtu-and-jumbo-tcp-for-highly-improved-
> > performance/ <https://fossjon.com/2025/08/05/modifying-openvpn-source-
> > code-to-allow-for-bulk-reads-max-mtu-and-jumbo-tcp-for-highly-improved-
> > performance/>
> >
>
>
>  > Max MTU which now matches the rest of your network clients (1500 bytes)
>
> I have no idea what this means. OpenVPN always supported different MTU
> sizes and MTU 1500 is standard.
>
>  > Bulk reads from the tun interface (6 reads)
>  > TCP connection protocol only (single jumbo write transfers)
>  > Performance improvements now allows for (6 reads x 1500 bytes == 9000
> bytes)
>
>
> So basically, from your explanation the main thing is to try to
> write/read multiple packets in one batch on the TCP socket instead of
> just reading/writing a single packet.  I can see that this improves
> performance.
>
> Do you have any idea how much this actually improves performance? Your
> blog post shows a speedtest with 500 MBit/s but that is not actually
> saying much. In my own test setup, the current OpenVPN is capable over a
> Gigabit, a before/after from your test setup would nice to see.
>
> > POC Diff: https://github.com/stoops/openvpn/compare/main <https://
> > github.com/stoops/openvpn/compare/main>…mods
>
> Might be good enough for a POC but from quick glance it looks like there
> needs to cleanup make this an actual commit.
> >
> > If this is the wrong place I apologize in advance,
> > Thanks for your time,
>
> No, this is the right place.
>
> Arne
>

Re: [Openvpn-devel] Modifying OpenVPN Source Code to Allow for Bulk-Reads, Max-MTU, and Jumbo-TCP

[Arne S. <ar...@rf...>]

Am 06.08.25 um 05:46 schrieb Jon Chiappetta via Openvpn-devel:
> Hi,
> 
> This is my first time posting to a mailing list. I worked on a possible 
> performance proof-of-concept modification to the OpenVPN source code. 
> The code is in rough shape as I just wrote it this long weekend but I 
> wanted to see if anyone had any interesting feedback for such work.
> 
> Blog Post: https://fossjon.com/2025/08/05/modifying-openvpn-source-code- 
> to-allow-for-bulk-reads-max-mtu-and-jumbo-tcp-for-highly-improved- 
> performance/ <https://fossjon.com/2025/08/05/modifying-openvpn-source- 
> code-to-allow-for-bulk-reads-max-mtu-and-jumbo-tcp-for-highly-improved- 
> performance/>
> 


 > Max MTU which now matches the rest of your network clients (1500 bytes)

I have no idea what this means. OpenVPN always supported different MTU 
sizes and MTU 1500 is standard.

 > Bulk reads from the tun interface (6 reads)
 > TCP connection protocol only (single jumbo write transfers)
 > Performance improvements now allows for (6 reads x 1500 bytes == 9000 
bytes)


So basically, from your explanation the main thing is to try to 
write/read multiple packets in one batch on the TCP socket instead of 
just reading/writing a single packet.  I can see that this improves 
performance.

Do you have any idea how much this actually improves performance? Your 
blog post shows a speedtest with 500 MBit/s but that is not actually 
saying much. In my own test setup, the current OpenVPN is capable over a 
Gigabit, a before/after from your test setup would nice to see.

> POC Diff: https://github.com/stoops/openvpn/compare/main <https:// 
> github.com/stoops/openvpn/compare/main>…mods

Might be good enough for a POC but from quick glance it looks like there 
needs to cleanup make this an actual commit.
> 
> If this is the wrong place I apologize in advance,
> Thanks for your time,

No, this is the right place.

Arne

[Openvpn-devel] Modifying OpenVPN Source Code to Allow for Bulk-Reads, Max-MTU, and Jumbo-TCP

[Jon C. <ro...@fo...>]

Hi,

This is my first time posting to a mailing list. I worked on a possible
performance proof-of-concept modification to the OpenVPN source code. The
code is in rough shape as I just wrote it this long weekend but I wanted to
see if anyone had any interesting feedback for such work.

Blog Post:
https://fossjon.com/2025/08/05/modifying-openvpn-source-code-to-allow-for-bulk-reads-max-mtu-and-jumbo-tcp-for-highly-improved-performance/

POC Diff: https://github.com/stoops/openvpn/compare/main…mods

If this is the wrong place I apologize in advance,
Thanks for your time,

Jon Chiappetta

[Openvpn-devel] [M] Change in openvpn[master]: redirect-gateway: add route toward new remote host when using persist...

[mrbff (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

Hello flichtenheld, plaisthos, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/901?usp=email

to look at the new patch set (#2).


Change subject: redirect-gateway: add route toward new remote host when using persist-tun
......................................................................

redirect-gateway: add route toward new remote host when using persist-tun

When both --redirect-gateway and --persist-tun are used together,
a route must be added for each new remote host we attempt to connect to.

Change-Id: Ic7b486ccb85df7fc1d6a573ac1315d235728822c
Signed-off-by: Marco Baffo <ma...@ma...>
---
M src/openvpn/init.c
M src/openvpn/route.c
M src/openvpn/route.h
3 files changed, 187 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/01/901/2

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 40ae2c8..e79b777 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2101,6 +2101,10 @@
                         signal_description(c->sig->signal_received, c->sig->signal_text),
                         "route-pre-down", c->c2.es);
 
+            if (c->mode == CM_P2P && c->options.persist_tun)
+            {
+                del_route_towards_remote(c);
+            }
             delete_routes(c->c1.route_list, c->c1.route_ipv6_list, c->c1.tuntap,
                           ROUTE_OPTION_FLAGS(&c->options), c->c2.es, &c->net_ctx);
         }
@@ -2145,6 +2149,11 @@
                         c->c2.es);
         }
 
+        if (c->mode == CM_P2P && c->options.persist_tun)
+        {
+            del_route_towards_remote(c);
+        }
+
         del_wfp_block(c, adapter_index);
     }
     gc_free(&gc);
@@ -4714,6 +4723,17 @@
         }
     }
 
+    /**
+     * When using both --redirect-gateway and --persist-tun,
+     * if the connection to the server is lost, a /32 (or /128 if IPv6) route must be added
+     * to ensure connectivity to the next remote.
+     */
+    if (c->mode == CM_P2P
+        && c->options.persist_tun)
+    {
+        add_route_towards_remote(c);
+    }
+
     /*
      * Actually do UID/GID downgrade, and chroot, if requested.
      * May be delayed by --client, --pull, or --up-delay.
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index e504485..1f43d5a 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -42,6 +42,7 @@
 #include "options.h"
 #include "networking.h"
 #include "integer.h"
+#include "openvpn.h"
 
 #include "memdbg.h"
 
@@ -844,30 +845,14 @@
     }
 
     /* add VPN server host route if needed */
-    if (need_remote_ipv6_route)
+    if (need_remote_ipv6_route
+        && !(rl6->iflags & RL_DID_LOCAL))
     {
         if ((rl6->rgi6.flags & (RGI_ADDR_DEFINED | RGI_IFACE_DEFINED))
             == (RGI_ADDR_DEFINED | RGI_IFACE_DEFINED))
         {
-            struct route_ipv6 *r6;
-            ALLOC_OBJ_CLEAR_GC(r6, struct route_ipv6, &rl6->gc);
-
-            r6->network = *remote_host_ipv6;
-            r6->netbits = 128;
-            if (!(rl6->rgi6.flags & RGI_ON_LINK))
-            {
-                r6->gateway = rl6->rgi6.gateway.addr_ipv6;
-            }
-            r6->metric = 1;
-#ifdef _WIN32
-            r6->adapter_index = rl6->rgi6.adapter_index;
-#else
-            r6->iface = rl6->rgi6.iface;
-#endif
-            r6->flags = RT_DEFINED | RT_METRIC_DEFINED;
-
-            r6->next = rl6->routes_ipv6;
-            rl6->routes_ipv6 = r6;
+            rl6->routes_ipv6 = create_host_route_ipv6(*remote_host_ipv6, rl6);
+            rl6->iflags |= RL_DID_LOCAL;
         }
         else
         {
@@ -880,6 +865,30 @@
     return ret;
 }
 
+struct route_ipv6 *
+create_host_route_ipv6(struct in6_addr remote_host_ipv6, struct route_ipv6_list *rl6)
+{
+    struct route_ipv6 *r6;
+    ALLOC_OBJ_CLEAR_GC(r6, struct route_ipv6, &rl6->gc);
+
+    r6->network = remote_host_ipv6;
+    r6->netbits = 128;
+    if (!(rl6->rgi6.flags & RGI_ON_LINK))
+    {
+        r6->gateway = rl6->rgi6.gateway.addr_ipv6;
+    }
+    r6->metric = 1;
+#ifdef _WIN32
+    r6->adapter_index = rl6->rgi6.adapter_index;
+#else
+    r6->iface = rl6->rgi6.iface;
+#endif
+    r6->flags = RT_DEFINED | RT_METRIC_DEFINED;
+    r6->next = rl6->routes_ipv6;
+
+    return r6;
+}
+
 static bool
 add_route3(in_addr_t network, in_addr_t netmask, in_addr_t gateway, const struct tuntap *tt,
            unsigned int flags, const struct route_gateway_info *rgi, const struct env_set *es,
@@ -992,7 +1001,8 @@
                 /* if remote_host is not ipv4 (ie: ipv6), just skip
                  * adding this special /32 route */
                 if ((rl->spec.flags & RTSA_REMOTE_HOST)
-                    && rl->spec.remote_host != IPV4_INVALID_ADDR)
+                    && rl->spec.remote_host != IPV4_INVALID_ADDR
+                    && !(rl->iflags & RL_DID_LOCAL))
                 {
                     ret = add_route3(rl->spec.remote_host, IPV4_NETMASK_HOST, rl->rgi.gateway.addr,
                                      tt, flags | ROUTE_REF_GW, &rl->rgi, es, ctx);
@@ -1203,7 +1213,7 @@
         {
             delete_route_ipv6(r6, tt, es, ctx);
         }
-        rl6->iflags &= ~RL_ROUTES_ADDED;
+        rl6->iflags &= ~(RL_ROUTES_ADDED | RL_DID_LOCAL);
     }
 
     if (rl6)
@@ -1212,6 +1222,130 @@
     }
 }
 
+void
+add_route_towards_remote(struct context *c)
+{
+    ASSERT(c->c1.link_socket_addrs);
+
+    int current_remote_family = c->c1.link_socket_addrs[0].actual.dest.addr.sa.sa_family;
+
+    if (current_remote_family == AF_INET && c->c1.route_list
+        && (c->c1.route_list->flags & RG_REROUTE_GW))
+    {
+        if (c->c1.route_list->iflags & RL_DID_LOCAL)
+        {
+            del_route_towards_remote(c);
+        }
+
+        in_addr_t current_remote = ntohl(c->c1.link_socket_addrs[0].actual.dest.addr.in4.sin_addr.s_addr);
+
+        if (add_route3(current_remote,
+                       IPV4_NETMASK_HOST,
+                       c->c1.route_list->rgi.gateway.addr,
+                       c->c1.tuntap,
+                       ROUTE_OPTION_FLAGS(&c->options) | ROUTE_REF_GW,
+                       &c->c1.route_list->rgi,
+                       c->c2.es,
+                       &c->net_ctx))
+        {
+            c->c1.route_list->iflags |= RL_DID_LOCAL;
+            c->c1.route_list->spec.remote_host = current_remote;
+        }
+    }
+    else if (current_remote_family == AF_INET6 && c->c1.route_ipv6_list
+             && (c->c1.route_ipv6_list->flags & RG_REROUTE_GW))
+    {
+        if (c->c1.route_ipv6_list->iflags & RL_DID_LOCAL)
+        {
+            del_route_towards_remote(c);
+        }
+
+        const struct in6_addr *remote_host_ipv6 = &(c->c1.link_socket_addrs[0].actual.dest.addr.in6.sin6_addr);
+        struct route_ipv6_list *rl6 = c->c1.route_ipv6_list;
+
+        if ((rl6->rgi6.flags & (RGI_ADDR_DEFINED | RGI_IFACE_DEFINED)) == (RGI_ADDR_DEFINED | RGI_IFACE_DEFINED))
+        {
+            struct route_ipv6 *r6 = create_host_route_ipv6(*remote_host_ipv6, rl6);
+
+            if (add_route_ipv6(r6, c->c1.tuntap,
+                               ROUTE_OPTION_FLAGS(&c->options), c->c2.es, &c->net_ctx))
+            {
+                rl6->routes_ipv6 = r6;
+                rl6->iflags |= RL_DID_LOCAL;
+            }
+        }
+        else
+        {
+            msg(M_WARN, "ROUTE6: IPv6 route overlaps with IPv6 remote address, but could not determine IPv6 gateway address + interface, expect failure\n");
+        }
+    }
+}
+
+void
+del_route_towards_remote(struct context *c)
+{
+    /* If the function is called from do_close_tun() it means that the socket
+     * has already been closed and c->c2.link_sockets[0]->info.lsa` used in
+     * `add_route_towards_remote()` cleaned up. So we should use
+     * `c->c1.link_socket_addrs[0]` instead.
+     */
+    ASSERT(c->c1.link_socket_addrs);
+
+    int current_remote_family = 0;
+
+    if (c->c1.link_socket_addrs[0].actual.dest.addr.sa.sa_family)
+    {
+        current_remote_family = c->c1.link_socket_addrs[0].actual.dest.addr.sa.sa_family;
+    }
+    else if (c->c1.link_socket_addrs[0].current_remote)
+    {
+        current_remote_family = c->c1.link_socket_addrs[0].current_remote->ai_family;
+    }
+
+    if (current_remote_family == AF_INET && c->c1.route_list
+        && (c->c1.route_list->flags & RG_REROUTE_GW)
+        && (c->c1.route_list->iflags & RL_DID_LOCAL))
+    {
+        del_route3(c->c1.route_list->spec.remote_host,
+                   IPV4_NETMASK_HOST,
+                   c->c1.route_list->rgi.gateway.addr,
+                   c->c1.tuntap,
+                   ROUTE_OPTION_FLAGS(&c->options) | ROUTE_REF_GW,
+                   &c->c1.route_list->rgi,
+                   c->c2.es,
+                   &c->net_ctx);
+        c->c1.route_list->iflags &= ~RL_DID_LOCAL;
+    }
+    else if (current_remote_family == AF_INET6 && c->c1.route_ipv6_list
+             && (c->c1.route_ipv6_list->flags & RG_REROUTE_GW)
+             && (c->c1.route_ipv6_list->iflags & RL_DID_LOCAL))
+    {
+        struct in6_addr *remote_host_ipv6 = &c->c1.route_ipv6_list->remote_host_ipv6;
+        struct route_ipv6 *host_route = create_host_route_ipv6(*remote_host_ipv6, c->c1.route_ipv6_list);
+
+        for (struct route_ipv6 *prev = NULL, *r6 = c->c1.route_ipv6_list->routes_ipv6; r6; r6 = r6->next)
+        {
+            if (memcmp(&r6->network, &host_route->network, sizeof(struct in6_addr))
+                || r6->netbits != host_route->netbits
+                || memcmp(&r6->gateway, &host_route->gateway, sizeof(struct in6_addr)))
+            {
+                continue;
+            }
+
+            delete_route_ipv6(r6, c->c1.tuntap, c->c2.es, &c->net_ctx);
+            if (!prev)
+            {
+                c->c1.route_ipv6_list->routes_ipv6 = r6->next;
+            }
+            else
+            {
+                prev->next = r6->next;
+            }
+            c->c1.route_ipv6_list->iflags &= ~RL_DID_LOCAL;
+        }
+    }
+}
+
 #ifndef ENABLE_SMALL
 
 static const char *
diff --git a/src/openvpn/route.h b/src/openvpn/route.h
index ea8b767..642ec35 100644
--- a/src/openvpn/route.h
+++ b/src/openvpn/route.h
@@ -309,6 +309,14 @@
                const struct route_gateway_info *rgi, const struct env_set *es,
                openvpn_net_ctx_t *ctx);
 
+/* Function to add a route towards the new remote when using
+ * `--redirect-gateway` and `--persist-tun` options together. */
+void add_route_towards_remote(struct context *c);
+
+/* Function to delete the route towards the remote when using
+ * `--redirect-gateway` and `--persist-tun` options together. */
+void del_route_towards_remote(struct context *c);
+
 void add_route_to_option_list(struct route_option_list *l, const char *network, const char *netmask,
                               const char *gateway, const char *metric, int table_id);
 
@@ -359,6 +367,9 @@
 void print_default_gateway(const int msglevel, const struct route_gateway_info *rgi,
                            const struct route_ipv6_gateway_info *rgi6);
 
+/* Return a `route_ipv6 *` /128 IPv6 route toward the remote host */
+struct route_ipv6 *create_host_route_ipv6(struct in6_addr remote_host_ipv6, struct route_ipv6_list *rl6);
+
 /*
  * Test if addr is reachable via a local interface (return ILA_LOCAL),
  * or if it needs to be routed via the default gateway (return

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/901?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ic7b486ccb85df7fc1d6a573ac1315d235728822c
Gerrit-Change-Number: 901
Gerrit-PatchSet: 2
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [M] Change in openvpn[master]: Remove uncrustify config and reformat-all.sh

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/830?usp=email )

Change subject: Remove uncrustify config and reformat-all.sh
......................................................................

Remove uncrustify config and reformat-all.sh

Replaced with clang-format.

Change-Id: I15d4946800cbfaead67a73450ff3b12193814e54
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32543.html
Signed-off-by: Gert Doering <ge...@gr...>
---
D dev-tools/reformat-all.sh
D dev-tools/special-files.lst
D dev-tools/uncrustify.conf
3 files changed, 0 insertions(+), 219 deletions(-)




diff --git a/dev-tools/reformat-all.sh b/dev-tools/reformat-all.sh
deleted file mode 100755
index 0a31a83..0000000
--- a/dev-tools/reformat-all.sh
+++ /dev/null
@@ -1,135 +0,0 @@
-#!/bin/sh
-# reformat-all.sh - Reformat all git files in the checked out
-#                   git branch using uncrustify.
-#
-# Copyright (C) 2016-2025 - David Sommerseth <da...@op...>
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License
-# as published by the Free Software Foundation; either version 2
-# of the License.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, see <https://www.gnu.org/licenses/>.
-#
-
-tstamp="$(date +%Y%m%d-%H%M%S)"
-files="$(pwd)/reformat-all_files-$tstamp.lst"
-log="$(pwd)/reformat-all_log-$tstamp.txt"
-
-srcroot="$(git rev-parse --show-toplevel)"
-cfg="$srcroot/dev-tools/uncrustify.conf"
-specialfiles="$srcroot/dev-tools/special-files.lst"
-
-export gitfiles=0
-export procfiles=0
-
-# Go to the root of the source tree
-cd "$srcroot"
-
-{
-    echo -n "** Starting $0: "
-    date
-
-    # Find all C source/header files
-    git ls-files | grep -E ".*\.[ch](\.in$|$)" > "${files}.git"
-
-    # Manage files which needs special treatment
-    awk -F\# '{gsub("\n| ", "", $1); print $1}' "$specialfiles" > "${files}.sp"
-    while read srcfile
-    do
-        res=$(grep "$srcfile" "${files}.sp" 2>/dev/null)
-        if [ $? -ne 0 ]; then
-            # If grep didn't find the file among special files,
-            # process it normally
-            echo "$srcfile" >> "$files"
-        else
-            mode=$(echo "$res" | cut -d:  -f1)
-            case "$mode" in
-                E)
-                    echo "** INFO **  Excluding '$srcfile'"
-                    ;;
-                P)
-                    echo "** INFO **  Pre-patching '$srcfile'"
-                    patchfile="${srcroot}"/dev-tools/reformat-patches/before_$(echo "$srcfile" | tr "/" "_").patch
-                    if [ -r "$patchfile" ]; then
-                        git apply "$patchfile"
-                        if [ $? -ne 0 ]; then
-                            echo "** ERROR **  Failed to apply pre-patch file: $patchfile"
-                            exit 2
-                        fi
-                    else
-                        echo "** WARN ** Pre-patch file for $srcfile is missing: $patchfile"
-                    fi
-                    echo "$srcfile" >> "${files}.postpatch"
-                    echo "$srcfile" >> "$files"
-                    ;;
-                *)
-                    echo "** WARN ** Unknown mode '$mode' for file '$srcfile'"
-                    ;;
-            esac
-        fi
-    done < "${files}.git"
-    rm -f "${files}.git" "${files}.sp"
-
-    # Kick off uncrustify
-    echo
-    echo "** INFO ** Running: uncrustify -c $cfg --no-backup -l C -F $files"
-    uncrustify -c "$cfg" --no-backup -l C -F "$files" 2>&1
-    res=$?
-    echo "** INFO ** Uncrustify completed (exit code $res)"
-} | tee "${log}-1"  # Log needs to be closed here, to be processed in next block
-
-{
-    # Check the results
-    gitfiles=$(wc -l "$files" | cut -d\  -f1)
-    procfiles=$(grep "Parsing: " "${log}-1" | wc -l)
-    echo
-    echo "C source/header files checked into git: $gitfiles"
-    echo "Files processed by uncrustify:          $procfiles"
-    echo
-
-    # Post-Patch files modified after we uncrustify have adjusted them
-    if [ -r "${files}.postpatch" ]; then
-        while read srcfile;
-        do
-            patchfile="${srcroot}"/dev-tools/reformat-patches/after_$(echo "$srcfile" | tr "/" "_").patch
-            if [ -r "$patchfile" ]; then
-                echo "** INFO **  Post-patching '$srcfile'"
-                git apply "$patchfile"
-                if [ $? -ne 0 ]; then
-                    echo "** WARN ** Failed to apply $patchfile"
-                fi
-            else
-                echo "** WARN ** Post-patch file for $srcfile is missing: $patchfile"
-            fi
-        done < "${files}.postpatch"
-        rm -f "${files}.postpatch"
-    fi
-} | tee "${log}-2" # Log needs to be closed here, to be processed in next block
-
-cat "${log}-1" "${log}-2" > "$log"
-
-{
-    ec=1
-    echo
-    if [ "$gitfiles" -eq "$procfiles" ]; then
-        echo "Reformatting completed successfully"
-        ec=0
-    else
-        last=$(tail -n1 "${log}-1")
-        echo "** ERROR ** Reformating failed to process all files."
-        echo "            uncrustify exit code: $res"
-        echo "            Last log line: $last"
-        echo
-    fi
-    rm -f "${log}-1" "${log}-2"
-} | tee -a "$log"
-rm -f "${files}"
-
-exit $ec
diff --git a/dev-tools/special-files.lst b/dev-tools/special-files.lst
deleted file mode 100644
index e5f2fc2..0000000
--- a/dev-tools/special-files.lst
+++ /dev/null
@@ -1,5 +0,0 @@
-E:doc/doxygen/doc_key_generation.h     # @verbatim section gets mistreated, exclude it
-E:src/compat/compat-lz4.c              # Preserve LZ4 upstream formatting
-E:src/compat/compat-lz4.h              # Preserve LZ4 upstream formatting
-E:src/openvpn/ovpn_dco_linux.h         # Preserve ovpn-dco upstream formatting
-E:src/openvpn/ovpn_dco_win.h           # Preserve ovpn-dco-win upstream formatting
diff --git a/dev-tools/uncrustify.conf b/dev-tools/uncrustify.conf
deleted file mode 100644
index 325f310..0000000
--- a/dev-tools/uncrustify.conf
+++ /dev/null
@@ -1,79 +0,0 @@
-# Use Allman-style
-indent_columns=4
-indent_braces=false
-indent_else_if=false
-indent_switch_case=4
-indent_label=1
-nl_if_brace=add
-nl_brace_else=add
-nl_elseif_brace=add
-nl_else_brace=add
-nl_else_if=remove
-nl_for_brace=add
-nl_while_brace=add
-nl_switch_brace=add
-nl_fdef_brace=add
-nl_do_brace=add
-sp_func_proto_paren=Remove
-sp_func_def_paren=Remove
-sp_func_call_paren=Remove
-sp_sizeof_paren=Remove
-
-# No tabs, spaces only
-indent_with_tabs=0
-align_with_tabs=false
-cmt_convert_tab_to_spaces=true
-
-# Do not put spaces between the # and preprocessor statements
-pp_space=remove
-
-# Various whitespace fiddling
-sp_assign=add
-sp_before_sparen=add
-sp_inside_sparen=remove
-sp_cond_colon=add
-sp_cond_question=add
-sp_bool=add
-sp_else_brace=add
-sp_brace_else=add
-sp_after_comma=add
-pos_arith=Lead
-pos_bool=Lead
-nl_func_type_name=add
-nl_before_case=true
-nl_assign_leave_one_liners=true
-nl_enum_leave_one_liners=true
-nl_brace_fparen=add
-nl_max=4
-nl_after_func_proto=2
-nl_end_of_file_min=1
-nl_end_of_file=force
-
-# Always use scoping braces for conditionals
-mod_full_brace_if=add
-mod_full_brace_if_chain=false
-mod_full_brace_while=add
-mod_full_brace_for=add
-mod_full_brace_do=add
-
-# Annotate #else and #endif statements
-mod_add_long_ifdef_endif_comment=20
-mod_add_long_ifdef_else_comment=5
-
-# Misc cleanup
-mod_remove_extra_semicolon=true
-
-# leave blank at end of empty for() statements
-sp_after_semi_for_empty=Add
-
-# Use C-style comments (/* .. */)
-cmt_c_nl_end=true
-cmt_star_cont=true
-cmt_cpp_to_c=true
-
-# Use "char **a"-style pointer stars/dereferences
-sp_before_ptr_star=Add
-sp_between_ptr_star=Remove
-sp_after_ptr_star=Remove
-sp_before_byref=Add
-sp_after_byref=Remove

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/830?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I15d4946800cbfaead67a73450ff3b12193814e54
Gerrit-Change-Number: 830
Gerrit-PatchSet: 17
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: merged

[Openvpn-devel] [M] Change in openvpn[master]: Remove uncrustify config and reformat-all.sh

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#17) to the change originally created by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/830?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by cron2


Change subject: Remove uncrustify config and reformat-all.sh
......................................................................

Remove uncrustify config and reformat-all.sh

Replaced with clang-format.

Change-Id: I15d4946800cbfaead67a73450ff3b12193814e54
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32543.html
Signed-off-by: Gert Doering <ge...@gr...>
---
D dev-tools/reformat-all.sh
D dev-tools/special-files.lst
D dev-tools/uncrustify.conf
3 files changed, 0 insertions(+), 219 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/30/830/17

diff --git a/dev-tools/reformat-all.sh b/dev-tools/reformat-all.sh
deleted file mode 100755
index 0a31a83..0000000
--- a/dev-tools/reformat-all.sh
+++ /dev/null
@@ -1,135 +0,0 @@
-#!/bin/sh
-# reformat-all.sh - Reformat all git files in the checked out
-#                   git branch using uncrustify.
-#
-# Copyright (C) 2016-2025 - David Sommerseth <da...@op...>
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License
-# as published by the Free Software Foundation; either version 2
-# of the License.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, see <https://www.gnu.org/licenses/>.
-#
-
-tstamp="$(date +%Y%m%d-%H%M%S)"
-files="$(pwd)/reformat-all_files-$tstamp.lst"
-log="$(pwd)/reformat-all_log-$tstamp.txt"
-
-srcroot="$(git rev-parse --show-toplevel)"
-cfg="$srcroot/dev-tools/uncrustify.conf"
-specialfiles="$srcroot/dev-tools/special-files.lst"
-
-export gitfiles=0
-export procfiles=0
-
-# Go to the root of the source tree
-cd "$srcroot"
-
-{
-    echo -n "** Starting $0: "
-    date
-
-    # Find all C source/header files
-    git ls-files | grep -E ".*\.[ch](\.in$|$)" > "${files}.git"
-
-    # Manage files which needs special treatment
-    awk -F\# '{gsub("\n| ", "", $1); print $1}' "$specialfiles" > "${files}.sp"
-    while read srcfile
-    do
-        res=$(grep "$srcfile" "${files}.sp" 2>/dev/null)
-        if [ $? -ne 0 ]; then
-            # If grep didn't find the file among special files,
-            # process it normally
-            echo "$srcfile" >> "$files"
-        else
-            mode=$(echo "$res" | cut -d:  -f1)
-            case "$mode" in
-                E)
-                    echo "** INFO **  Excluding '$srcfile'"
-                    ;;
-                P)
-                    echo "** INFO **  Pre-patching '$srcfile'"
-                    patchfile="${srcroot}"/dev-tools/reformat-patches/before_$(echo "$srcfile" | tr "/" "_").patch
-                    if [ -r "$patchfile" ]; then
-                        git apply "$patchfile"
-                        if [ $? -ne 0 ]; then
-                            echo "** ERROR **  Failed to apply pre-patch file: $patchfile"
-                            exit 2
-                        fi
-                    else
-                        echo "** WARN ** Pre-patch file for $srcfile is missing: $patchfile"
-                    fi
-                    echo "$srcfile" >> "${files}.postpatch"
-                    echo "$srcfile" >> "$files"
-                    ;;
-                *)
-                    echo "** WARN ** Unknown mode '$mode' for file '$srcfile'"
-                    ;;
-            esac
-        fi
-    done < "${files}.git"
-    rm -f "${files}.git" "${files}.sp"
-
-    # Kick off uncrustify
-    echo
-    echo "** INFO ** Running: uncrustify -c $cfg --no-backup -l C -F $files"
-    uncrustify -c "$cfg" --no-backup -l C -F "$files" 2>&1
-    res=$?
-    echo "** INFO ** Uncrustify completed (exit code $res)"
-} | tee "${log}-1"  # Log needs to be closed here, to be processed in next block
-
-{
-    # Check the results
-    gitfiles=$(wc -l "$files" | cut -d\  -f1)
-    procfiles=$(grep "Parsing: " "${log}-1" | wc -l)
-    echo
-    echo "C source/header files checked into git: $gitfiles"
-    echo "Files processed by uncrustify:          $procfiles"
-    echo
-
-    # Post-Patch files modified after we uncrustify have adjusted them
-    if [ -r "${files}.postpatch" ]; then
-        while read srcfile;
-        do
-            patchfile="${srcroot}"/dev-tools/reformat-patches/after_$(echo "$srcfile" | tr "/" "_").patch
-            if [ -r "$patchfile" ]; then
-                echo "** INFO **  Post-patching '$srcfile'"
-                git apply "$patchfile"
-                if [ $? -ne 0 ]; then
-                    echo "** WARN ** Failed to apply $patchfile"
-                fi
-            else
-                echo "** WARN ** Post-patch file for $srcfile is missing: $patchfile"
-            fi
-        done < "${files}.postpatch"
-        rm -f "${files}.postpatch"
-    fi
-} | tee "${log}-2" # Log needs to be closed here, to be processed in next block
-
-cat "${log}-1" "${log}-2" > "$log"
-
-{
-    ec=1
-    echo
-    if [ "$gitfiles" -eq "$procfiles" ]; then
-        echo "Reformatting completed successfully"
-        ec=0
-    else
-        last=$(tail -n1 "${log}-1")
-        echo "** ERROR ** Reformating failed to process all files."
-        echo "            uncrustify exit code: $res"
-        echo "            Last log line: $last"
-        echo
-    fi
-    rm -f "${log}-1" "${log}-2"
-} | tee -a "$log"
-rm -f "${files}"
-
-exit $ec
diff --git a/dev-tools/special-files.lst b/dev-tools/special-files.lst
deleted file mode 100644
index e5f2fc2..0000000
--- a/dev-tools/special-files.lst
+++ /dev/null
@@ -1,5 +0,0 @@
-E:doc/doxygen/doc_key_generation.h     # @verbatim section gets mistreated, exclude it
-E:src/compat/compat-lz4.c              # Preserve LZ4 upstream formatting
-E:src/compat/compat-lz4.h              # Preserve LZ4 upstream formatting
-E:src/openvpn/ovpn_dco_linux.h         # Preserve ovpn-dco upstream formatting
-E:src/openvpn/ovpn_dco_win.h           # Preserve ovpn-dco-win upstream formatting
diff --git a/dev-tools/uncrustify.conf b/dev-tools/uncrustify.conf
deleted file mode 100644
index 325f310..0000000
--- a/dev-tools/uncrustify.conf
+++ /dev/null
@@ -1,79 +0,0 @@
-# Use Allman-style
-indent_columns=4
-indent_braces=false
-indent_else_if=false
-indent_switch_case=4
-indent_label=1
-nl_if_brace=add
-nl_brace_else=add
-nl_elseif_brace=add
-nl_else_brace=add
-nl_else_if=remove
-nl_for_brace=add
-nl_while_brace=add
-nl_switch_brace=add
-nl_fdef_brace=add
-nl_do_brace=add
-sp_func_proto_paren=Remove
-sp_func_def_paren=Remove
-sp_func_call_paren=Remove
-sp_sizeof_paren=Remove
-
-# No tabs, spaces only
-indent_with_tabs=0
-align_with_tabs=false
-cmt_convert_tab_to_spaces=true
-
-# Do not put spaces between the # and preprocessor statements
-pp_space=remove
-
-# Various whitespace fiddling
-sp_assign=add
-sp_before_sparen=add
-sp_inside_sparen=remove
-sp_cond_colon=add
-sp_cond_question=add
-sp_bool=add
-sp_else_brace=add
-sp_brace_else=add
-sp_after_comma=add
-pos_arith=Lead
-pos_bool=Lead
-nl_func_type_name=add
-nl_before_case=true
-nl_assign_leave_one_liners=true
-nl_enum_leave_one_liners=true
-nl_brace_fparen=add
-nl_max=4
-nl_after_func_proto=2
-nl_end_of_file_min=1
-nl_end_of_file=force
-
-# Always use scoping braces for conditionals
-mod_full_brace_if=add
-mod_full_brace_if_chain=false
-mod_full_brace_while=add
-mod_full_brace_for=add
-mod_full_brace_do=add
-
-# Annotate #else and #endif statements
-mod_add_long_ifdef_endif_comment=20
-mod_add_long_ifdef_else_comment=5
-
-# Misc cleanup
-mod_remove_extra_semicolon=true
-
-# leave blank at end of empty for() statements
-sp_after_semi_for_empty=Add
-
-# Use C-style comments (/* .. */)
-cmt_c_nl_end=true
-cmt_star_cont=true
-cmt_cpp_to_c=true
-
-# Use "char **a"-style pointer stars/dereferences
-sp_before_ptr_star=Add
-sp_between_ptr_star=Remove
-sp_after_ptr_star=Remove
-sp_before_byref=Add
-sp_after_byref=Remove

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/830?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I15d4946800cbfaead67a73450ff3b12193814e54
Gerrit-Change-Number: 830
Gerrit-PatchSet: 17
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [PATCH applied] Re: Remove uncrustify config and reformat-all.sh

[Gert D. <ge...@gr...>]

I discussed a while (mostly with myself) whether we want to keep this,
as it is a useful tool "for the next great reformatting, at least" - but
it's way too complicated for what we need to day (like, pre-patching
files to work around uncrustify bugs is what we didn't do in a long time).

For the record, the source tree can be reformatted just fine with this:

  clang-format --verbose -i \
	`find . -name "*.[ch]" -print | egrep -v 'ovpn_dco_(linux|win)\.h'`

Your patch has been applied to the master branch.

commit 75f9bd37ae94ad2e126f68276cee52fc8af3079f
Author: Frank Lichtenheld
Date:   Tue Aug 5 18:59:00 2025 +0200

     Remove uncrustify config and reformat-all.sh

     Signed-off-by: Frank Lichtenheld <fr...@li...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg32543.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH v16] Remove uncrustify config and reformat-all.sh

[Gert D. <ge...@gr...>]

From: Frank Lichtenheld <fr...@li...>

Replaced with clang-format.

Change-Id: I15d4946800cbfaead67a73450ff3b12193814e54
Signed-off-by: Frank Lichtenheld <fr...@li...>
Acked-by: Gert Doering <ge...@gr...>
---

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

Acked-by according to Gerrit (reflected above):
Gert Doering <ge...@gr...>

        
diff --git a/dev-tools/reformat-all.sh b/dev-tools/reformat-all.sh
deleted file mode 100755
index 0a31a83..0000000
--- a/dev-tools/reformat-all.sh
+++ /dev/null
@@ -1,135 +0,0 @@
-#!/bin/sh
-# reformat-all.sh - Reformat all git files in the checked out
-#                   git branch using uncrustify.
-#
-# Copyright (C) 2016-2025 - David Sommerseth <da...@op...>
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License
-# as published by the Free Software Foundation; either version 2
-# of the License.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, see <https://www.gnu.org/licenses/>.
-#
-
-tstamp="$(date +%Y%m%d-%H%M%S)"
-files="$(pwd)/reformat-all_files-$tstamp.lst"
-log="$(pwd)/reformat-all_log-$tstamp.txt"
-
-srcroot="$(git rev-parse --show-toplevel)"
-cfg="$srcroot/dev-tools/uncrustify.conf"
-specialfiles="$srcroot/dev-tools/special-files.lst"
-
-export gitfiles=0
-export procfiles=0
-
-# Go to the root of the source tree
-cd "$srcroot"
-
-{
-    echo -n "** Starting $0: "
-    date
-
-    # Find all C source/header files
-    git ls-files | grep -E ".*\.[ch](\.in$|$)" > "${files}.git"
-
-    # Manage files which needs special treatment
-    awk -F\# '{gsub("\n| ", "", $1); print $1}' "$specialfiles" > "${files}.sp"
-    while read srcfile
-    do
-        res=$(grep "$srcfile" "${files}.sp" 2>/dev/null)
-        if [ $? -ne 0 ]; then
-            # If grep didn't find the file among special files,
-            # process it normally
-            echo "$srcfile" >> "$files"
-        else
-            mode=$(echo "$res" | cut -d:  -f1)
-            case "$mode" in
-                E)
-                    echo "** INFO **  Excluding '$srcfile'"
-                    ;;
-                P)
-                    echo "** INFO **  Pre-patching '$srcfile'"
-                    patchfile="${srcroot}"/dev-tools/reformat-patches/before_$(echo "$srcfile" | tr "/" "_").patch
-                    if [ -r "$patchfile" ]; then
-                        git apply "$patchfile"
-                        if [ $? -ne 0 ]; then
-                            echo "** ERROR **  Failed to apply pre-patch file: $patchfile"
-                            exit 2
-                        fi
-                    else
-                        echo "** WARN ** Pre-patch file for $srcfile is missing: $patchfile"
-                    fi
-                    echo "$srcfile" >> "${files}.postpatch"
-                    echo "$srcfile" >> "$files"
-                    ;;
-                *)
-                    echo "** WARN ** Unknown mode '$mode' for file '$srcfile'"
-                    ;;
-            esac
-        fi
-    done < "${files}.git"
-    rm -f "${files}.git" "${files}.sp"
-
-    # Kick off uncrustify
-    echo
-    echo "** INFO ** Running: uncrustify -c $cfg --no-backup -l C -F $files"
-    uncrustify -c "$cfg" --no-backup -l C -F "$files" 2>&1
-    res=$?
-    echo "** INFO ** Uncrustify completed (exit code $res)"
-} | tee "${log}-1"  # Log needs to be closed here, to be processed in next block
-
-{
-    # Check the results
-    gitfiles=$(wc -l "$files" | cut -d\  -f1)
-    procfiles=$(grep "Parsing: " "${log}-1" | wc -l)
-    echo
-    echo "C source/header files checked into git: $gitfiles"
-    echo "Files processed by uncrustify:          $procfiles"
-    echo
-
-    # Post-Patch files modified after we uncrustify have adjusted them
-    if [ -r "${files}.postpatch" ]; then
-        while read srcfile;
-        do
-            patchfile="${srcroot}"/dev-tools/reformat-patches/after_$(echo "$srcfile" | tr "/" "_").patch
-            if [ -r "$patchfile" ]; then
-                echo "** INFO **  Post-patching '$srcfile'"
-                git apply "$patchfile"
-                if [ $? -ne 0 ]; then
-                    echo "** WARN ** Failed to apply $patchfile"
-                fi
-            else
-                echo "** WARN ** Post-patch file for $srcfile is missing: $patchfile"
-            fi
-        done < "${files}.postpatch"
-        rm -f "${files}.postpatch"
-    fi
-} | tee "${log}-2" # Log needs to be closed here, to be processed in next block
-
-cat "${log}-1" "${log}-2" > "$log"
-
-{
-    ec=1
-    echo
-    if [ "$gitfiles" -eq "$procfiles" ]; then
-        echo "Reformatting completed successfully"
-        ec=0
-    else
-        last=$(tail -n1 "${log}-1")
-        echo "** ERROR ** Reformating failed to process all files."
-        echo "            uncrustify exit code: $res"
-        echo "            Last log line: $last"
-        echo
-    fi
-    rm -f "${log}-1" "${log}-2"
-} | tee -a "$log"
-rm -f "${files}"
-
-exit $ec
diff --git a/dev-tools/special-files.lst b/dev-tools/special-files.lst
deleted file mode 100644
index e5f2fc2..0000000
--- a/dev-tools/special-files.lst
+++ /dev/null
@@ -1,5 +0,0 @@
-E:doc/doxygen/doc_key_generation.h     # @verbatim section gets mistreated, exclude it
-E:src/compat/compat-lz4.c              # Preserve LZ4 upstream formatting
-E:src/compat/compat-lz4.h              # Preserve LZ4 upstream formatting
-E:src/openvpn/ovpn_dco_linux.h         # Preserve ovpn-dco upstream formatting
-E:src/openvpn/ovpn_dco_win.h           # Preserve ovpn-dco-win upstream formatting
diff --git a/dev-tools/uncrustify.conf b/dev-tools/uncrustify.conf
deleted file mode 100644
index 325f310..0000000
--- a/dev-tools/uncrustify.conf
+++ /dev/null
@@ -1,79 +0,0 @@
-# Use Allman-style
-indent_columns=4
-indent_braces=false
-indent_else_if=false
-indent_switch_case=4
-indent_label=1
-nl_if_brace=add
-nl_brace_else=add
-nl_elseif_brace=add
-nl_else_brace=add
-nl_else_if=remove
-nl_for_brace=add
-nl_while_brace=add
-nl_switch_brace=add
-nl_fdef_brace=add
-nl_do_brace=add
-sp_func_proto_paren=Remove
-sp_func_def_paren=Remove
-sp_func_call_paren=Remove
-sp_sizeof_paren=Remove
-
-# No tabs, spaces only
-indent_with_tabs=0
-align_with_tabs=false
-cmt_convert_tab_to_spaces=true
-
-# Do not put spaces between the # and preprocessor statements
-pp_space=remove
-
-# Various whitespace fiddling
-sp_assign=add
-sp_before_sparen=add
-sp_inside_sparen=remove
-sp_cond_colon=add
-sp_cond_question=add
-sp_bool=add
-sp_else_brace=add
-sp_brace_else=add
-sp_after_comma=add
-pos_arith=Lead
-pos_bool=Lead
-nl_func_type_name=add
-nl_before_case=true
-nl_assign_leave_one_liners=true
-nl_enum_leave_one_liners=true
-nl_brace_fparen=add
-nl_max=4
-nl_after_func_proto=2
-nl_end_of_file_min=1
-nl_end_of_file=force
-
-# Always use scoping braces for conditionals
-mod_full_brace_if=add
-mod_full_brace_if_chain=false
-mod_full_brace_while=add
-mod_full_brace_for=add
-mod_full_brace_do=add
-
-# Annotate #else and #endif statements
-mod_add_long_ifdef_endif_comment=20
-mod_add_long_ifdef_else_comment=5
-
-# Misc cleanup
-mod_remove_extra_semicolon=true
-
-# leave blank at end of empty for() statements
-sp_after_semi_for_empty=Add
-
-# Use C-style comments (/* .. */)
-cmt_c_nl_end=true
-cmt_star_cont=true
-cmt_cpp_to_c=true
-
-# Use "char **a"-style pointer stars/dereferences
-sp_before_ptr_star=Add
-sp_between_ptr_star=Remove
-sp_after_ptr_star=Remove
-sp_before_byref=Add
-sp_after_byref=Remove

[Openvpn-devel] [M] Change in openvpn[master]: Remove uncrustify config and reformat-all.sh

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/830?usp=email )

Change subject: Remove uncrustify config and reformat-all.sh
......................................................................


Patch Set 16: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/830?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I15d4946800cbfaead67a73450ff3b12193814e54
Gerrit-Change-Number: 830
Gerrit-PatchSet: 16
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Tue, 05 Aug 2025 16:58:50 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: ssl_common: Make sure ssl flags are treated as unsigned

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1104?usp=email )

Change subject: ssl_common: Make sure ssl flags are treated as unsigned
......................................................................


Set Ready For Review


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1104?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I03e5ece7580ca4ebd41a7928ead544df46e8bad1
Gerrit-Change-Number: 1104
Gerrit-PatchSet: 4
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Comment-Date: Tue, 05 Aug 2025 16:02:31 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Openvpn-devel] [S] Change in openvpn[master]: route: Make sure various route flags are treated as unsigned

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1109?usp=email

to look at the new patch set (#4).


Change subject: route: Make sure various route flags are treated as unsigned
......................................................................

route: Make sure various route flags are treated as unsigned

The variables that hold them are already unsigned, make sure
the flags are as well to avoid spurious conversion warnings.

Change-Id: Ib7f78abbcd52c00a32afdea36ef635681ac8e127
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/route.h
1 file changed, 14 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/09/1109/4

diff --git a/src/openvpn/route.h b/src/openvpn/route.h
index ea8b767..9b6a47e 100644
--- a/src/openvpn/route.h
+++ b/src/openvpn/route.h
@@ -83,14 +83,14 @@
 };
 
 /* redirect-gateway flags */
-#define RG_ENABLE      (1 << 0)
-#define RG_LOCAL       (1 << 1)
-#define RG_DEF1        (1 << 2)
-#define RG_BYPASS_DHCP (1 << 3)
-#define RG_BYPASS_DNS  (1 << 4)
-#define RG_REROUTE_GW  (1 << 5)
-#define RG_AUTO_LOCAL  (1 << 6)
-#define RG_BLOCK_LOCAL (1 << 7)
+#define RG_ENABLE      (1u << 0)
+#define RG_LOCAL       (1u << 1)
+#define RG_DEF1        (1u << 2)
+#define RG_BYPASS_DHCP (1u << 3)
+#define RG_BYPASS_DNS  (1u << 4)
+#define RG_REROUTE_GW  (1u << 5)
+#define RG_AUTO_LOCAL  (1u << 6)
+#define RG_BLOCK_LOCAL (1u << 7)
 
 struct route_option_list
 {
@@ -117,9 +117,9 @@
 
 struct route_ipv4
 {
-#define RT_DEFINED        (1 << 0)
-#define RT_ADDED          (1 << 1)
-#define RT_METRIC_DEFINED (1 << 2)
+#define RT_DEFINED        (1u << 0)
+#define RT_ADDED          (1u << 1)
+#define RT_METRIC_DEFINED (1u << 2)
     struct route_ipv4 *next;
     unsigned int flags;
     const struct route_option *option;
@@ -227,9 +227,9 @@
 
 struct route_list
 {
-#define RL_DID_REDIRECT_DEFAULT_GATEWAY (1 << 0)
-#define RL_DID_LOCAL                    (1 << 1)
-#define RL_ROUTES_ADDED                 (1 << 2)
+#define RL_DID_REDIRECT_DEFAULT_GATEWAY (1u << 0)
+#define RL_DID_LOCAL                    (1u << 1)
+#define RL_ROUTES_ADDED                 (1u << 2)
     unsigned int iflags;
 
     struct route_special_addr spec;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1109?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ib7f78abbcd52c00a32afdea36ef635681ac8e127
Gerrit-Change-Number: 1109
Gerrit-PatchSet: 4
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newpatchset

[Openvpn-devel] [M] Change in openvpn[master]: list: Make types of hash elements consistent

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1119?usp=email

to look at the new patch set (#2).


Change subject: list: Make types of hash elements consistent
......................................................................

list: Make types of hash elements consistent

Really no use in having the indices and limits in int.

Change-Id: I3334465738fb1fbf508dfd719b6a238b500cc0ae
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/integer.h
M src/openvpn/list.c
M src/openvpn/list.h
M src/openvpn/multi.c
M src/openvpn/multi.h
M src/openvpn/options.c
M src/openvpn/options.h
M tests/unit_tests/openvpn/test_misc.c
8 files changed, 62 insertions(+), 49 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/19/1119/2

diff --git a/src/openvpn/integer.h b/src/openvpn/integer.h
index b82379e..2a6f14a 100644
--- a/src/openvpn/integer.h
+++ b/src/openvpn/integer.h
@@ -135,6 +135,27 @@
     }
 }
 
+static inline unsigned int
+constrain_uint(unsigned int x, unsigned int min, unsigned int max)
+{
+    if (min > max)
+    {
+        return min;
+    }
+    if (x < min)
+    {
+        return min;
+    }
+    else if (x > max)
+    {
+        return max;
+    }
+    else
+    {
+        return x;
+    }
+}
+
 /*
  * Functions used for circular buffer index arithmetic.
  */
diff --git a/src/openvpn/list.c b/src/openvpn/list.c
index f30d540..1b90d82 100644
--- a/src/openvpn/list.c
+++ b/src/openvpn/list.c
@@ -34,22 +34,21 @@
 #include "memdbg.h"
 
 struct hash *
-hash_init(const int n_buckets, const uint32_t iv,
+hash_init(const uint32_t n_buckets, const uint32_t iv,
           uint32_t (*hash_function)(const void *key, uint32_t iv),
           bool (*compare_function)(const void *key1, const void *key2))
 {
     struct hash *h;
-    int i;
 
     ASSERT(n_buckets > 0);
     ALLOC_OBJ_CLEAR(h, struct hash);
-    h->n_buckets = (int)adjust_power_of_2(n_buckets);
+    h->n_buckets = (uint32_t)adjust_power_of_2(n_buckets);
     h->mask = h->n_buckets - 1;
     h->hash_function = hash_function;
     h->compare_function = compare_function;
     h->iv = iv;
     ALLOC_ARRAY(h->buckets, struct hash_bucket, h->n_buckets);
-    for (i = 0; i < h->n_buckets; ++i)
+    for (uint32_t i = 0; i < h->n_buckets; ++i)
     {
         struct hash_bucket *b = &h->buckets[i];
         b->list = NULL;
@@ -60,8 +59,7 @@
 void
 hash_free(struct hash *hash)
 {
-    int i;
-    for (i = 0; i < hash->n_buckets; ++i)
+    for (uint32_t i = 0; i < hash->n_buckets; ++i)
     {
         struct hash_bucket *b = &hash->buckets[i];
         struct hash_element *he = b->list;
@@ -212,15 +210,15 @@
 }
 
 void
-hash_iterator_init_range(struct hash *hash, struct hash_iterator *hi, int start_bucket,
-                         int end_bucket)
+hash_iterator_init_range(struct hash *hash, struct hash_iterator *hi, uint32_t start_bucket,
+                         uint32_t end_bucket)
 {
     if (end_bucket > hash->n_buckets)
     {
         end_bucket = hash->n_buckets;
     }
 
-    ASSERT(start_bucket >= 0 && start_bucket <= end_bucket);
+    ASSERT(start_bucket <= end_bucket);
 
     hi->hash = hash;
     hi->elem = NULL;
@@ -325,6 +323,9 @@
  * the return value.  Every 1-bit and 2-bit delta achieves avalanche.
  * About 36+6len instructions.
  *
+ * #define hashsize(n) ((uint32_t)1<<(n))
+ * #define hashmask(n) (hashsize(n)-1)
+ *
  * The best hash table sizes are powers of 2.  There is no need to do
  * mod a prime (mod is sooo slow!).  If you need less than 32 bits,
  * use a bitmask.  For example, if you need only 10 bits, do
diff --git a/src/openvpn/list.h b/src/openvpn/list.h
index fb3302d..5702b95 100644
--- a/src/openvpn/list.h
+++ b/src/openvpn/list.h
@@ -36,14 +36,11 @@
 #include "basic.h"
 #include "buffer.h"
 
-#define hashsize(n) ((uint32_t)1 << (n))
-#define hashmask(n) (hashsize(n) - 1)
-
 struct hash_element
 {
     void *value;
     const void *key;
-    unsigned int hash_value;
+    uint32_t hash_value;
     struct hash_element *next;
 };
 
@@ -54,16 +51,16 @@
 
 struct hash
 {
-    int n_buckets;
-    int n_elements;
-    int mask;
+    uint32_t n_buckets;
+    uint32_t n_elements;
+    uint32_t mask;
     uint32_t iv;
     uint32_t (*hash_function)(const void *key, uint32_t iv);
     bool (*compare_function)(const void *key1, const void *key2); /* return true if equal */
     struct hash_bucket *buckets;
 };
 
-struct hash *hash_init(const int n_buckets, const uint32_t iv,
+struct hash *hash_init(const uint32_t n_buckets, const uint32_t iv,
                        uint32_t (*hash_function)(const void *key, uint32_t iv),
                        bool (*compare_function)(const void *key1, const void *key2));
 
@@ -81,17 +78,17 @@
 struct hash_iterator
 {
     struct hash *hash;
-    int bucket_index;
+    uint32_t bucket_index;
     struct hash_bucket *bucket;
     struct hash_element *elem;
     struct hash_element *last;
     bool bucket_marked;
-    int bucket_index_start;
-    int bucket_index_end;
+    uint32_t bucket_index_start;
+    uint32_t bucket_index_end;
 };
 
-void hash_iterator_init_range(struct hash *hash, struct hash_iterator *hi, int start_bucket,
-                              int end_bucket);
+void hash_iterator_init_range(struct hash *hash, struct hash_iterator *hi, uint32_t start_bucket,
+                              uint32_t end_bucket);
 
 void hash_iterator_init(struct hash *hash, struct hash_iterator *iter);
 
@@ -109,13 +106,13 @@
     return (*hash->hash_function)(key, hash->iv);
 }
 
-static inline int
+static inline uint32_t
 hash_n_elements(const struct hash *hash)
 {
     return hash->n_elements;
 }
 
-static inline int
+static inline uint32_t
 hash_n_buckets(const struct hash *hash)
 {
     return hash->n_buckets;
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 24de3ae..4aeedb9 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -169,18 +169,12 @@
 }
 
 static void
-multi_reap_range(const struct multi_context *m, int start_bucket, int end_bucket)
+multi_reap_range(const struct multi_context *m, uint32_t start_bucket, uint32_t end_bucket)
 {
     struct gc_arena gc = gc_new();
     struct hash_iterator hi;
     struct hash_element *he;
 
-    if (start_bucket < 0)
-    {
-        start_bucket = 0;
-        end_bucket = hash_n_buckets(m->vhash);
-    }
-
     dmsg(D_MULTI_DEBUG, "MULTI: REAP range %d -> %d", start_bucket, end_bucket);
     hash_iterator_init_range(m->vhash, &hi, start_bucket, end_bucket);
     while ((he = hash_iterator_next(&hi)) != NULL)
@@ -201,11 +195,11 @@
 static void
 multi_reap_all(const struct multi_context *m)
 {
-    multi_reap_range(m, -1, 0);
+    multi_reap_range(m, 0, hash_n_buckets(m->vhash));
 }
 
 static struct multi_reap *
-multi_reap_new(int buckets_per_pass)
+multi_reap_new(uint32_t buckets_per_pass)
 {
     struct multi_reap *mr;
     ALLOC_OBJ(mr, struct multi_reap);
@@ -237,10 +231,10 @@
 /*
  * How many buckets in vhash to reap per pass.
  */
-static int
-reap_buckets_per_pass(int n_buckets)
+static uint32_t
+reap_buckets_per_pass(uint32_t n_buckets)
 {
-    return constrain_int(n_buckets / REAP_DIVISOR, REAP_MIN, REAP_MAX);
+    return constrain_uint(n_buckets / REAP_DIVISOR, REAP_MIN, REAP_MAX);
 }
 
 #ifdef ENABLE_MANAGEMENT
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index e87e465..7c894d8 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -51,8 +51,8 @@
  */
 struct multi_reap
 {
-    int bucket_base;
-    int buckets_per_pass;
+    uint32_t bucket_base;
+    uint32_t buckets_per_pass;
     time_t last_call;
 };
 
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index e909691..d3db03b 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7945,11 +7945,11 @@
     }
     else if (streq(p[0], "hash-size") && p[1] && p[2] && !p[3])
     {
-        int real, virtual;
+        uint32_t real, virtual;
 
         VERIFY_PERMISSION(OPT_P_GENERAL);
-        real = atoi_warn(p[1], msglevel);
-        virtual = atoi_warn(p[2], msglevel);
+        real = positive_atoi(p[1], msglevel);
+        virtual = positive_atoi(p[2], msglevel);
         if (real < 1 || virtual < 1)
         {
             msg(msglevel, "--hash-size sizes must be >= 1 (preferably a power of 2)");
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index a04aa83..9a92fa9 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -499,8 +499,8 @@
     struct in6_addr ifconfig_ipv6_pool_base; /* IPv6 */
     int ifconfig_ipv6_pool_netbits;          /* IPv6 */
 
-    int real_hash_size;
-    int virtual_hash_size;
+    uint32_t real_hash_size;
+    uint32_t virtual_hash_size;
     const char *client_connect_script;
     const char *client_disconnect_script;
     const char *learn_address_script;
diff --git a/tests/unit_tests/openvpn/test_misc.c b/tests/unit_tests/openvpn/test_misc.c
index 01e1a5d..07e0880 100644
--- a/tests/unit_tests/openvpn/test_misc.c
+++ b/tests/unit_tests/openvpn/test_misc.c
@@ -128,7 +128,7 @@
 word_hash_function(const void *key, uint32_t iv)
 {
     const char *str = (const char *)key;
-    const int len = strlen(str);
+    const uint32_t len = (uint32_t)strlen(str);
     return hash_func((const uint8_t *)str, len, iv);
 }
 
@@ -138,11 +138,11 @@
     return strcmp((const char *)key1, (const char *)key2) == 0;
 }
 
-static unsigned long
+static uint32_t
 get_random(void)
 {
     /* rand() is not very random, but it's C99 and this is just for testing */
-    return rand();
+    return (uint32_t)rand();
 }
 
 static struct hash_element *
@@ -176,7 +176,7 @@
     struct hash *hash = hash_init(10000, get_random(), word_hash_function, word_compare_function);
     struct hash *nhash = hash_init(256, get_random(), word_hash_function, word_compare_function);
 
-    printf("hash_init n_buckets=%d mask=0x%08x\n", hash->n_buckets, hash->mask);
+    printf("hash_init n_buckets=%u mask=0x%08x\n", hash->n_buckets, hash->mask);
 
     char wordfile[PATH_MAX] = { 0 };
     openvpn_test_get_srcdir_dir(wordfile, PATH_MAX, "/../../../COPYRIGHT.GPL");
@@ -254,10 +254,10 @@
 
     /* output contents of hash table */
     {
-        ptr_type inc = 0;
+        uint32_t inc = 0;
         int count = 0;
 
-        for (ptr_type base = 0; base < hash_n_buckets(hash); base += inc)
+        for (uint32_t base = 0; base < hash_n_buckets(hash); base += inc)
         {
             struct hash_iterator hi;
             struct hash_element *he;

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1119?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I3334465738fb1fbf508dfd719b6a238b500cc0ae
Gerrit-Change-Number: 1119
Gerrit-PatchSet: 2
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-MessageType: newpatchset