224 lines
7.5 KiB
Diff
224 lines
7.5 KiB
Diff
From 456ac566938a1da774db06126a2fa6c0cba514b3 Mon Sep 17 00:00:00 2001
|
|
From: Doug Engert <deengert@gmail.com>
|
|
Date: Wed, 14 Jul 2021 11:15:10 -0500
|
|
Subject: [PATCH] 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
|
|
---
|
|
src/libopensc/card-piv.c | 112 +++++++++++++++++++++------------------
|
|
1 file changed, 60 insertions(+), 52 deletions(-)
|
|
|
|
diff --git a/src/libopensc/card-piv.c b/src/libopensc/card-piv.c
|
|
index f144b2ccef..77e4864f66 100644
|
|
--- a/src/libopensc/card-piv.c
|
|
+++ b/src/libopensc/card-piv.c
|
|
@@ -608,14 +608,12 @@ static int piv_generate_key(sc_card_t *card,
|
|
const u8 *cp;
|
|
keydata->exponent = 0;
|
|
|
|
- /* expected tag is 7f49. */
|
|
- /* we will whatever tag is present */
|
|
-
|
|
cp = rbuf;
|
|
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);
|
|
- if (cp == NULL) {
|
|
+ if (cp == NULL || in_len == 0 || cla_out != 0x60 || tag_out != 0x1f49) {
|
|
r = SC_ERROR_ASN1_OBJECT_NOT_FOUND;
|
|
}
|
|
if (r != SC_SUCCESS) {
|
|
@@ -1032,7 +1030,7 @@ piv_cache_internal_data(sc_card_t *card, int enumtag)
|
|
priv->obj_cache[enumtag].obj_len,
|
|
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);
|
|
|
|
/* 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 */
|
|
body = sc_asn1_find_tag(card->ctx, rbuf,
|
|
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");
|
|
r = SC_ERROR_INVALID_DATA;
|
|
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 */
|
|
body = sc_asn1_find_tag(card->ctx, rbuf,
|
|
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");
|
|
r = SC_ERROR_INVALID_DATA;
|
|
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 */
|
|
body = sc_asn1_find_tag(card->ctx, rbuf,
|
|
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");
|
|
r = SC_ERROR_INVALID_DATA;
|
|
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;
|
|
if (rbuflen != 0) {
|
|
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 */
|
|
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);
|
|
int r, tmplen, tmplen2;
|
|
u8 *p;
|
|
- const u8 *tag;
|
|
+ const unsigned char *p2;
|
|
size_t taglen;
|
|
- const u8 *body;
|
|
size_t bodylen;
|
|
+ unsigned int cla, tag;
|
|
unsigned int real_alg_id, op_tag;
|
|
|
|
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,
|
|
sbuf, p - sbuf, rbuf, sizeof rbuf);
|
|
+ if (r < 0)
|
|
+ goto err;
|
|
|
|
- if (r >= 0) {
|
|
- body = sc_asn1_find_tag(card->ctx, rbuf, r, 0x7c, &bodylen);
|
|
- if (body) {
|
|
- tag = sc_asn1_find_tag(card->ctx, body, bodylen, 0x82, &taglen);
|
|
- if (tag) {
|
|
- memcpy(out, tag, taglen);
|
|
- r = taglen;
|
|
- } else
|
|
- r = SC_ERROR_INVALID_DATA;
|
|
- } else
|
|
- r = SC_ERROR_INVALID_DATA;
|
|
+ p2 = rbuf;
|
|
+ r = sc_asn1_read_tag(&p2, r, &cla, &tag, &bodylen);
|
|
+ if (p2 == NULL || r < 0 || bodylen == 0 || (cla|tag) != 0x7C) {
|
|
+ LOG_TEST_GOTO_ERR(card->ctx, SC_ERROR_INVALID_DATA, "Can't find 0x7C");
|
|
+ }
|
|
+
|
|
+ r = sc_asn1_read_tag(&p2, bodylen, &cla, &tag, &taglen);
|
|
+ if (p2 == NULL || r < 0 || taglen == 0 || (cla|tag) != 0x82) {
|
|
+ LOG_TEST_GOTO_ERR(card->ctx, SC_ERROR_INVALID_DATA, "Can't find 0x82");
|
|
}
|
|
|
|
+ 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);
|
|
}
|
|
|
|
@@ -2394,19 +2400,19 @@ piv_compute_signature(sc_card_t *card, const u8 * data, size_t datalen,
|
|
int i;
|
|
size_t nLen;
|
|
u8 rbuf[128]; /* For EC conversions 384 will fit */
|
|
- const u8 * body;
|
|
- size_t bodylen;
|
|
- const u8 * tag;
|
|
- size_t taglen;
|
|
+ const unsigned char *pseq, *pint, *ptemp, *pend;
|
|
+ unsigned int cla, tag;
|
|
+ size_t seqlen;
|
|
+ size_t intlen;
|
|
+ size_t templen;
|
|
|
|
SC_FUNC_CALLED(card->ctx, SC_LOG_DEBUG_VERBOSE);
|
|
|
|
/* The PIV returns a DER SEQUENCE{INTEGER, INTEGER}
|
|
- * Which may have leading 00 to force positive
|
|
- * TODO: -DEE should check if PKCS15 want the same
|
|
- * But PKCS11 just wants 2* filed_length in bytes
|
|
+ * Which may have leading 00 to force a positive integer
|
|
+ * But PKCS11 just wants 2* field_length in bytes
|
|
* 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 ) {
|
|
@@ -2424,32 +2430,34 @@ piv_compute_signature(sc_card_t *card, const u8 * data, size_t datalen,
|
|
if (r < 0)
|
|
goto err;
|
|
|
|
- body = sc_asn1_find_tag(card->ctx, rbuf, r, 0x30, &bodylen);
|
|
-
|
|
- for (i = 0; i<2; i++) {
|
|
- if (body) {
|
|
- tag = sc_asn1_find_tag(card->ctx, body, bodylen, 0x02, &taglen);
|
|
- if (tag) {
|
|
- bodylen -= taglen - (tag - body);
|
|
- body = tag + taglen;
|
|
-
|
|
- if (taglen > nLen) { /* drop leading 00 if present */
|
|
- if (*tag != 0x00) {
|
|
- r = SC_ERROR_INVALID_DATA;
|
|
- goto err;
|
|
- }
|
|
- tag++;
|
|
- taglen--;
|
|
- }
|
|
- memcpy(out + nLen*i + nLen - taglen , tag, taglen);
|
|
- } else {
|
|
+ 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");
|
|
+
|
|
+ pint = pseq;
|
|
+ pend = pseq + seqlen;
|
|
+ for (i = 0; i < 2; i++) {
|
|
+ r = sc_asn1_read_tag(&pint, (pend - pint), &cla, &tag, &intlen);
|
|
+ if (pint == NULL || r < 0 || intlen == 0 || (cla|tag) != 0x02)
|
|
+ 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");
|
|
+
|
|
+ ptemp = pint;
|
|
+ templen = intlen;
|
|
+ if (intlen > nLen) { /* drop leading 00 if present */
|
|
+ if (*ptemp != 0x00) {
|
|
+ LOG_TEST_GOTO_ERR(card->ctx, SC_ERROR_INVALID_DATA,"Signature too long");
|
|
r = SC_ERROR_INVALID_DATA;
|
|
goto err;
|
|
}
|
|
- } else {
|
|
- r = SC_ERROR_INVALID_DATA;
|
|
- goto err;
|
|
+ ptemp++;
|
|
+ templen--;
|
|
}
|
|
+ memcpy(out + nLen*i + nLen - templen , ptemp, templen);
|
|
+ pint += intlen; /* next integer */
|
|
+
|
|
}
|
|
r = 2 * nLen;
|
|
} else { /* RSA is all set */
|