add cipher test for buffer lengths

Use CHECK_XXX functions in cipher_drive, extend CHECK_XXX to improve testing.
cisco · Jul 7, 2024 · 3515b82 · 3515b82
1 parent 2eaf98b
commit 3515b82
Show file tree

Hide file tree

Showing 13 changed files with 305 additions and 82 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -386,7 +386,7 @@ if(LIBSRTP_TEST_APPS)
     target_link_libraries(datatypes_driver srtp3)
     add_test(datatypes_driver datatypes_driver -v)
 
-    add_executable(cipher_driver crypto/test/cipher_driver.c test/getopt_s.c)
+    add_executable(cipher_driver crypto/test/cipher_driver.c test/getopt_s.c test/util.c)
     target_set_warnings(
             TARGET
             cipher_driver

diff --git a/Makefile.in b/Makefile.in
@@ -229,7 +229,7 @@ test/roc_driver$(EXE): test/roc_driver.c test/ut_sim.c
 test/replay_driver$(EXE): test/replay_driver.c test/ut_sim.c
 	$(COMPILE) -I$(srcdir)/test $(LDFLAGS) -o $@ $^ $(LIBS) $(SRTPLIB)
 
-crypto/test/cipher_driver$(EXE): crypto/test/cipher_driver.c test/getopt_s.c
+crypto/test/cipher_driver$(EXE): crypto/test/cipher_driver.c test/getopt_s.c test/util.c
 	$(COMPILE) -I$(srcdir)/test $(LDFLAGS) -o $@ $^ $(LIBS) $(SRTPLIB)
 
 crypto/test/kernel_driver$(EXE): crypto/test/kernel_driver.c test/getopt_s.c

diff --git a/crypto/cipher/aes_gcm_mbedtls.c b/crypto/cipher/aes_gcm_mbedtls.c
@@ -290,7 +290,7 @@ static srtp_err_status_t srtp_aes_gcm_mbedtls_encrypt(void *cv,
     srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv;
     int errCode = 0;
 
-    if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) {
+    if (c->dir != srtp_direction_encrypt) {
         return srtp_err_status_bad_param;
     }
 
@@ -331,8 +331,12 @@ static srtp_err_status_t srtp_aes_gcm_mbedtls_decrypt(void *cv,
     srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv;
     int errCode = 0;
 
-    if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) {
-        return (srtp_err_status_bad_param);
+    if (c->dir != srtp_direction_decrypt) {
+        return srtp_err_status_bad_param;
+    }
+
+    if (src_len < c->tag_len) {
+        return srtp_err_status_bad_param;
     }
 
     if (*dst_len < (src_len - c->tag_len)) {
@@ -347,7 +351,7 @@ static srtp_err_status_t srtp_aes_gcm_mbedtls_decrypt(void *cv,
         src + (src_len - c->tag_len), c->tag_len, src, dst);
     c->aad_size = 0;
     if (errCode != 0) {
-        return (srtp_err_status_auth_fail);
+        return srtp_err_status_auth_fail;
     }
 
     /*
@@ -356,7 +360,7 @@ static srtp_err_status_t srtp_aes_gcm_mbedtls_decrypt(void *cv,
      */
     *dst_len = (src_len - c->tag_len);
 
-    return (srtp_err_status_ok);
+    return srtp_err_status_ok;
 }
 
 /*

diff --git a/crypto/cipher/aes_gcm_nss.c b/crypto/cipher/aes_gcm_nss.c
@@ -301,16 +301,36 @@ static srtp_err_status_t srtp_aes_gcm_nss_do_crypto(void *cv,
     SECItem param = { siBuffer, (unsigned char *)&c->params,
                       sizeof(CK_GCM_PARAMS) };
     if (encrypt) {
+        if (c->dir != srtp_direction_encrypt) {
+            return srtp_err_status_bad_param;
+        }
+
+        if (*dst_len < src_len + c->tag_size) {
+            return srtp_err_status_buffer_small;
+        }
+
         rv = PK11_Encrypt(c->key, CKM_AES_GCM, &param, dst, &out_len, *dst_len,
                           src, src_len);
     } else {
+        if (c->dir != srtp_direction_decrypt) {
+            return srtp_err_status_bad_param;
+        }
+
+        if (src_len < c->tag_size) {
+            return srtp_err_status_bad_param;
+        }
+
+        if (*dst_len < src_len - c->tag_size) {
+            return srtp_err_status_buffer_small;
+        }
+
         rv = PK11_Decrypt(c->key, CKM_AES_GCM, &param, dst, &out_len, *dst_len,
                           src, src_len);
     }
     *dst_len = out_len;
-    srtp_err_status_t status = (srtp_err_status_ok);
+    srtp_err_status_t status = srtp_err_status_ok;
     if (rv != SECSuccess) {
-        status = (srtp_err_status_cipher_fail);
+        status = srtp_err_status_cipher_fail;
     }
 
     return status;

diff --git a/crypto/cipher/aes_gcm_ossl.c b/crypto/cipher/aes_gcm_ossl.c
@@ -300,7 +300,7 @@ static srtp_err_status_t srtp_aes_gcm_openssl_encrypt(void *cv,
 {
     srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv;
 
-    if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) {
+    if (c->dir != srtp_direction_encrypt) {
         return srtp_err_status_bad_param;
     }
 
@@ -347,7 +347,11 @@ static srtp_err_status_t srtp_aes_gcm_openssl_decrypt(void *cv,
 {
     srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv;
 
-    if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) {
+    if (c->dir != srtp_direction_decrypt) {
+        return srtp_err_status_bad_param;
+    }
+
+    if (src_len < c->tag_len) {
         return srtp_err_status_bad_param;
     }
 

diff --git a/crypto/cipher/aes_gcm_wssl.c b/crypto/cipher/aes_gcm_wssl.c
@@ -330,7 +330,7 @@ static srtp_err_status_t srtp_aes_gcm_wolfssl_encrypt(void *cv,
     srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv;
     int err;
 
-    if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) {
+    if (c->dir != srtp_direction_encrypt) {
         return srtp_err_status_bad_param;
     }
 
@@ -385,7 +385,11 @@ static srtp_err_status_t srtp_aes_gcm_wolfssl_decrypt(void *cv,
     srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv;
     int err;
 
-    if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) {
+    if (c->dir != srtp_direction_decrypt) {
+        return srtp_err_status_bad_param;
+    }
+
+    if (src_len < c->tag_len) {
         return srtp_err_status_bad_param;
     }
 

diff --git a/crypto/cipher/aes_icm_nss.c b/crypto/cipher/aes_icm_nss.c
@@ -334,12 +334,20 @@ static srtp_err_status_t srtp_aes_icm_nss_encrypt(void *cv,
         return srtp_err_status_bad_param;
     }
 
+    if (dst_len == NULL) {
+        return srtp_err_status_bad_param;
+    }
+
+    if (*dst_len < src_len) {
+        return srtp_err_status_buffer_small;
+    }
+
     int out_len = 0;
     int rv = PK11_CipherOp(c->ctx, dst, &out_len, *dst_len, src, src_len);
     *dst_len = out_len;
-    srtp_err_status_t status = (srtp_err_status_ok);
+    srtp_err_status_t status = srtp_err_status_ok;
     if (rv != SECSuccess) {
-        status = (srtp_err_status_cipher_fail);
+        status = srtp_err_status_cipher_fail;
     }
 
     return status;

diff --git a/crypto/cipher/aes_icm_ossl.c b/crypto/cipher/aes_icm_ossl.c
@@ -308,6 +308,14 @@ static srtp_err_status_t srtp_aes_icm_openssl_encrypt(void *cv,
 
     debug_print(srtp_mod_aes_icm, "rs0: %s", v128_hex_string(&c->counter));
 
+    if (dst_len == NULL) {
+        return srtp_err_status_bad_param;
+    }
+
+    if (*dst_len < src_len) {
+        return srtp_err_status_buffer_small;
+    }
+
     if (!EVP_EncryptUpdate(c->ctx, dst, &len, src, src_len)) {
         return srtp_err_status_cipher_fail;
     }

diff --git a/crypto/cipher/aes_icm_wssl.c b/crypto/cipher/aes_icm_wssl.c
@@ -318,6 +318,14 @@ static srtp_err_status_t srtp_aes_icm_wolfssl_encrypt(void *cv,
     int err;
     debug_print(srtp_mod_aes_icm, "rs0: %s", v128_hex_string(&c->counter));
 
+    if (dst_len == NULL) {
+        return srtp_err_status_bad_param;
+    }
+
+    if (*dst_len < src_len) {
+        return srtp_err_status_buffer_small;
+    }
+
     err = wc_AesCtrEncrypt(c->ctx, dst, src, src_len);
     if (err < 0) {
         debug_print(srtp_mod_aes_icm, "wolfSSL encrypt error: %d", err);

diff --git a/crypto/include/aes_gcm.h b/crypto/include/aes_gcm.h
@@ -76,11 +76,11 @@ typedef struct {
 #include <wolfssl/wolfcrypt/aes.h>
 
 typedef struct {
-    int key_size;
-    int tag_len;
+    size_t key_size;
+    size_t tag_len;
 #ifndef WOLFSSL_AESGCM_STREAM
-    int aad_size;
-    int iv_len;
+    size_t aad_size;
+    size_t iv_len;
     uint8_t iv[GCM_NONCE_MID_SZ];
     uint8_t aad[MAX_AD_SIZE];
 #endif