diff --git a/lib/internal.c b/lib/internal.c index 20b911a..496d976 100644 --- a/lib/internal.c +++ b/lib/internal.c @@ -241,6 +241,7 @@ des_rc des_import_key(const int type, const unsigned char* keyraw, const size_t EXIT: #ifdef _WINDOWS if (pbSessionBlob) { + yc_memzero(pbSessionBlob, cbSessionBlob); free(pbSessionBlob); pbSessionBlob = NULL; } @@ -353,6 +354,7 @@ EXIT: bool yk_des_is_weak_key(const unsigned char *key, const size_t cb_key) { #ifdef _WINDOWS + bool rv = false; /* defined weak keys, borrowed from openssl to be consistent across platforms */ static const unsigned char weak_keys[][DES_LEN_DES] = { /* weak keys */ @@ -400,11 +402,13 @@ bool yk_des_is_weak_key(const unsigned char *key, const size_t cb_key) { if ((0 == memcmp(weak_keys[i], tmp, DES_LEN_DES)) || (0 == memcmp(weak_keys[i], tmp + DES_LEN_DES, DES_LEN_DES)) || (0 == memcmp(weak_keys[i], tmp + 2*DES_LEN_DES, DES_LEN_DES))) { - return true; + rv = true; + break; } } - return false; + yc_memzero(tmp, DES_LEN_3DES); + return rv; #else (void)cb_key; /* unused */ @@ -458,6 +462,8 @@ pkcs5_rc pkcs5_pbkdf2_sha1(const uint8_t* password, const size_t cb_password, co */ if (STATUS_SUCCESS == BCryptOpenAlgorithmProvider(&hAlg, BCRYPT_SHA1_ALGORITHM, NULL, BCRYPT_ALG_HANDLE_HMAC_FLAG)) { + /* suppress const qualifier warning b/c BCrypt doesn't take const input buffers */ +#pragma warning(suppress: 4090) if (STATUS_SUCCESS != BCryptDeriveKeyPBKDF2(hAlg, (PUCHAR)password, (ULONG)cb_password, (PUCHAR)salt, (ULONG)cb_salt, iterations, key, (ULONG)cb_key, 0)) { rc = PKCS5_GENERAL_ERROR; } diff --git a/lib/internal.h b/lib/internal.h index 498b912..83d1b91 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -126,6 +126,7 @@ extern "C" #define CB_OBJ_TAG_MIN 2 // 1 byte tag + 1 byte len #define CB_OBJ_TAG_MAX (CB_OBJ_TAG_MIN + 2) // 1 byte tag + 3 bytes len +#define CB_PIN_MAX 8 #define member_size(type, member) sizeof(((type*)0)->member) typedef enum { @@ -192,6 +193,7 @@ ykpiv_rc _ykpiv_ensure_application_selected(ykpiv_state *state); ykpiv_rc _ykpiv_select_application(ykpiv_state *state); unsigned int _ykpiv_set_length(unsigned char *buffer, size_t length); unsigned int _ykpiv_get_length(const unsigned char *buffer, size_t *len); +bool _ykpiv_has_valid_length(const unsigned char* buffer, size_t len); void* _ykpiv_alloc(ykpiv_state *state, size_t size); void* _ykpiv_realloc(ykpiv_state *state, void *address, size_t size); @@ -236,6 +238,21 @@ typedef enum _yc_log_level_t { void yc_log_event(uint32_t id, yc_log_level_t level, const char *sz_format, ...); +#ifdef _WIN32 +#include +#define yc_memzero SecureZeroMemory +#elif defined(BSD) +#include +#define yc_memzero explicit_bzero +#elif defined(__linux__) +#include +#define yc_memzero OPENSSL_cleanse +#else +#define __STDC_WANT_LIB_EXT1__ 1 +#include +#define yc_memzero(_p, _n) (void)memset_s(_p, (rsize_t)_n, 0, (rsize_t)_n) +#endif + #ifdef __cplusplus } #endif diff --git a/lib/util.c b/lib/util.c index 4133187..3d4384c 100644 --- a/lib/util.c +++ b/lib/util.c @@ -218,6 +218,7 @@ ykpiv_rc ykpiv_util_list_keys(ykpiv_state *state, uint8_t *key_count, ykpiv_key ykpiv_rc res = YKPIV_OK; ykpiv_key *pKey = NULL; uint8_t *pData = NULL; + uint8_t *pTemp = NULL; size_t cbData = 0; size_t offset = 0; uint8_t buf[CB_BUF_MAX]; @@ -274,17 +275,21 @@ ykpiv_rc ykpiv_util_list_keys(ykpiv_state *state, uint8_t *key_count, ykpiv_key for (i = 0; i < sizeof(SLOTS); i++) { cbBuf = sizeof(buf); + res = _read_certificate(state, SLOTS[i], buf, &cbBuf); - if (YKPIV_OK == (res = _read_certificate(state, SLOTS[i], buf, &cbBuf))) { + if ((res == YKPIV_OK) && (cbBuf > 0)) { // add current slot to result, grow result buffer if necessary cbRealloc = (sizeof(ykpiv_key) + cbBuf - 1) > (cbData - offset) ? MAX((sizeof(ykpiv_key) + cbBuf - 1) - (cbData - offset), CB_PAGE) : 0; if (0 != cbRealloc) { - if (NULL == (pData = _ykpiv_realloc(state, pData, cbData + cbRealloc))) { + if (!(pTemp = _ykpiv_realloc(state, pData, cbData + cbRealloc))) { + /* realloc failed, pData will be freed in cleanup */ res = YKPIV_MEMORY_ERROR; goto Cleanup; } + pData = pTemp; + pTemp = NULL; } cbData += cbRealloc; @@ -555,6 +560,7 @@ ykpiv_rc ykpiv_util_read_msroots(ykpiv_state *state, uint8_t **data, size_t *dat int object_id = 0; uint8_t tag = 0; uint8_t *pData = NULL; + uint8_t *pTemp = NULL; size_t cbData = 0; size_t cbRealloc = 0; size_t offset = 0; @@ -605,10 +611,13 @@ ykpiv_rc ykpiv_util_read_msroots(ykpiv_state *state, uint8_t **data, size_t *dat cbRealloc = len > (cbData - offset) ? len - (cbData - offset) : 0; if (0 != cbRealloc) { - if (NULL == (pData = _ykpiv_realloc(state, pData, cbData + cbRealloc))) { + if (!(pTemp = _ykpiv_realloc(state, pData, cbData + cbRealloc))) { + /* realloc failed, pData will be freed in cleanup */ res = YKPIV_MEMORY_ERROR; goto Cleanup; } + pData = pTemp; + pTemp = NULL; } cbData += cbRealloc; @@ -1145,7 +1154,7 @@ ykpiv_rc ykpiv_util_get_protected_mgm(ykpiv_state *state, ykpiv_mgm *mgm) { Cleanup: - memset(data, 0, sizeof(data)); + yc_memzero(data, sizeof(data)); _ykpiv_end_transaction(state); return res; @@ -1183,7 +1192,7 @@ ykpiv_rc ykpiv_util_set_protected_mgm(ykpiv_state *state, ykpiv_mgm *mgm) { } } - if (YKPIV_OK != (res = _ykpiv_begin_transaction(state))) return YKPIV_PCSC_ERROR; + if (YKPIV_OK != (res = _ykpiv_begin_transaction(state))) { res = YKPIV_PCSC_ERROR; goto Cleanup; } if (YKPIV_OK != (res = _ykpiv_ensure_application_selected(state))) goto Cleanup; /* try to set the mgm key as long as we don't encounter a fatal error */ @@ -1278,8 +1287,8 @@ ykpiv_rc ykpiv_util_set_protected_mgm(ykpiv_state *state, ykpiv_mgm *mgm) { Cleanup: - memset(data, 0, sizeof(data)); - memset(mgm_key, 0, sizeof(mgm_key)); + yc_memzero(data, sizeof(data)); + yc_memzero(mgm_key, sizeof(mgm_key)); _ykpiv_end_transaction(state); return res; @@ -1398,8 +1407,10 @@ static ykpiv_rc _write_certificate(ykpiv_state *state, uint8_t slot, uint8_t *da // calculate the required length of the encoded object req_len = 1 /* cert tag */ + 3 /* compression tag + data*/ + 2 /* lrc */; req_len += _ykpiv_set_length(buf, data_len); + req_len += data_len; - if (req_len > _obj_size_max(state)) return YKPIV_SIZE_ERROR; + if (req_len < data_len) return YKPIV_SIZE_ERROR; /* detect overflow of unsigned size_t */ + if (req_len > _obj_size_max(state)) return YKPIV_SIZE_ERROR; /* obj_size_max includes limits for TLV encoding */ buf[offset++] = TAG_CERT; offset += _ykpiv_set_length(buf + offset, data_len); @@ -1446,6 +1457,11 @@ static ykpiv_rc _get_metadata_item(uint8_t *data, size_t cb_data, uint8_t tag, u while (p_temp < (data + cb_data)) { tag_temp = *p_temp++; + + if (!_ykpiv_has_valid_length(p_temp, (data + cb_data - p_temp))) { + return YKPIV_SIZE_ERROR; + } + p_temp += _ykpiv_get_length(p_temp, &cb_temp); if (tag_temp == tag) { diff --git a/lib/ykpiv.c b/lib/ykpiv.c index 3466b12..700ccc3 100644 --- a/lib/ykpiv.c +++ b/lib/ykpiv.c @@ -186,6 +186,19 @@ unsigned int _ykpiv_get_length(const unsigned char *buffer, size_t *len) { return 0; } +bool _ykpiv_has_valid_length(const unsigned char* buffer, size_t len) { + if ((buffer[0] < 0x81) && (len > 0)) { + return true; + } + else if (((*buffer & 0x7f) == 1) && (len > 1)) { + return true; + } + else if (((*buffer & 0x7f) == 2) && (len > 2)) { + return true; + } + return false; +} + static unsigned char *set_object(int object_id, unsigned char *buffer) { *buffer++ = 0x5c; if(object_id == YKPIV_OBJ_DISCOVERY) { @@ -291,8 +304,8 @@ ykpiv_rc _ykpiv_select_application(ykpiv_state *state) { * can determine how to get the serial number, which for the NEO/Yk4 * will result in another selection of the PIV applet. */ - _ykpiv_get_version(state, NULL); - _ykpiv_get_serial(state, NULL, false); + res = _ykpiv_get_version(state, NULL); + if (res == YKPIV_OK) res = _ykpiv_get_serial(state, NULL, false); return res; } @@ -684,6 +697,7 @@ ykpiv_rc ykpiv_authenticate(ykpiv_state *state, unsigned const char *key) { uint32_t recv_len = sizeof(data); int sw; ykpiv_rc res; + des_rc drc = DES_OK; des_key* mgm_key = NULL; size_t out_len = 0; @@ -728,7 +742,12 @@ ykpiv_rc ykpiv_authenticate(ykpiv_state *state, unsigned const char *key) { unsigned char *dataptr = apdu.st.data; unsigned char response[8]; out_len = sizeof(response); - des_decrypt(mgm_key, challenge, sizeof(challenge), response, &out_len); + drc = des_decrypt(mgm_key, challenge, sizeof(challenge), response, &out_len); + + if (drc != DES_OK) { + res = YKPIV_AUTHENTICATION_ERROR; + goto Cleanup; + } recv_len = sizeof(data); memset(apdu.raw, 0, sizeof(apdu)); @@ -766,7 +785,13 @@ ykpiv_rc ykpiv_authenticate(ykpiv_state *state, unsigned const char *key) { { unsigned char response[8]; out_len = sizeof(response); - des_encrypt(mgm_key, challenge, sizeof(challenge), response, &out_len); + drc = des_encrypt(mgm_key, challenge, sizeof(challenge), response, &out_len); + + if (drc != DES_OK) { + res = YKPIV_AUTHENTICATION_ERROR; + goto Cleanup; + } + if (memcmp(response, data + 4, 8) == 0) { res = YKPIV_OK; } @@ -838,6 +863,7 @@ ykpiv_rc ykpiv_set_mgmkey2(ykpiv_state *state, const unsigned char *new_key, con res = YKPIV_GENERIC_ERROR; Cleanup: + yc_memzero(&apdu, sizeof(APDU)); _ykpiv_end_transaction(state); return res; } @@ -1044,6 +1070,12 @@ static ykpiv_rc _ykpiv_get_version(ykpiv_state *state, ykpiv_version_t *p_versio if((res = _send_data(state, &apdu, data, &recv_len, &sw)) != YKPIV_OK) { return res; } else if(sw == SW_SUCCESS) { + + /* check that we received enough data for the verson number */ + if (recv_len < 3) { + return YKPIV_SIZE_ERROR; + } + state->ver.major = data[0]; state->ver.minor = data[1]; state->ver.patch = data[2]; @@ -1182,6 +1214,11 @@ static ykpiv_rc _ykpiv_get_serial(ykpiv_state *state, uint32_t *p_serial, bool f } } + /* check that we received enough data for the serial number */ + if (recv_len < 4) { + return YKPIV_SIZE_ERROR; + } + p_temp = (uint8_t*)(&state->serial); *p_temp++ = data[3]; @@ -1222,6 +1259,7 @@ static ykpiv_rc _cache_pin(ykpiv_state *state, const char *pin, size_t len) { return YKPIV_OK; } if (state->pin) { + yc_memzero(state->pin, strnlen(state->pin, CB_PIN_MAX)); _ykpiv_free(state, state->pin); state->pin = NULL; } @@ -1244,7 +1282,7 @@ static ykpiv_rc _verify(ykpiv_state *state, const char *pin, const size_t pin_le int sw; ykpiv_rc res; - if (pin_len > 8) { + if (pin_len > CB_PIN_MAX) { return YKPIV_SIZE_ERROR; } @@ -1255,27 +1293,36 @@ static ykpiv_rc _verify(ykpiv_state *state, const char *pin, const size_t pin_le apdu.st.lc = pin ? 0x08 : 0; if (pin) { memcpy(apdu.st.data, pin, pin_len); - if (pin_len < 8) { - memset(apdu.st.data + pin_len, 0xff, 8 - pin_len); + if (pin_len < CB_PIN_MAX) { + memset(apdu.st.data + pin_len, 0xff, CB_PIN_MAX - pin_len); } } - if ((res = _send_data(state, &apdu, data, &recv_len, &sw)) != YKPIV_OK) { + + res = _send_data(state, &apdu, data, &recv_len, &sw); + yc_memzero(&apdu, sizeof(apdu)); + + if (res != YKPIV_OK) { return res; - } else if (sw == SW_SUCCESS) { + } + else if (sw == SW_SUCCESS) { 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) { + } + else if ((sw >> 8) == 0x63) { if (tries) *tries = (sw & 0xf); return YKPIV_WRONG_PIN; - } else if (sw == SW_ERR_AUTH_BLOCKED) { + } + else if (sw == SW_ERR_AUTH_BLOCKED) { if (tries) *tries = 0; return YKPIV_WRONG_PIN; - } else { + } + else { return YKPIV_GENERIC_ERROR; } } @@ -1361,10 +1408,10 @@ static ykpiv_rc _ykpiv_change_pin(ykpiv_state *state, int action, const char * c unsigned char data[0xff]; unsigned long recv_len = sizeof(data); ykpiv_rc res; - if (current_pin_len > 8) { + if (current_pin_len > CB_PIN_MAX) { return YKPIV_SIZE_ERROR; } - if (new_pin_len > 8) { + if (new_pin_len > CB_PIN_MAX) { return YKPIV_SIZE_ERROR; } if(action == CHREF_ACT_UNBLOCK_PIN) { @@ -1374,14 +1421,17 @@ static ykpiv_rc _ykpiv_change_pin(ykpiv_state *state, int action, const char * c templ[3] = 0x81; } memcpy(indata, current_pin, current_pin_len); - if(current_pin_len < 8) { - memset(indata + current_pin_len, 0xff, 8 - current_pin_len); + if(current_pin_len < CB_PIN_MAX) { + memset(indata + current_pin_len, 0xff, CB_PIN_MAX - current_pin_len); } - memcpy(indata + 8, new_pin, new_pin_len); - if(new_pin_len < 8) { - memset(indata + 8 + new_pin_len, 0xff, 8 - new_pin_len); + memcpy(indata + CB_PIN_MAX, new_pin, new_pin_len); + if(new_pin_len < CB_PIN_MAX) { + memset(indata + CB_PIN_MAX + new_pin_len, 0xff, CB_PIN_MAX - new_pin_len); } + res = ykpiv_transfer_data(state, templ, indata, sizeof(indata), data, &recv_len, &sw); + yc_memzero(indata, sizeof(indata)); + if(res != YKPIV_OK) { return res; } else if(sw != SW_SUCCESS) { @@ -1477,8 +1527,14 @@ ykpiv_rc _ykpiv_fetch_object(ykpiv_state *state, int object_id, } if(sw == SW_SUCCESS) { - size_t outlen; - unsigned int offs = _ykpiv_get_length(data + 1, &outlen); + size_t outlen = 0; + unsigned int offs = 0; + + if ((*len < 2) || !_ykpiv_has_valid_length(data + 1, *len - 1)) { + return YKPIV_SIZE_ERROR; + } + + offs = _ykpiv_get_length(data + 1, &outlen); if(offs == 0) { return YKPIV_SIZE_ERROR; } @@ -1658,7 +1714,8 @@ ykpiv_rc ykpiv_import_private_key(ykpiv_state *state, const unsigned char key, u padding = elem_len - lens[i]; remaining = (uintptr_t)key_data + sizeof(key_data) - (uintptr_t)in_ptr; if (padding > remaining) { - return YKPIV_ALGORITHM_ERROR; + res = YKPIV_ALGORITHM_ERROR; + goto Cleanup; } memset(in_ptr, 0, padding); in_ptr += padding; @@ -1693,6 +1750,7 @@ ykpiv_rc ykpiv_import_private_key(ykpiv_state *state, const unsigned char key, u } Cleanup: + yc_memzero(key_data, sizeof(key_data)); _ykpiv_end_transaction(state); return res; } @@ -1811,6 +1869,7 @@ ykpiv_rc ykpiv_auth_verifyresponse(ykpiv_state *state, uint8_t *response, const Cleanup: + yc_memzero(&apdu, sizeof(apdu)); _ykpiv_end_transaction(state); return res; }