Skip to content

Commit

Permalink
Ensure that cert verify returns verification failure and not a failure (
Browse files Browse the repository at this point in the history 
#381)
  • Loading branch information
@mrohera
mrohera authored Oct 6, 2018
1 parent 52c631f commit d2195f8
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 42 deletions.
2 changes: 1 addition & 1 deletion edgelet/hsm-sys/azure-iot-hsm-c/src/edge_hsm_client_store.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1641,7 +1641,7 @@ static int generate_edge_hsm_certificates_if_needed(void)
else if ((load_status == LOAD_ERR_VERIFICATION_FAILED) ||
(load_status == LOAD_ERR_NOT_FOUND))
{
LOG_DEBUG("Load status %d. Generating owner and device CA certs and keys", load_status);
LOG_INFO("Load status %d. Regenerating owner and device CA certs and keys", load_status);
if (create_owner_ca_cert() != 0)
{
result = __FAILURE__;
Expand Down
73 changes: 44 additions & 29 deletions edgelet/hsm-sys/azure-iot-hsm-c/src/edge_pki_openssl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -632,12 +632,13 @@ static int cert_set_core_properties
return result;
}

static int validate_certificate_expiration(X509* x509_cert, double *exp_seconds_left)
static int validate_certificate_expiration(X509* x509_cert, double *exp_seconds_left, bool *is_expired)
{
int result;
time_t exp_time;
double seconds_left = 0;

*is_expired = true;
time_t now = time(NULL);
ASN1_TIME *exp_asn1 = X509_get_notAfter(x509_cert);
if ((exp_asn1->type != ASN1_TIME_STRING_UTC_FORMAT) &&
Expand All @@ -651,13 +652,16 @@ static int validate_certificate_expiration(X509* x509_cert, double *exp_seconds_
LOG_ERROR("Could not parse expiration date from certificate");
result = __FAILURE__;
}
else if ((seconds_left = difftime(exp_time, now)) <= 0)
{
LOG_ERROR("Certificate has expired");
result = __FAILURE__;
}
else
{
if ((seconds_left = difftime(exp_time, now)) <= 0)
{
LOG_ERROR("Certificate has expired");
}
else
{
*is_expired = false;
}
result = 0;
}

Expand Down Expand Up @@ -688,9 +692,11 @@ static int cert_set_expiration
{
// determine max validity in seconds of issuer
double exp_seconds_left_from_now = 0;
if (validate_certificate_expiration(issuer_cert, &exp_seconds_left_from_now) != 0)
bool is_expired = true;
int status = validate_certificate_expiration(issuer_cert, &exp_seconds_left_from_now, &is_expired);
if ((status != 0) || (is_expired))
{
LOG_ERROR("Issuer certificate expiration failure");
LOG_ERROR("Issuer certificate expiration failure. Status %d, verify status: %d", status, is_expired);
result = __FAILURE__;
}
else
Expand Down Expand Up @@ -1355,19 +1361,12 @@ static int check_certificates
int result;
X509_STORE_CTX *store_ctxt = NULL;
X509* x509_cert = NULL;
double exp_seconds = 0;

if ((x509_cert = load_certificate_file(cert_file)) == NULL)
{
LOG_ERROR("Could not create X509 to verify certificate %s", cert_file);
result = __FAILURE__;
}
else if (validate_certificate_expiration(x509_cert, &exp_seconds) != 0)
{
LOG_ERROR("Certificate file has expired %s", cert_file);
X509_free(x509_cert);
result = __FAILURE__;
}
else if ((store_ctxt = X509_STORE_CTX_new()) == NULL)
{
LOG_ERROR("Could not create X509 store context");
Expand All @@ -1386,26 +1385,42 @@ static int check_certificates
}
else
{
double exp_seconds = 0;
int status;
if ((status = X509_verify_cert(store_ctxt)) <= 0)
bool is_expired = true;

status = validate_certificate_expiration(x509_cert, &exp_seconds, &is_expired);
if (status != 0)
{
const char *msg;
int err_code = X509_STORE_CTX_get_error(store_ctxt);
msg = X509_verify_cert_error_string(err_code);
if (msg == NULL)
{
msg = "";
}
LOG_ERROR("Could not verify certificate %s using issuer certificate %s.",
cert_file, issuer_cert_file);
LOG_ERROR("Verification status: %d, Error: %d, Msg: '%s'", status, err_code, msg);
LOG_ERROR("Verifying certificate expiration failed for %s", cert_file);
result = __FAILURE__;
}
else
{
LOG_DEBUG("Certificate validated %s", cert_file);
*verify_status = true;
if (is_expired)
{
LOG_INFO("Certificate file has expired %s", cert_file);
}
else if ((status = X509_verify_cert(store_ctxt)) <= 0)
{
const char *msg;
int err_code = X509_STORE_CTX_get_error(store_ctxt);
msg = X509_verify_cert_error_string(err_code);
if (msg == NULL)
{
msg = "";
}
LOG_ERROR("Could not verify certificate %s using issuer certificate %s.",
cert_file, issuer_cert_file);
LOG_ERROR("Verification status: %d, Error: %d, Msg: '%s'", status, err_code, msg);
}
else
{
LOG_DEBUG("Certificate validated %s", cert_file);
*verify_status = true;
}
result = 0;
}
result = 0;
}
X509_STORE_CTX_free(store_ctxt);
X509_free(x509_cert);
Expand Down
64 changes: 52 additions & 12 deletions edgelet/hsm-sys/azure-iot-hsm-c/tests/edge_openssl_pki_ut/edge_openssl_pki_ut.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,14 @@ static ASN1_TIME TEST_ASN1_TIME_AFTER = {
.flags = 0
};

unsigned char ASN1_DATA_EXPIRED[] = "EXP012345678";
static ASN1_TIME TEST_ASN1_TIME_AFTER_EXPIRED = {
.length = VALID_ASN1_TIME_STRING_UTC_LEN,
.type = VALID_ASN1_TIME_STRING_UTC_FORMAT,
.data = ASN1_DATA_EXPIRED,
.flags = 0
};

static ASN1_TIME TEST_UTC_NOW_TIME_FROM_ASN1 = {
VALID_ASN1_TIME_STRING_UTC_LEN,
VALID_ASN1_TIME_STRING_UTC_FORMAT,
Expand Down Expand Up @@ -638,7 +646,14 @@ static time_t test_hook_get_utc_time_from_asn_string
(void)length;

time_t now = time(NULL);
return now + TEST_UTC_TIME_FROM_ASN1;
int offset = TEST_UTC_TIME_FROM_ASN1;

if (memcmp(time_value, ASN1_DATA_EXPIRED, sizeof(ASN1_DATA_EXPIRED)) == 0)
{
// this ensures that certificate will always be evaluated as expired
offset = -5;
}
return now + offset;
}

static ASN1_TIME* test_hook_X509_gmtime_adj(ASN1_TIME *s, long adj)
Expand Down Expand Up @@ -1621,6 +1636,7 @@ static void test_helper_verify_certificate
const char *key_file,
const char *issuer_cert_file,
bool force_set_verify_return_value,
ASN1_TIME *force_set_asn1_time,
char *failed_function_list,
size_t failed_function_size
)
Expand Down Expand Up @@ -1681,14 +1697,6 @@ static void test_helper_verify_certificate

test_helper_load_cert_file(TEST_CERT_FILE, TEST_X509, &i, failed_function_list, failed_function_size);

STRICT_EXPECTED_CALL(mocked_X509_get_notAfter(TEST_X509));
ASSERT_IS_TRUE_WITH_MSG((i < failed_function_size), "Line:" TOSTRING(__LINE__));
i++;

STRICT_EXPECTED_CALL(get_utc_time_from_asn_string(TEST_ASN1_TIME_AFTER.data, VALID_ASN1_TIME_STRING_UTC_LEN));
ASSERT_IS_TRUE_WITH_MSG((i < failed_function_size), "Line:" TOSTRING(__LINE__));
failed_function_list[i++] = 1;

STRICT_EXPECTED_CALL(X509_STORE_CTX_new());
ASSERT_IS_TRUE_WITH_MSG((i < failed_function_size), "Line:" TOSTRING(__LINE__));
failed_function_list[i++] = 1;
Expand All @@ -1705,6 +1713,15 @@ static void test_helper_verify_certificate
ASSERT_IS_TRUE_WITH_MSG((i < failed_function_size), "Line:" TOSTRING(__LINE__));
failed_function_list[i++] = 1;

ASN1_TIME *asn1_time = (force_set_asn1_time != NULL) ? force_set_asn1_time : &TEST_ASN1_TIME_AFTER;
STRICT_EXPECTED_CALL(mocked_X509_get_notAfter(TEST_X509)).SetReturn(asn1_time);
ASSERT_IS_TRUE_WITH_MSG((i < failed_function_size), "Line:" TOSTRING(__LINE__));
i++;

STRICT_EXPECTED_CALL(get_utc_time_from_asn_string(asn1_time->data, VALID_ASN1_TIME_STRING_UTC_LEN));
ASSERT_IS_TRUE_WITH_MSG((i < failed_function_size), "Line:" TOSTRING(__LINE__));
failed_function_list[i++] = 1;

int return_value = (force_set_verify_return_value)?1:0;
STRICT_EXPECTED_CALL(X509_verify_cert(TEST_STORE_CTXT)).SetReturn(return_value);
ASSERT_IS_TRUE_WITH_MSG((i < failed_function_size), "Line:" TOSTRING(__LINE__));
Expand Down Expand Up @@ -2970,7 +2987,7 @@ BEGIN_TEST_SUITE(edge_openssl_pki_unittests)
size_t failed_function_size = MAX_FAILED_FUNCTION_LIST_SIZE;
char failed_function_list[MAX_FAILED_FUNCTION_LIST_SIZE];
memset(failed_function_list, 0, failed_function_size);
test_helper_verify_certificate(TEST_CERT_FILE, TEST_KEY_FILE, TEST_ISSUER_CERT_FILE, true, failed_function_list, failed_function_size);
test_helper_verify_certificate(TEST_CERT_FILE, TEST_KEY_FILE, TEST_ISSUER_CERT_FILE, true, NULL, failed_function_list, failed_function_size);
bool verify_status = true;

// act
Expand Down Expand Up @@ -3020,7 +3037,7 @@ BEGIN_TEST_SUITE(edge_openssl_pki_unittests)
size_t failed_function_size = MAX_FAILED_FUNCTION_LIST_SIZE;
char failed_function_list[MAX_FAILED_FUNCTION_LIST_SIZE];
memset(failed_function_list, 0, failed_function_size);
test_helper_verify_certificate(TEST_CERT_FILE, TEST_KEY_FILE, TEST_ISSUER_CERT_FILE, false, failed_function_list, failed_function_size);
test_helper_verify_certificate(TEST_CERT_FILE, TEST_KEY_FILE, TEST_ISSUER_CERT_FILE, false, NULL, failed_function_list, failed_function_size);
bool verify_status = false;

// act
Expand All @@ -3034,6 +3051,29 @@ BEGIN_TEST_SUITE(edge_openssl_pki_unittests)
// cleanup
}

/**
* Test function for API
* verify_certificate
*/
TEST_FUNCTION(verify_certificate_expired_certificate_verifies_false_and_returns_success)
{
// arrange
size_t failed_function_size = MAX_FAILED_FUNCTION_LIST_SIZE;
char failed_function_list[MAX_FAILED_FUNCTION_LIST_SIZE];
memset(failed_function_list, 0, failed_function_size);
test_helper_verify_certificate(TEST_CERT_FILE, TEST_KEY_FILE, TEST_ISSUER_CERT_FILE, false, &TEST_ASN1_TIME_AFTER_EXPIRED, failed_function_list, failed_function_size);
bool verify_status = true;

// act
int status = verify_certificate(TEST_CERT_FILE, TEST_KEY_FILE, TEST_ISSUER_CERT_FILE, &verify_status);

// assert
ASSERT_ARE_EQUAL_WITH_MSG(int, 0, status, "Line:" TOSTRING(__LINE__));
ASSERT_IS_FALSE_WITH_MSG(verify_status, "Line:" TOSTRING(__LINE__));

// cleanup
}

/**
* Test function for API
* verify_certificate
Expand All @@ -3047,7 +3087,7 @@ BEGIN_TEST_SUITE(edge_openssl_pki_unittests)
size_t failed_function_size = MAX_FAILED_FUNCTION_LIST_SIZE;
char failed_function_list[MAX_FAILED_FUNCTION_LIST_SIZE];
memset(failed_function_list, 0, failed_function_size);
test_helper_verify_certificate(TEST_CERT_FILE, TEST_KEY_FILE, TEST_ISSUER_CERT_FILE, true, failed_function_list, failed_function_size);
test_helper_verify_certificate(TEST_CERT_FILE, TEST_KEY_FILE, TEST_ISSUER_CERT_FILE, true, NULL, failed_function_list, failed_function_size);
umock_c_negative_tests_snapshot();

for (size_t i = 0; i < umock_c_negative_tests_call_count(); i++)
Expand Down

0 comments on commit d2195f8

Please sign in to comment.