PIV Improved parsing of data from the card

Based on Fuzz testing, many of the calls to sc_asn1_find_tag were replaced
with sc_asn1_read_tag. The input is also tested that the
expected tag is  the first byte. Additional tests are also add.

sc_asn1_find_tag will skip 0X00 or 0Xff if found. NIST sp800-73-x specs
do not allow these extra bytes.

 On branch PIV-improved-parsing
 Changes to be committed:
	modified:   card-piv.c
This commit is contained in:
Doug Engert 2021-07-14 11:15:10 -05:00 committed by Jakub Jelen
parent 8453c0d99a
commit 456ac56693
1 changed files with 58 additions and 50 deletions

View File

@ -608,14 +608,12 @@ static int piv_generate_key(sc_card_t *card,
const u8 *cp; const u8 *cp;
keydata->exponent = 0; keydata->exponent = 0;
/* expected tag is 7f49. */
/* we will whatever tag is present */
cp = rbuf; cp = rbuf;
in_len = r; in_len = r;
/* expected tag is 0x7f49,returned as cla_out == 0x60 and tag_out = 0x1F49 */
r = sc_asn1_read_tag(&cp, in_len, &cla_out, &tag_out, &in_len); r = sc_asn1_read_tag(&cp, in_len, &cla_out, &tag_out, &in_len);
if (cp == NULL) { if (cp == NULL || in_len == 0 || cla_out != 0x60 || tag_out != 0x1f49) {
r = SC_ERROR_ASN1_OBJECT_NOT_FOUND; r = SC_ERROR_ASN1_OBJECT_NOT_FOUND;
} }
if (r != SC_SUCCESS) { if (r != SC_SUCCESS) {
@ -1032,7 +1030,7 @@ piv_cache_internal_data(sc_card_t *card, int enumtag)
priv->obj_cache[enumtag].obj_len, priv->obj_cache[enumtag].obj_len,
0x53, &bodylen); 0x53, &bodylen);
if (body == NULL) if (body == NULL || priv->obj_cache[enumtag].obj_data[0] != 0x53)
LOG_FUNC_RETURN(card->ctx, SC_ERROR_OBJECT_NOT_VALID); LOG_FUNC_RETURN(card->ctx, SC_ERROR_OBJECT_NOT_VALID);
/* get the certificate out */ /* get the certificate out */
@ -1611,7 +1609,7 @@ static int piv_general_mutual_authenticate(sc_card_t *card,
/* Remove the encompassing outer TLV of 0x7C and get the data */ /* Remove the encompassing outer TLV of 0x7C and get the data */
body = sc_asn1_find_tag(card->ctx, rbuf, body = sc_asn1_find_tag(card->ctx, rbuf,
r, 0x7C, &body_len); r, 0x7C, &body_len);
if (!body) { if (!body || rbuf[0] != 0x7C) {
sc_debug(card->ctx, SC_LOG_DEBUG_VERBOSE, "Invalid Witness Data response of NULL\n"); sc_debug(card->ctx, SC_LOG_DEBUG_VERBOSE, "Invalid Witness Data response of NULL\n");
r = SC_ERROR_INVALID_DATA; r = SC_ERROR_INVALID_DATA;
goto err; goto err;
@ -1753,7 +1751,7 @@ static int piv_general_mutual_authenticate(sc_card_t *card,
/* Remove the encompassing outer TLV of 0x7C and get the data */ /* Remove the encompassing outer TLV of 0x7C and get the data */
body = sc_asn1_find_tag(card->ctx, rbuf, body = sc_asn1_find_tag(card->ctx, rbuf,
r, 0x7C, &body_len); r, 0x7C, &body_len);
if(!body) { if(!body || rbuf[0] != 0x7C) {
sc_debug(card->ctx, SC_LOG_DEBUG_VERBOSE, "Could not find outer tag 0x7C in response"); sc_debug(card->ctx, SC_LOG_DEBUG_VERBOSE, "Could not find outer tag 0x7C in response");
r = SC_ERROR_INVALID_DATA; r = SC_ERROR_INVALID_DATA;
goto err; goto err;
@ -1914,7 +1912,7 @@ static int piv_general_external_authenticate(sc_card_t *card,
/* Remove the encompassing outer TLV of 0x7C and get the data */ /* Remove the encompassing outer TLV of 0x7C and get the data */
body = sc_asn1_find_tag(card->ctx, rbuf, body = sc_asn1_find_tag(card->ctx, rbuf,
r, 0x7C, &body_len); r, 0x7C, &body_len);
if (!body) { if (!body || rbuf[0] != 0x7C) {
sc_debug(card->ctx, SC_LOG_DEBUG_VERBOSE, "Invalid Challenge Data response of NULL\n"); sc_debug(card->ctx, SC_LOG_DEBUG_VERBOSE, "Invalid Challenge Data response of NULL\n");
r = SC_ERROR_INVALID_DATA; r = SC_ERROR_INVALID_DATA;
goto err; goto err;
@ -2079,7 +2077,7 @@ piv_get_serial_nr_from_CHUI(sc_card_t* card, sc_serial_number_t* serial)
r = SC_ERROR_INTERNAL; r = SC_ERROR_INTERNAL;
if (rbuflen != 0) { if (rbuflen != 0) {
body = sc_asn1_find_tag(card->ctx, rbuf, rbuflen, 0x53, &bodylen); /* Pass the outer wrapper asn1 */ body = sc_asn1_find_tag(card->ctx, rbuf, rbuflen, 0x53, &bodylen); /* Pass the outer wrapper asn1 */
if (body != NULL && bodylen != 0) { if (body != NULL && bodylen != 0 && rbuf[0] == 0x53) {
fascn = sc_asn1_find_tag(card->ctx, body, bodylen, 0x30, &fascnlen); /* Find the FASC-N data */ fascn = sc_asn1_find_tag(card->ctx, body, bodylen, 0x30, &fascnlen); /* Find the FASC-N data */
guid = sc_asn1_find_tag(card->ctx, body, bodylen, 0x34, &guidlen); guid = sc_asn1_find_tag(card->ctx, body, bodylen, 0x34, &guidlen);
@ -2311,10 +2309,10 @@ static int piv_validate_general_authentication(sc_card_t *card,
piv_private_data_t * priv = PIV_DATA(card); piv_private_data_t * priv = PIV_DATA(card);
int r, tmplen, tmplen2; int r, tmplen, tmplen2;
u8 *p; u8 *p;
const u8 *tag; const unsigned char *p2;
size_t taglen; size_t taglen;
const u8 *body;
size_t bodylen; size_t bodylen;
unsigned int cla, tag;
unsigned int real_alg_id, op_tag; unsigned int real_alg_id, op_tag;
u8 sbuf[4096]; /* needs work. for 3072 keys, needs 384+10 or so */ u8 sbuf[4096]; /* needs work. for 3072 keys, needs 384+10 or so */
@ -2367,20 +2365,28 @@ static int piv_validate_general_authentication(sc_card_t *card,
r = piv_general_io(card, 0x87, real_alg_id, priv->key_ref, r = piv_general_io(card, 0x87, real_alg_id, priv->key_ref,
sbuf, p - sbuf, rbuf, sizeof rbuf); sbuf, p - sbuf, rbuf, sizeof rbuf);
if (r < 0)
goto err;
if (r >= 0) { p2 = rbuf;
body = sc_asn1_find_tag(card->ctx, rbuf, r, 0x7c, &bodylen); r = sc_asn1_read_tag(&p2, r, &cla, &tag, &bodylen);
if (body) { if (p2 == NULL || r < 0 || bodylen == 0 || (cla|tag) != 0x7C) {
tag = sc_asn1_find_tag(card->ctx, body, bodylen, 0x82, &taglen); LOG_TEST_GOTO_ERR(card->ctx, SC_ERROR_INVALID_DATA, "Can't find 0x7C");
if (tag) { }
memcpy(out, tag, taglen);
r = taglen; r = sc_asn1_read_tag(&p2, bodylen, &cla, &tag, &taglen);
} else if (p2 == NULL || r < 0 || taglen == 0 || (cla|tag) != 0x82) {
r = SC_ERROR_INVALID_DATA; LOG_TEST_GOTO_ERR(card->ctx, SC_ERROR_INVALID_DATA, "Can't find 0x82");
} else
r = SC_ERROR_INVALID_DATA;
} }
if (taglen > outlen) {
LOG_TEST_GOTO_ERR(card->ctx, SC_ERROR_INVALID_DATA, "data read longer then buffer");
}
memcpy(out, p2, taglen);
r = taglen;
err:
LOG_FUNC_RETURN(card->ctx, r); LOG_FUNC_RETURN(card->ctx, r);
} }
@ -2394,19 +2400,19 @@ piv_compute_signature(sc_card_t *card, const u8 * data, size_t datalen,
int i; int i;
size_t nLen; size_t nLen;
u8 rbuf[128]; /* For EC conversions 384 will fit */ u8 rbuf[128]; /* For EC conversions 384 will fit */
const u8 * body; const unsigned char *pseq, *pint, *ptemp, *pend;
size_t bodylen; unsigned int cla, tag;
const u8 * tag; size_t seqlen;
size_t taglen; size_t intlen;
size_t templen;
SC_FUNC_CALLED(card->ctx, SC_LOG_DEBUG_VERBOSE); SC_FUNC_CALLED(card->ctx, SC_LOG_DEBUG_VERBOSE);
/* The PIV returns a DER SEQUENCE{INTEGER, INTEGER} /* The PIV returns a DER SEQUENCE{INTEGER, INTEGER}
* Which may have leading 00 to force positive * Which may have leading 00 to force a positive integer
* TODO: -DEE should check if PKCS15 want the same * But PKCS11 just wants 2* field_length in bytes
* But PKCS11 just wants 2* filed_length in bytes
* So we have to strip out the integers * So we have to strip out the integers
* if present and pad on left if too short. * and pad on left if too short.
*/ */
if (priv->alg_id == 0x11 || priv->alg_id == 0x14 ) { if (priv->alg_id == 0x11 || priv->alg_id == 0x14 ) {
@ -2424,32 +2430,34 @@ piv_compute_signature(sc_card_t *card, const u8 * data, size_t datalen,
if (r < 0) if (r < 0)
goto err; goto err;
body = sc_asn1_find_tag(card->ctx, rbuf, r, 0x30, &bodylen); pseq = rbuf;
r = sc_asn1_read_tag(&pseq, r, &cla, &tag, &seqlen);
if (pseq == NULL || r < 0 || seqlen == 0 || (cla|tag) != 0x30)
LOG_TEST_GOTO_ERR(card->ctx, SC_ERROR_INVALID_DATA, "Can't find 0x30");
for (i = 0; i<2; i++) { pint = pseq;
if (body) { pend = pseq + seqlen;
tag = sc_asn1_find_tag(card->ctx, body, bodylen, 0x02, &taglen); for (i = 0; i < 2; i++) {
if (tag) { r = sc_asn1_read_tag(&pint, (pend - pint), &cla, &tag, &intlen);
bodylen -= taglen - (tag - body); if (pint == NULL || r < 0 || intlen == 0 || (cla|tag) != 0x02)
body = tag + taglen; LOG_TEST_GOTO_ERR(card->ctx, SC_ERROR_INVALID_DATA, "Can't find 0x02");
if (intlen > nLen + 1)
LOG_TEST_GOTO_ERR(card->ctx, SC_ERROR_INVALID_DATA,"Signature too long");
if (taglen > nLen) { /* drop leading 00 if present */ ptemp = pint;
if (*tag != 0x00) { templen = intlen;
r = SC_ERROR_INVALID_DATA; if (intlen > nLen) { /* drop leading 00 if present */
goto err; if (*ptemp != 0x00) {
} LOG_TEST_GOTO_ERR(card->ctx, SC_ERROR_INVALID_DATA,"Signature too long");
tag++;
taglen--;
}
memcpy(out + nLen*i + nLen - taglen , tag, taglen);
} else {
r = SC_ERROR_INVALID_DATA; r = SC_ERROR_INVALID_DATA;
goto err; goto err;
} }
} else { ptemp++;
r = SC_ERROR_INVALID_DATA; templen--;
goto err;
} }
memcpy(out + nLen*i + nLen - templen , ptemp, templen);
pint += intlen; /* next integer */
} }
r = 2 * nLen; r = 2 * nLen;
} else { /* RSA is all set */ } else { /* RSA is all set */