Merge pull request #560 from cisco/feature/56cUts

Fixes to safeC stub, updating all changes from internal
cisco · Jan 23, 2021 · e90b70e · e90b70e
2 parents 2584d9e + 065714d
commit e90b70e
Show file tree

Hide file tree

Showing 14 changed files with 111 additions and 61 deletions.
diff --git a/app/app_kas.c b/app/app_kas.c
@@ -619,6 +619,7 @@ int app_kas_ifc_handler(ACVP_TEST_CASE *test_case) {
     return rv;
 }
 
+
 int app_kts_ifc_handler(ACVP_TEST_CASE *test_case) {
     if (!test_case) {
         return -1;

diff --git a/app/app_kas_kdf.c b/app/app_kas_kdf.c
@@ -300,6 +300,9 @@ int app_kas_kdf_onestep_handler(ACVP_TEST_CASE *test_case) {
     unsigned char *fixedInfo = NULL;
     unsigned char *h_output = NULL;
     unsigned char *result = NULL;
+#if OPENSSL_VERSION_NUMBER <= 0x10100000L
+    HMAC_CTX static_ctx;
+#endif
     HMAC_CTX *hmac_ctx = NULL;
     EVP_MD_CTX *sha_ctx = NULL;
 
@@ -316,7 +319,6 @@ int app_kas_kdf_onestep_handler(ACVP_TEST_CASE *test_case) {
     //if the test case has a salt, we are using HMAC, otherwise, SHA
     if (stc->salt) {
 #if OPENSSL_VERSION_NUMBER <= 0x10100000L
-        HMAC_CTX static_ctx;
         hmac_ctx = &static_ctx;
         HMAC_CTX_init(hmac_ctx);
 #else

diff --git a/app/ketopt.h b/app/ketopt.h
@@ -12,7 +12,7 @@
 #define ko_required_argument 1
 #define ko_optional_argument 2
 
-#define OPTION_NAME_MAX 16
+#define OPTION_NAME_MAX 128
 #define OSTR_MAX 2 /* Change according to the ostr parameter in app_cli.c */
 
 typedef struct {

diff --git a/ms/resources/Source.def b/ms/resources/Source.def
@@ -102,9 +102,9 @@ EXPORTS
   acvp_cleanup
   acvp_version
   acvp_protocol_version
-  acvp_kas_kdf_enable
-  acvp_kas_kdf_set_parm
-  acvp_kas_kdf_set_domain
+  acvp_cap_kas_kdf_enable
+  acvp_cap_kas_kdf_set_parm
+  acvp_cap_kas_kdf_set_domain
   acvp_cap_kas_ifc_enable
   acvp_cap_kas_ifc_set_parm
   acvp_cap_kas_ifc_set_exponent

diff --git a/ms/resources/acvp_app.vcxproj b/ms/resources/acvp_app.vcxproj
@@ -394,6 +394,7 @@
     <ClCompile Include="..\..\app\app_hmac.c" />
     <ClCompile Include="..\..\app\app_kas.c" />
     <ClCompile Include="..\..\app\app_kdf.c" />
+    <ClCompile Include="..\..\app\app_kas_kdf.c" />
     <ClCompile Include="..\..\app\app_main.c" />
     <ClCompile Include="..\..\app\app_rsa.c" />
     <ClCompile Include="..\..\app\app_sha.c" />

diff --git a/ms/resources/acvp_app.vcxproj.filters b/ms/resources/acvp_app.vcxproj.filters
@@ -45,6 +45,9 @@
     <ClCompile Include="..\..\app\app_kdf.c">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\app\app_kas_kdf.c">
+      <Filter>Source Files</Filter>
+    </ClCompile>
     <ClCompile Include="..\..\app\app_main.c">
       <Filter>Source Files</Filter>
     </ClCompile>

diff --git a/ms/resources/libacvp.vcxproj b/ms/resources/libacvp.vcxproj
@@ -397,6 +397,11 @@
     <ClCompile Include="..\..\src\acvp_hmac.c" />
     <ClCompile Include="..\..\src\acvp_kas_ecc.c" />
     <ClCompile Include="..\..\src\acvp_kas_ffc.c" />
+    <ClCompile Include="..\..\src\acvp_kas_ifc.c" />
+    <ClCompile Include="..\..\src\acvp_kts_ifc.c" />
+    <ClCompile Include="..\..\src\acvp_pbkdf.c" />
+    <ClCompile Include="..\..\src\acvp_kas_kdf.c" />
+    <ClCompile Include="..\..\src\acvp_rsa_prim.c" />
     <ClCompile Include="..\..\src\acvp_kdf108.c" />
     <ClCompile Include="..\..\src\acvp_kdf135_ikev1.c" />
     <ClCompile Include="..\..\src\acvp_kdf135_ikev2.c" />

diff --git a/ms/resources/libacvp.vcxproj.filters b/ms/resources/libacvp.vcxproj.filters
@@ -54,6 +54,21 @@
     <ClCompile Include="..\..\src\acvp_kas_ffc.c">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\src\acvp_kas_ifc.c">
+      <Filter>Source Files</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\src\acvp_kts_ifc.c">
+      <Filter>Source Files</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\src\acvp_pbkdf.c">
+      <Filter>Source Files</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\src\acvp_kas_kdf.c">
+      <Filter>Source Files</Filter>
+    </ClCompile>
+    <ClCompile Include="..\..\src\acvp_rsa_prim.c">
+      <Filter>Source Files</Filter>
+    </ClCompile>
     <ClCompile Include="..\..\src\acvp_kdf108.c">
       <Filter>Source Files</Filter>
     </ClCompile>

diff --git a/safe_c_stub/src/safe_str_stub.c b/safe_c_stub/src/safe_str_stub.c
@@ -36,7 +36,6 @@
 #include <stdint.h>
 #include <string.h>
 #include <ctype.h>
-
 #include "safe_lib.h"
 
 #define SAFEC_STUB_UNUSED(x) (void)(x)
@@ -49,8 +48,8 @@
  */
 errno_t strcmp_s (const char *dest, rsize_t dmax, const char *src, int *indicator) {
     if (!src || !dest) return (ESNULLP);
-    if (dmax == 0) return (ESZEROL);
-    *indicator = strncmp(dest, src, dmax);
+    if (dmax == 0 || dmax > RSIZE_MAX_STR) return (ESZEROL);
+    *indicator = strcmp(dest, src);
     return (EOK);
 }
 
@@ -62,8 +61,9 @@ errno_t strcmp_s (const char *dest, rsize_t dmax, const char *src, int *indicato
 errno_t strncmp_s (const char *dest, rsize_t dmax, const char *src, rsize_t smax, int *indicator) {
     if (!src || !dest) return (ESNULLP);
     if (dmax == 0) return (ESZEROL);
-    if (smax > RSIZE_MAX_STR) return (EINVAL);
-    *indicator = strncmp(dest, src, dmax);
+    size_t dlen = strnlen(dest, dmax);
+    if (smax > RSIZE_MAX_STR || smax > dlen) return (EINVAL);
+    *indicator = strncmp(dest, src, smax);
     return (EOK);
 }
 

diff --git a/src/acvp_kas_kdf.c b/src/acvp_kas_kdf.c
@@ -436,77 +436,79 @@ ACVP_KAS_KDF_PATTERN_CANDIDATE cmp_pattern_str(ACVP_CTX *ctx, ACVP_CIPHER cipher
         ACVP_LOG_ERR("pattern candidate too long");
         return 0;
     }
-    strncmp_s(str, len, ACVP_KAS_KDF_PATTERN_UPARTYINFO_STR, sizeof(ACVP_KAS_KDF_PATTERN_UPARTYINFO_STR) - 1, &diff);
+    strcmp_s(str, len, ACVP_KAS_KDF_PATTERN_UPARTYINFO_STR, &diff);
     if (!diff && len == sizeof(ACVP_KAS_KDF_PATTERN_UPARTYINFO_STR) - 1) {
         return ACVP_KAS_KDF_PATTERN_UPARTYINFO;
     }
-    strncmp_s(str, len, ACVP_KAS_KDF_PATTERN_VPARTYINFO_STR, sizeof(ACVP_KAS_KDF_PATTERN_VPARTYINFO_STR) - 1, &diff);
+    strcmp_s(str, len, ACVP_KAS_KDF_PATTERN_VPARTYINFO_STR, &diff);
     if (!diff && len == sizeof(ACVP_KAS_KDF_PATTERN_VPARTYINFO_STR) - 1) {
         return ACVP_KAS_KDF_PATTERN_VPARTYINFO;
     }
-    strncmp_s(str, len, ACVP_KAS_KDF_PATTERN_CONTEXT_STR, sizeof(ACVP_KAS_KDF_PATTERN_CONTEXT_STR) - 1, &diff);
+    strcmp_s(str, len, ACVP_KAS_KDF_PATTERN_CONTEXT_STR, &diff);
     if (!diff && len == sizeof(ACVP_KAS_KDF_PATTERN_CONTEXT_STR) - 1) {
         return ACVP_KAS_KDF_PATTERN_CONTEXT;
     }
-    strncmp_s(str, len, ACVP_KAS_KDF_PATTERN_ALGID_STR, sizeof(ACVP_KAS_KDF_PATTERN_ALGID_STR) - 1, &diff);
+    strcmp_s(str, len, ACVP_KAS_KDF_PATTERN_ALGID_STR, &diff);
     if (!diff && len == sizeof(ACVP_KAS_KDF_PATTERN_ALGID_STR) - 1) {
         return ACVP_KAS_KDF_PATTERN_ALGID;
     }
-    strncmp_s(str, len, ACVP_KAS_KDF_PATTERN_LABEL_STR, sizeof(ACVP_KAS_KDF_PATTERN_LABEL_STR) - 1, &diff);
+    strcmp_s(str, len, ACVP_KAS_KDF_PATTERN_LABEL_STR, &diff);
     if (!diff && len == sizeof(ACVP_KAS_KDF_PATTERN_LABEL_STR) - 1) {
         return ACVP_KAS_KDF_PATTERN_LABEL;
     }
-    strncmp_s(str, len, ACVP_KAS_KDF_PATTERN_LENGTH_STR, sizeof(ACVP_KAS_KDF_PATTERN_LENGTH_STR) - 1, &diff);
+    strcmp_s(str, len, ACVP_KAS_KDF_PATTERN_LENGTH_STR, &diff);
     if (!diff && len == sizeof(ACVP_KAS_KDF_PATTERN_LENGTH_STR) - 1) {
         return ACVP_KAS_KDF_PATTERN_L;
     }
     //only compares first X number of characters, so should match, even though string is literal[0000000]
-    strncmp_s(ACVP_KAS_KDF_PATTERN_LITERAL_STR, sizeof(ACVP_KAS_KDF_PATTERN_LITERAL_STR) - 1, str, len, &diff);
-    if (!diff) {
-        //copy string so it can be tokenized
-        tmp = calloc(len + 1, sizeof(char));
-        if (!tmp) {
-            ACVP_LOG_ERR("Failed to allocate memory when checking literal pattern");
-            goto err;
-        }
-        strncpy_s(tmp, len + 1, str, len);
+    if (sizeof(ACVP_KAS_KDF_PATTERN_LITERAL_STR) - 1 < len) {
+        strncmp_s(str, len, ACVP_KAS_KDF_PATTERN_LITERAL_STR, sizeof(ACVP_KAS_KDF_PATTERN_LITERAL_STR) - 1, &diff);
+        if (!diff) {
+            //copy string so it can be tokenized
+            tmp = calloc(len + 1, sizeof(char));
+            if (!tmp) {
+                ACVP_LOG_ERR("Failed to allocate memory when checking literal pattern");
+                goto err;
+            }
+            strncpy_s(tmp, len + 1, str, len);
 
-        //tokenize around the [] characters
-        token = strtok_s(tmp, &len, "[", &lit);
-        if (!token) { 
-            ACVP_LOG_ERR("Invalid literal pattern candidate");
-            goto err;
-        }
-        token = strtok_s(NULL, &len, "]", &lit); //the actual hex string
-        if (!token) { 
-            ACVP_LOG_ERR("Invalid literal pattern candidate");
-            goto err;
-        }
-        if (strnlen_s(token, ACVP_KAS_KDF_PATTERN_LITERAL_STR_LEN_MAX + 1) > ACVP_KAS_KDF_PATTERN_LITERAL_STR_LEN_MAX) {
-            ACVP_LOG_ERR("Patttern literal too long");
-            goto err;
-        }
-        if (cipher == ACVP_KAS_HKDF) {
-            tc->tc.kas_hkdf->literalCandidate = calloc(ACVP_KAS_KDF_PATTERN_LITERAL_BYTE_MAX, 1);
-            if (!tc->tc.kas_hkdf->literalCandidate) {
-                ACVP_LOG_ERR("Failed to allocate memory when setting literal pattern");
+            //tokenize around the [] characters
+            token = strtok_s(tmp, &len, "[", &lit);
+            if (!token) { 
+                ACVP_LOG_ERR("Invalid literal pattern candidate");
                 goto err;
             }
-            rv = acvp_hexstr_to_bin(token, tc->tc.kas_hkdf->literalCandidate, ACVP_KAS_KDF_PATTERN_LITERAL_BYTE_MAX, &(tc->tc.kas_hkdf->literalLen));
-        } else {
-            tc->tc.kas_kdf_onestep->literalCandidate = calloc(ACVP_KAS_KDF_PATTERN_LITERAL_BYTE_MAX, 1);
-            if (!tc->tc.kas_kdf_onestep->literalCandidate) {
-                ACVP_LOG_ERR("Failed to allocate memory when setting literal pattern");
+            token = strtok_s(NULL, &len, "]", &lit); //the actual hex string
+            if (!token) { 
+                ACVP_LOG_ERR("Invalid literal pattern candidate");
                 goto err;
             }
-            rv = acvp_hexstr_to_bin(token, tc->tc.kas_kdf_onestep->literalCandidate, ACVP_KAS_KDF_PATTERN_LITERAL_BYTE_MAX, &(tc->tc.kas_kdf_onestep->literalLen));
-        }
-        if (rv != ACVP_SUCCESS) {
-            ACVP_LOG_ERR("Hex conversion failure (literal candidate)");
-            goto err;
+            if (strnlen_s(token, ACVP_KAS_KDF_PATTERN_LITERAL_STR_LEN_MAX + 1) > ACVP_KAS_KDF_PATTERN_LITERAL_STR_LEN_MAX) {
+                ACVP_LOG_ERR("Patttern literal too long");
+                goto err;
+            }
+            if (cipher == ACVP_KAS_HKDF) {
+                tc->tc.kas_hkdf->literalCandidate = calloc(ACVP_KAS_KDF_PATTERN_LITERAL_BYTE_MAX, 1);
+                if (!tc->tc.kas_hkdf->literalCandidate) {
+                    ACVP_LOG_ERR("Failed to allocate memory when setting literal pattern");
+                    goto err;
+                }
+                rv = acvp_hexstr_to_bin(token, tc->tc.kas_hkdf->literalCandidate, ACVP_KAS_KDF_PATTERN_LITERAL_BYTE_MAX, &(tc->tc.kas_hkdf->literalLen));
+            } else {
+                tc->tc.kas_kdf_onestep->literalCandidate = calloc(ACVP_KAS_KDF_PATTERN_LITERAL_BYTE_MAX, 1);
+                if (!tc->tc.kas_kdf_onestep->literalCandidate) {
+                    ACVP_LOG_ERR("Failed to allocate memory when setting literal pattern");
+                    goto err;
+                }
+                rv = acvp_hexstr_to_bin(token, tc->tc.kas_kdf_onestep->literalCandidate, ACVP_KAS_KDF_PATTERN_LITERAL_BYTE_MAX, &(tc->tc.kas_kdf_onestep->literalLen));
+            }
+            if (rv != ACVP_SUCCESS) {
+                ACVP_LOG_ERR("Hex conversion failure (literal candidate)");
+                goto err;
+            }
+            if (tmp) free(tmp);
+            return ACVP_KAS_KDF_PATTERN_LITERAL;
         }
-        if (tmp) free(tmp);
-        return ACVP_KAS_KDF_PATTERN_LITERAL;
     }
 
     ACVP_LOG_ERR("Candidate string provided does not match any valid candidate");
@@ -516,7 +518,7 @@ ACVP_KAS_KDF_PATTERN_CANDIDATE cmp_pattern_str(ACVP_CTX *ctx, ACVP_CIPHER cipher
 }
 
 static ACVP_KAS_KDF_PATTERN_CANDIDATE* read_info_pattern(ACVP_CTX *ctx, ACVP_CIPHER cipher, const char *str, ACVP_TEST_CASE *tc) {
-    ACVP_KAS_KDF_PATTERN_CANDIDATE currentCand = -1;
+    ACVP_KAS_KDF_PATTERN_CANDIDATE currentCand;
     char *cpy = NULL;
     ACVP_KAS_KDF_PATTERN_CANDIDATE *rv = NULL;
     int hasUParty = 0, hasVParty = 0; //Currently, these are required

diff --git a/src/acvp_rsa_prim.c b/src/acvp_rsa_prim.c
@@ -390,6 +390,7 @@ ACVP_RESULT acvp_rsa_decprim_kat_handler(ACVP_CTX *ctx, JSON_Object *obj) {
                     ACVP_LOG_ERR("Server JSON missing 'cipher'");
                     rv = ACVP_MISSING_ARG;
                     json_value_free(r_tval);
+                    json_value_free(r_cval);
                     goto err;
                 }
                 cipher_len = strnlen_s(cipher, ACVP_RSA_EXP_BYTE_MAX + 1);
@@ -398,6 +399,7 @@ ACVP_RESULT acvp_rsa_decprim_kat_handler(ACVP_CTX *ctx, JSON_Object *obj) {
                                  ACVP_RSA_SEEDLEN_MAX);
                     rv = ACVP_INVALID_ARG;
                     json_value_free(r_tval);
+                    json_value_free(r_cval);
                     goto err;
                 }
 
@@ -413,6 +415,7 @@ ACVP_RESULT acvp_rsa_decprim_kat_handler(ACVP_CTX *ctx, JSON_Object *obj) {
                            ACVP_LOG_ERR("ERROR: crypto module failed the operation");
                            rv = ACVP_CRYPTO_MODULE_FAIL;
                            json_value_free(r_tval);
+                           json_value_free(r_cval);
                            goto err;
                        }
                     ACVP_LOG_INFO("Looping on fail/pass %d/%d %d/%d", fail, stc.fail, pass, stc.pass);
@@ -428,6 +431,7 @@ ACVP_RESULT acvp_rsa_decprim_kat_handler(ACVP_CTX *ctx, JSON_Object *obj) {
                 if (rv != ACVP_SUCCESS) {
                     ACVP_LOG_ERR("ERROR: JSON output failure in primitive module");
                     json_value_free(r_tval);
+                    json_value_free(r_cval);
                     goto err;
                 }
                 /*
@@ -632,13 +636,15 @@ ACVP_RESULT acvp_rsa_sigprim_kat_handler(ACVP_CTX *ctx, JSON_Object *obj) {
             if (!e_str || !n_str || !d_str) {
                 ACVP_LOG_ERR("Missing e|n|d from server json");
                 rv = ACVP_MISSING_ARG;
+                json_value_free(r_tval);
                 goto err;
             }
             if ((strnlen_s(e_str, ACVP_RSA_EXP_LEN_MAX + 1) > ACVP_RSA_EXP_LEN_MAX) ||
                 (strnlen_s(n_str, ACVP_RSA_EXP_LEN_MAX + 1) > ACVP_RSA_EXP_LEN_MAX) ||
                 (strnlen_s(d_str, ACVP_RSA_EXP_LEN_MAX + 1) > ACVP_RSA_EXP_LEN_MAX)) {
                 ACVP_LOG_ERR("server provided d or e or n of invalid length");
                 rv = ACVP_INVALID_ARG;
+                json_value_free(r_tval);
                 goto err;
             }
 

diff --git a/src/acvp_util.c b/src/acvp_util.c
@@ -587,7 +587,7 @@ const char* acvp_lookup_aux_function_alg_str(ACVP_CIPHER alg) {
 ACVP_CIPHER acvp_lookup_aux_function_alg_tbl(const char *str) {
     int diff = 1, i = 0;
     for (i = 0; i < acvp_aux_function_tbl_len; i++) {
-        strncmp_s(acvp_aux_function_tbl[i].name, ACVP_ALG_NAME_MAX, str, ACVP_ALG_NAME_MAX, &diff);
+    strcmp_s(acvp_aux_function_tbl[i].name, strnlen_s(acvp_aux_function_tbl[i].name, ACVP_ALG_NAME_MAX), str, &diff);
         if (!diff) {
             return acvp_aux_function_tbl[i].cipher;
         }

diff --git a/test/test_acvp_kas_ifc.c b/test/test_acvp_kas_ifc.c
@@ -51,6 +51,7 @@ static void setup(void) {
     rv = acvp_cap_kas_ifc_set_exponent(ctx, ACVP_KAS_IFC_SSC, ACVP_KAS_IFC_FIXEDPUBEXP, expo_str);
     cr_assert(rv == ACVP_SUCCESS);
 
+    free(expo_str);
 }
 
 static void teardown(void) {
@@ -94,6 +95,7 @@ Test(KAS_IFC_CAPABILITY, good) {
     cr_assert(rv == ACVP_SUCCESS);
 
     teardown_ctx(&ctx);
+    free(expo_str);
 }
 
 /*