Fix connection cookie not including address and fix endianness in test

We accidentially checked the adress family size instead of the address
family.

For  unit test checks we need to consider endianess to ensure the hmac
for the adress is always the same. The real code does not care about
endian since it only needs it to be same on the same architecture.

Converting the session to endianess is strictly speaking unecessary
for the actual function of the function but is almost no overhead
and makes the unit testing more robust.

Reported by David trying to the package on Red Hat/s390x and painfully
debugged by setting up a s390x qemu machine that takes 40s just to
run ./configure.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221206133647.954724-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25619.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 67bef0357280040b83f2185c91c4f830ba542d6b)

diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c
index 7891e10ee4d8db781822d578b806096d9e90a372..46bca21d86cf51dba101d4bb46badb841c9d4b6b 100644 (file)
--- a/src/openvpn/ssl_pkt.c
+++ b/src/openvpn/ssl_pkt.c
@@ -495,7 +495,7 @@ calculate_session_id_hmac(struct session_id client_sid,
     /* Get the valid time quantisation for our hmac,
      * we divide time by handwindow/2 and allow the previous
      * and future session time if specified by offset */
-    uint32_t session_id_time = now/((handwindow+1)/2) + offset;
+    uint32_t session_id_time = ntohl(now/((handwindow+1)/2) + offset);
 
     hmac_ctx_reset(hmac);
     /* We do not care about endian here since it does not need to be
@@ -504,7 +504,7 @@ calculate_session_id_hmac(struct session_id client_sid,
                     sizeof(session_id_time));
 
     /* add client IP and port */
-    switch (af_addr_size(from->addr.sa.sa_family))
+    switch (from->addr.sa.sa_family)
     {
         case AF_INET:
             hmac_ctx_update(hmac, (const uint8_t *) &from->addr.in4, sizeof(struct sockaddr_in));

diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c
index 2d771e30148e743235273832028298aabee0e9c5..f447e493a93c93f7c5fabc693b1eca6493557068 100644 (file)
--- a/tests/unit_tests/openvpn/test_pkt.c
+++ b/tests/unit_tests/openvpn/test_pkt.c
@@ -435,6 +435,8 @@ test_verify_hmac_none(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 };
 
@@ -463,7 +465,7 @@ init_static_hmac(void)
     ASSERT(md_valid("SHA256"));
     hmac_ctx_t *hmac_ctx = hmac_ctx_new();
 
-    uint8_t key[SHA256_DIGEST_LENGTH] = {1, 2, 3};
+    uint8_t key[SHA256_DIGEST_LENGTH] = {1, 2, 3, 0};
 
     hmac_ctx_init(hmac_ctx, key, "SHA256");
     return hmac_ctx;
@@ -475,22 +477,18 @@ test_calc_session_id_hmac_static(void **ut_state)
     hmac_ctx_t *hmac = init_static_hmac();
     static const int handwindow = 100;
 
-    struct openvpn_sockaddr addr = {0 };
+    struct openvpn_sockaddr addr = { 0 };
 
-    /* we do not use htons functions here since the hmac calculate function
-     * also does not care about the endianness of the data but just assumes
-     * the endianness doesn't change between calls */
     addr.addr.in4.sin_family = AF_INET;
-    addr.addr.in4.sin_addr.s_addr = 0xff000ff;
-    addr.addr.in4.sin_port = 1194;
-
+    addr.addr.in4.sin_addr.s_addr = ntohl(0xff000ff);
+    addr.addr.in4.sin_port = ntohs(1195);
 
     struct session_id client_id = { {0, 1, 2, 3, 4, 5, 6, 7}};
 
     now = 1005;
     struct session_id server_id = calculate_session_id_hmac(client_id, &addr, hmac, handwindow, 0);
 
-    struct session_id expected_server_id = { {0xba,  0x83, 0xa9, 0x00, 0x72, 0xbd, 0x93, 0xba }};
+    struct session_id expected_server_id = { {0x84, 0x73, 0x52, 0x2b, 0x5b, 0xa9, 0x2a, 0x70 }};
     assert_memory_equal(expected_server_id.id, server_id.id, SID_SIZE);
 
     struct session_id server_id_m1 = calculate_session_id_hmac(client_id, &addr, hmac, handwindow, -1);