[Openvpn-devel] crypto: respect ECB argument type from prototype

Message ID 20210428081054.29081-1-a@unstable.cc
State Accepted
Headers show
Series [Openvpn-devel] crypto: respect ECB argument type from prototype | expand

Commit Message

Antonio Quartulli April 27, 2021, 10:10 p.m. UTC
From: Antonio Quartulli <antonio@openvpn.net>

Crypto backends are implementing the cipher_des_encrypt_ecb()
function without fully respecting the type of the argumentis as described
in the function prototype.

All ECB arguments (key, input block and output block) are expected to
be 8 bytes long, for this reason the prototype specifies the arguments
as 3 arrays of 8 bytes in size.

Convert the implementations to also explicitly mention the size of the
array they expect to receive in input.

Fixes these warnings:

crypto_openssl.c:866:39: warning: argument 2 of type ‘unsigned char *’ declared as a pointer [-Warray-parameter=]
  866 |                        unsigned char *src,
      |                        ~~~~~~~~~~~~~~~^~~
In file included from crypto.h:125,
                 from crypto_openssl.c:42:
crypto_backend.h:202:43: note: previously declared as an array ‘unsigned char[8]’
  202 |                             unsigned char src[DES_KEY_LENGTH],
      |                             ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
crypto_openssl.c:867:39: warning: argument 3 of type ‘unsigned char *’ declared as a pointer [-Warray-parameter=]
  867 |                        unsigned char *dst)
      |                        ~~~~~~~~~~~~~~~^~~
In file included from crypto.h:125,
                 from crypto_openssl.c:42:
crypto_backend.h:203:43: note: previously declared as an array ‘unsigned char[8]’
  203 |                             unsigned char dst[DES_KEY_LENGTH]);
      |                             ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
---
 src/openvpn/crypto_mbedtls.c | 4 ++--
 src/openvpn/crypto_openssl.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Arne Schwabe April 28, 2021, 12:29 a.m. UTC | #1
Am 28.04.21 um 10:10 schrieb Antonio Quartulli:
> From: Antonio Quartulli <antonio@openvpn.net>
> 
> Crypto backends are implementing the cipher_des_encrypt_ecb()
> function without fully respecting the type of the argumentis as described
> in the function prototype.
> 
> All ECB arguments (key, input block and output block) are expected to
> be 8 bytes long, for this reason the prototype specifies the arguments
> as 3 arrays of 8 bytes in size.
> 
> Convert the implementations to also explicitly mention the size of the
> array they expect to receive in input.
>

Acked-By: Arne Schwabe <arne@rfc2549.org>

In case someone wonders. We use *single* DES in ECB mode for NTLM. And
this was found by GCC 11.

Arne
Gert Doering April 28, 2021, 2:19 a.m. UTC | #2
Interesting find from gcc11 :-) 

Your patch has been applied to the master branch.

commit 6a3cbb43a3d09dbf0b5df33c741455f34bde2440
Author: Antonio Quartulli
Date:   Wed Apr 28 10:10:54 2021 +0200

     crypto: respect ECB argument type from prototype

     Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20210428081054.29081-1-a@unstable.cc>
     URL: https://www.mail-archive.com/search?l=mid&q=20210428081054.29081-1-a@unstable.cc
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c
index f960d0ae..836e80ac 100644
--- a/src/openvpn/crypto_mbedtls.c
+++ b/src/openvpn/crypto_mbedtls.c
@@ -768,8 +768,8 @@  cipher_ctx_final_check_tag(mbedtls_cipher_context_t *ctx, uint8_t *dst,
 
 void
 cipher_des_encrypt_ecb(const unsigned char key[DES_KEY_LENGTH],
-                       unsigned char *src,
-                       unsigned char *dst)
+                       unsigned char src[DES_KEY_LENGTH],
+                       unsigned char dst[DES_KEY_LENGTH])
 {
     mbedtls_des_context ctx;
 
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index f8b36bf8..af7311ec 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -863,8 +863,8 @@  cipher_ctx_final_check_tag(EVP_CIPHER_CTX *ctx, uint8_t *dst, int *dst_len,
 
 void
 cipher_des_encrypt_ecb(const unsigned char key[DES_KEY_LENGTH],
-                       unsigned char *src,
-                       unsigned char *dst)
+                       unsigned char src[DES_KEY_LENGTH],
+                       unsigned char dst[DES_KEY_LENGTH])
 {
     DES_key_schedule sched;