From 252226220a07f153f9c9cc121e33fab29265b0cb Mon Sep 17 00:00:00 2001 From: Trevor Bentley Date: Tue, 31 Oct 2017 12:29:16 +0100 Subject: [PATCH] Disable ensure_application_selected() by default, since it breaks PIN policy. --- lib/tests/api.c | 127 ++++++++++++++++++++++++++++++++++++++- lib/util.c | 5 +- lib/ykpiv.c | 53 ++++++++++++++-- ykcs11/tests/Makefile.am | 2 +- ykcs11/tests/reset.sh | 20 ++++++ 5 files changed, 198 insertions(+), 9 deletions(-) create mode 100755 ykcs11/tests/reset.sh diff --git a/lib/tests/api.c b/lib/tests/api.c index 81ec279..aff6aaa 100644 --- a/lib/tests/api.c +++ b/lib/tests/api.c @@ -281,6 +281,7 @@ bool prepare_rsa_signature(const unsigned char *in, unsigned int in_len, unsigne *out_len = (unsigned int)i2d_X509_SIG(&digestInfo, &out); return true; } + START_TEST(test_import_key) { ykpiv_rc res; @@ -431,6 +432,129 @@ START_TEST(test_import_key) { } END_TEST +START_TEST(test_pin_policy_always) { + ykpiv_rc res; + + { + unsigned char pp = YKPIV_PINPOLICY_ALWAYS; + unsigned char tp = YKPIV_TOUCHPOLICY_DEFAULT; + EVP_PKEY *private_key = NULL; + BIO *bio = NULL; + RSA *rsa_private_key = NULL; + unsigned char e[4]; + unsigned char p[128]; + unsigned char q[128]; + unsigned char dmp1[128]; + unsigned char dmq1[128]; + unsigned char iqmp[128]; + int element_len = 128; + + bio = BIO_new_mem_buf(private_key_pem, strlen(private_key_pem)); + private_key = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL); + ck_assert_ptr_nonnull(private_key); + BIO_free(bio); + rsa_private_key = EVP_PKEY_get1_RSA(private_key); + ck_assert_ptr_nonnull(rsa_private_key); + ck_assert(set_component(e, rsa_private_key->e, 3)); + ck_assert(set_component(p, rsa_private_key->p, element_len)); + ck_assert(set_component(q, rsa_private_key->q, element_len)); + ck_assert(set_component(dmp1, rsa_private_key->dmp1, element_len)); + ck_assert(set_component(dmq1, rsa_private_key->dmq1, element_len)); + ck_assert(set_component(iqmp, rsa_private_key->iqmp, element_len)); + + // Try wrong algorithm, fail. + res = ykpiv_import_private_key(g_state, + 0x9e, + YKPIV_ALGO_RSA1024, + p, element_len, + q, element_len, + dmp1, element_len, + dmq1, element_len, + iqmp, element_len, + NULL, 0, + pp, tp); + ck_assert_int_eq(res, YKPIV_ALGORITHM_ERROR); + + // Try right algorithm + res = ykpiv_import_private_key(g_state, + 0x9e, + YKPIV_ALGO_RSA2048, + p, element_len, + q, element_len, + dmp1, element_len, + dmq1, element_len, + iqmp, element_len, + NULL, 0, + pp, tp); + ck_assert_int_eq(res, YKPIV_OK); + EVP_PKEY_free(private_key); + } + + // Verify certificate + { + BIO *bio = NULL; + X509 *cert = NULL; + RSA *rsa = NULL; + EVP_PKEY *pub_key = NULL; + const EVP_MD *md = EVP_sha256(); + EVP_MD_CTX *mdctx; + + unsigned char signature[1024]; + unsigned char encoded[1024]; + unsigned char data[1024]; + unsigned char signinput[1024]; + unsigned char rand[128]; + + size_t sig_len = sizeof(signature); + size_t padlen = 256; + unsigned int enc_len; + unsigned int data_len; + + bio = BIO_new_mem_buf(certificate_pem, strlen(certificate_pem)); + cert = PEM_read_bio_X509(bio, NULL, NULL, NULL); + ck_assert_ptr_nonnull(cert); + BIO_free(bio); + pub_key = X509_get_pubkey(cert); + ck_assert_ptr_nonnull(pub_key); + rsa = EVP_PKEY_get1_RSA(pub_key); + ck_assert_ptr_nonnull(rsa); + + ck_assert_int_gt(RAND_pseudo_bytes(rand, 128), 0); + mdctx = EVP_MD_CTX_create(); + EVP_DigestInit_ex(mdctx, md, NULL); + EVP_DigestUpdate(mdctx, rand, 128); + EVP_DigestFinal_ex(mdctx, data, &data_len); + + prepare_rsa_signature(data, data_len, encoded, &enc_len, EVP_MD_type(md)); + ck_assert_int_ne(RSA_padding_add_PKCS1_type_1(signinput, padlen, encoded, enc_len), 0); + + // Sign without verify: fail + res = ykpiv_sign_data(g_state, signinput, padlen, signature, &sig_len, YKPIV_ALGO_RSA2048, 0x9e); + ck_assert_int_eq(res, YKPIV_AUTHENTICATION_ERROR); + + // Sign with verify: pass + res = ykpiv_verify(g_state, "123456", NULL); + ck_assert_int_eq(res, YKPIV_OK); + res = ykpiv_sign_data(g_state, signinput, padlen, signature, &sig_len, YKPIV_ALGO_RSA2048, 0x9e); + ck_assert_int_eq(res, YKPIV_OK); + + // Sign again without verify: fail + res = ykpiv_sign_data(g_state, signinput, padlen, signature, &sig_len, YKPIV_ALGO_RSA2048, 0x9e); + ck_assert_int_eq(res, YKPIV_AUTHENTICATION_ERROR); + + // Sign again with verify: pass + res = ykpiv_verify(g_state, "123456", NULL); + ck_assert_int_eq(res, YKPIV_OK); + res = ykpiv_sign_data(g_state, signinput, padlen, signature, &sig_len, YKPIV_ALGO_RSA2048, 0x9e); + ck_assert_int_eq(res, YKPIV_OK); + + ck_assert_int_eq(RSA_verify(EVP_MD_type(md), data, data_len, signature, sig_len, rsa), 1); + + X509_free(cert); + } +} +END_TEST + START_TEST(test_generate_key) { ykpiv_rc res; uint8_t *mod, *exp; @@ -440,7 +564,7 @@ START_TEST(test_generate_key) { res = ykpiv_util_generate_key(g_state, YKPIV_KEY_AUTHENTICATION, YKPIV_ALGO_RSA2048, - YKPIV_PINPOLICY_ONCE, + YKPIV_PINPOLICY_DEFAULT, YKPIV_TOUCHPOLICY_DEFAULT, &mod, &mod_len, @@ -802,6 +926,7 @@ Suite *test_suite(void) { tcase_add_test(tc, test_list_readers); tcase_add_test(tc, test_read_write_list_delete_cert); tcase_add_test(tc, test_import_key); + tcase_add_test(tc, test_pin_policy_always); tcase_add_test(tc, test_generate_key); // Must be last: tear down and re-test with custom memory allocator diff --git a/lib/util.c b/lib/util.c index f844f4a..8fa8f59 100644 --- a/lib/util.c +++ b/lib/util.c @@ -721,12 +721,11 @@ ykpiv_rc ykpiv_util_generate_key(ykpiv_state *state, uint8_t slot, uint8_t algor if (match == 3 && major == 4 && (minor < 3 || (minor == 3 && build < 5))) { fprintf(stderr, "On-chip RSA key generation on this YubiKey has been blocked.\n"); fprintf(stderr, "Please see https://yubi.co/ysa201701/ for details.\n"); - res = YKPIV_NOT_SUPPORTED; - goto Cleanup; + return YKPIV_NOT_SUPPORTED; } } else { fprintf(stderr, "Failed to get device version.\n"); - goto Cleanup; + return YKPIV_GENERIC_ERROR; } } diff --git a/lib/ykpiv.c b/lib/ykpiv.c index b6d0a05..df595a3 100644 --- a/lib/ykpiv.c +++ b/lib/ykpiv.c @@ -27,6 +27,7 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * */ +/** @file */ #include #include @@ -37,6 +38,40 @@ #include "internal.h" #include "ykpiv.h" +/** + * DISABLE_PIN_CACHE - disable in-RAM cache of PIN + * + * By default, the PIN is cached in RAM when provided to \p ykpiv_verify() or + * changed with \p ykpiv_change_pin(). If the USB connection is lost between + * calls, the device will be re-authenticated on the next call using the cached + * PIN. The PIN is cleared with a call to \p ykpiv_done(). + * + * The PIN cache prevents problems with long-running applications losing their + * authentication in some cases, such as when a laptop sleeps. + * + * The cache can be disabled by setting this define to 1 if it is not desired + * to store the PIN in RAM. + * + */ +#define DISABLE_PIN_CACHE 0 + +/** + * ENABLE_APPLICATION_RESELECT - re-select application for all public API calls + * + * If this is enabled, every public call (prefixed with \r ykpiv_) will check + * that the PIV application is currently selected, or re-select it if it is + * not. + * + * Auto re-selection allows a long-running PIV application to cooperate on + * a system that may simultaneously use the non-PIV applications of connected + * devices. + * + * This is \b DANGEROUS - with this enabled, slots with the policy + * \p YKPIV_PINPOLICY_ALWAYS will not be accessible. + * + */ +#define ENABLE_APPLICATION_RESELECTION 0 + #define YKPIV_MGM_DEFAULT "\x01\x02\x03\x04\x05\x06\x07\x08\x01\x02\x03\x04\x05\x06\x07\x08\x01\x02\x03\x04\x05\x06\x07\x08" static ykpiv_rc _cache_pin(ykpiv_state *state, const char *pin, size_t len); @@ -228,6 +263,7 @@ ykpiv_rc _ykpiv_select_application(ykpiv_state *state) { ykpiv_rc _ykpiv_ensure_application_selected(ykpiv_state *state) { ykpiv_rc res = YKPIV_OK; +#if ENABLE_APPLICATION_RESELECTION if (NULL == state) { return YKPIV_GENERIC_ERROR; } @@ -242,6 +278,9 @@ ykpiv_rc _ykpiv_ensure_application_selected(ykpiv_state *state) { } return res; +#else + return res; +#endif } static ykpiv_rc _ykpiv_connect(ykpiv_state *state, uintptr_t context, uintptr_t card) { @@ -333,7 +372,11 @@ ykpiv_rc ykpiv_connect(ykpiv_state *state, const char *wanted) { * was supplied by an external library. */ if (YKPIV_OK != (ret = _ykpiv_begin_transaction(state))) return YKPIV_PCSC_ERROR; +#if ENABLE_APPLICATION_RESELECTION ret = _ykpiv_ensure_application_selected(state); +#else + ret = _ykpiv_select_application(state); +#endif _ykpiv_end_transaction(state); return ret; } @@ -928,7 +971,7 @@ Cleanup: } static ykpiv_rc _cache_pin(ykpiv_state *state, const char *pin, size_t len) { -#ifdef DISABLE_PIN_CACHE +#if DISABLE_PIN_CACHE // Some embedded applications of this library may not want to keep the PIN // data in RAM for security reasons. return YKPIV_OK; @@ -976,9 +1019,11 @@ static ykpiv_rc _verify(ykpiv_state *state, const char *pin, const size_t pin_le if ((res = _send_data(state, &apdu, data, &recv_len, &sw)) != YKPIV_OK) { return res; } else if (sw == SW_SUCCESS) { - // Intentionally ignore errors. If the PIN fails to save, it will only - // be a problem if a reconnect is attempted. Failure deferred until then. - _cache_pin(state, pin, pin_len); + if (pin && pin_len) { + // Intentionally ignore errors. If the PIN fails to save, it will only + // be a problem if a reconnect is attempted. Failure deferred until then. + _cache_pin(state, pin, pin_len); + } if (tries) *tries = (sw & 0xf); return YKPIV_OK; } else if ((sw >> 8) == 0x63) { diff --git a/ykcs11/tests/Makefile.am b/ykcs11/tests/Makefile.am index 4ab1182..800b610 100644 --- a/ykcs11/tests/Makefile.am +++ b/ykcs11/tests/Makefile.am @@ -45,7 +45,7 @@ endif ykcs11_tests_LDADD = ../libykcs11.la $(OPENSSL_LIBS) check_PROGRAMS = ykcs11_tests -TESTS = $(check_PROGRAMS) +TESTS = reset.sh $(check_PROGRAMS) if ENABLE_COV AM_LDFLAGS += --coverage diff --git a/ykcs11/tests/reset.sh b/ykcs11/tests/reset.sh new file mode 100755 index 0000000..6c20551 --- /dev/null +++ b/ykcs11/tests/reset.sh @@ -0,0 +1,20 @@ +BIN="../../tool/yubico-piv-tool${EXEEXT}" + +# Verify that user has confirmed destructive hw-tests +if [ "x$YKPIV_ENV_HWTESTS_CONFIRMED" != "x1" ]; then + printf "\n***\n*** Hardware tests skipped. Run \"make hwcheck\".\n***\n\n" >&0 + exit 77 # exit code 77 == skipped tests +fi + +# Reset +$BIN -averify-pin -P000000 || true +$BIN -averify-pin -P000000 || true +$BIN -averify-pin -P000000 || true +$BIN -averify-pin -P000000 || true +$BIN -averify-pin -P000000 || true +$BIN -achange-puk -P000000 -N00000000 || true +$BIN -achange-puk -P000000 -N00000000 || true +$BIN -achange-puk -P000000 -N00000000 || true +$BIN -achange-puk -P000000 -N00000000 || true +$BIN -achange-puk -P000000 -N00000000 || true +$BIN -areset