From 6694d177e67f02d18c0bb0023661d1e2988771a5 Mon Sep 17 00:00:00 2001 From: jimxie Date: Thu, 10 Jan 2019 15:47:43 -0800 Subject: [PATCH 1/2] fix some problems in reading cert and cert test --- cryptoauthlib/lib/openssl/eccx08_cert.c | 46 ++++++++++++++++++------ cryptoauthlib/test/openssl/test_engine.c | 25 ++++++++----- 2 files changed, 53 insertions(+), 18 deletions(-) diff --git a/cryptoauthlib/lib/openssl/eccx08_cert.c b/cryptoauthlib/lib/openssl/eccx08_cert.c index 396dc09..95e3b4f 100644 --- a/cryptoauthlib/lib/openssl/eccx08_cert.c +++ b/cryptoauthlib/lib/openssl/eccx08_cert.c @@ -109,7 +109,7 @@ int eccx08_cert_free(void* cert) if (pCert->cert_elements) { - OPENSSL_cleanse(pCert->cert_elements, + OPENSSL_cleanse(pCert->cert_elements, sizeof(atcacert_cert_element_t) * pCert->cert_elements_count); OPENSSL_free(pCert->cert_elements); } @@ -132,7 +132,7 @@ atcacert_def_t * eccx08_cert_copy(atcacert_def_t * pCertOrig) return NULL; } - pCertCopy = eccx08_cert_new(pCertOrig->cert_template_size, + pCertCopy = eccx08_cert_new(pCertOrig->cert_template_size, pCertOrig->cert_elements_count); if (pCertCopy) @@ -202,8 +202,8 @@ ATCA_STATUS eccx08_cert_load_pubkey(const atcacert_def_t* def, const uint8_t key for(i=0; 3 > i && ATCA_SUCCESS == status; i++) { - status = atcab_read_zone(def->public_key_dev_loc.zone, - def->public_key_dev_loc.slot, + status = atcab_read_zone(def->public_key_dev_loc.zone, + def->public_key_dev_loc.slot, i, 0, &pKeyTmp[i * 32], 32); } @@ -217,13 +217,13 @@ ATCA_STATUS eccx08_cert_load_pubkey(const atcacert_def_t* def, const uint8_t key return status; } -int eccx08_cert_load_client(ENGINE *e, +int eccx08_cert_load_client(ENGINE *e, SSL *ssl, /**< Session */ STACK_OF(X509_NAME) *ca_dn, /**< Client CA Lists */ X509 **ppCert, /**< Output Cert */ EVP_PKEY **ppKey, /**< Output Private Key - We'll try to ignore it rather than faking one */ STACK_OF(X509) **ppOther, /**< Intermediate CAs - I.e. our signer cert */ - UI_METHOD *ui_method, + UI_METHOD *ui_method, void *callback_data) { uint8_t * pCertRaw = NULL; @@ -244,7 +244,20 @@ int eccx08_cert_load_client(ENGINE *e, DEBUG_ENGINE("*ppKey: %p\n", *ppKey); } - if (!ppCert || !ppKey || !g_cert_def_2_device_ptr) + /* + This part doesn't match the test function in "test_engine.c": + if (!ENGINE_load_ssl_client_cert(ateccx08_engine, NULL, NULL, &pCert, NULL, NULL, NULL, NULL)) + { + ENGINE_TEST_FAIL("Failed to load device certificate"); + } + + Here the fifth parameter "ppKey" is NULL which leads to return "ENGINE_OPENSSL_FAILURE" during the test: + Failed to load public key. + + So I did some modifications which check cert and public key seperately. + */ + // checking client cert is available or not + if (!ppCert || !g_cert_def_2_device_ptr || !g_cert_def_1_signer_ptr) { return ENGINE_OPENSSL_FAILURE; } @@ -276,12 +289,20 @@ int eccx08_cert_load_client(ENGINE *e, // /* Extract/Reconstruct the signer certificate */ // status = atcacert_read_cert(g_cert_def_1_signer_ptr, g_signer_1_ca_public_key, pCertRaw, &certRawSize); - status = eccx08_cert_load_pubkey(g_cert_def_1_signer_ptr, g_signer_1_ca_public_key); + /* + Function eccx08_cert_load_pubkey is going to load signer cert public key, which would be used to extract client cert. + So we need to creat a char array to keep signer cert public key. + **/ + // status = eccx08_cert_load_pubkey(g_cert_def_1_signer_ptr, g_signer_1_ca_public_key); + + // char array to keep signer cert public key - Jim + uint8_t signer_public_key[64]; + status = eccx08_cert_load_pubkey(g_cert_def_1_signer_ptr, signer_public_key); if(ATCA_SUCCESS == status) { /* Extract/Reconstruct the device certificate */ - status = atcacert_read_cert(g_cert_def_2_device_ptr, g_signer_1_ca_public_key, pCertRaw, &certRawSize); + status = (ATCA_STATUS)atcacert_read_cert(g_cert_def_2_device_ptr, signer_public_key, pCertRaw, &certRawSize); } /* Make sure we release the device before checking if the operation succeeded */ @@ -307,7 +328,7 @@ int eccx08_cert_load_client(ENGINE *e, per the TLS specification */ /** \todo Return intermediate cert list. If ppOther is specified we should return our intermediate certificate - however since OpenSSL core doesn't use it this feature is only if an application made the call so + however since OpenSSL core doesn't use it this feature is only if an application made the call so we'll put a todo here */ /* Return the newly reconstructed cert in OpenSSL's internal format */ @@ -315,6 +336,11 @@ int eccx08_cert_load_client(ENGINE *e, /* OpenSSL requires a matching "private" key but it luckily only compares the public key parameters which would normally be generated when the private key is loaded from a file */ + // The other check for ppKey + if (ppKey == NULL) + { + break; + } *ppKey = X509_get_pubkey(pCert); } while (0); diff --git a/cryptoauthlib/test/openssl/test_engine.c b/cryptoauthlib/test/openssl/test_engine.c index 7a9042e..6c845d6 100644 --- a/cryptoauthlib/test/openssl/test_engine.c +++ b/cryptoauthlib/test/openssl/test_engine.c @@ -17,7 +17,7 @@ #define TEST_ENGINE_CERT 1 /* This option registers all configured engine functionality with OpenSSL - this means some operations can be slow - certificate validation could end up + this means some operations can be slow - certificate validation could end up using the device for SHA256 & ECDSA verify operations */ #define TEST_REGISTER_ALL 1 @@ -39,11 +39,11 @@ TEST_SETUP(atca_engine) ERR_load_crypto_strings(); // ERR_print_errors_fp(stderr); - /* An Application can call this function - but if the library is unloaded - and cleaned up the config file won't be reloaded since this function + /* An Application can call this function - but if the library is unloaded + and cleaned up the config file won't be reloaded since this function is only allowed to run once */ //OPENSSL_config(NULL); - + /* For applications loading and unloading the library this is the way to load the configuration Also it will alert when the config file can not be found */ ENGINE_load_dynamic(); @@ -80,7 +80,7 @@ TEST_TEAR_DOWN(atca_engine) case scenario for removing library resources */ snprintf(fail_msg_buf, sizeof(fail_msg_buf), "\r\n"); - + // printf("Call FIPS_mode_set\n"); FIPS_mode_set(0); @@ -332,7 +332,7 @@ TEST(atca_engine, ecdsa) #endif } -/** \brief Test loading/reconstructing the device certificate - will fail if +/** \brief Test loading/reconstructing the device certificate - will fail if the engine was built with ATCA_OPENSSL_ENGINE_STATIC_CONFIG set to 0 */ TEST(atca_engine, cert) { @@ -370,8 +370,17 @@ TEST(atca_engine, cert) // } // printf("\n"); //} - - pubKey = X509_get_pubkey(pCert); + // We need to read signer cert public key instead of client cert public key. + uint8_t g_signer_cert[1024]; + size_t g_signer_cert_size = sizeof(g_signer_cert); + int ret = atcacert_read_cert(&g_cert_def_1_signer, g_signer_1_ca_public_key, g_signer_cert, &g_signer_cert_size); + if (ret != ATCACERT_E_SUCCESS) { + PRINTF("error in atcacert_read_cert: signer cert, ret = %d\n", ret); + return ret; + } + const unsigned char *q = g_signer_cert; + X509 *signer_cert = d2i_X509(NULL, &q, sizeof(g_signer_cert)); + pubKey = X509_get_pubkey(signer_cert); if (!pubKey) { ENGINE_TEST_FAIL("Failed to load public key"); From cc43a3ee5b3722f1d94e1da8031638e4542e8d26 Mon Sep 17 00:00:00 2001 From: Juice-XIJ Date: Thu, 10 Jan 2019 16:13:25 -0800 Subject: [PATCH 2/2] missing semicolon missing semicolon --- cryptoauthlib/test/openssl/test_engine.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cryptoauthlib/test/openssl/test_engine.c b/cryptoauthlib/test/openssl/test_engine.c index 6c845d6..d8d2f14 100644 --- a/cryptoauthlib/test/openssl/test_engine.c +++ b/cryptoauthlib/test/openssl/test_engine.c @@ -416,7 +416,7 @@ TEST(atca_engine, cert) } else if(ENGINE_OPENSSL_FAILURE == sslerr) { - TEST_FAIL_MESSAGE("Certificate is invalid") + TEST_FAIL_MESSAGE("Certificate is invalid"); } else { @@ -462,4 +462,4 @@ void RunAllEngineTests(void) } -#endif \ No newline at end of file +#endif