From 2829c5870fdcd855b44f6d156dc4d3949c4ed0f9 Mon Sep 17 00:00:00 2001 From: Martin Paljak Date: Sat, 20 Apr 2019 10:08:27 +0300 Subject: [PATCH] Address review comments Change-Id: I9aa97c8a9878dddd3e6f1a2baa877d188b9d7fe5 --- src/libopensc/card-esteid2018.c | 49 ++++++++++++++++--------------- src/libopensc/pkcs15-esteid2018.c | 8 ++--- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/libopensc/card-esteid2018.c b/src/libopensc/card-esteid2018.c index d00f1c7b..9de32e8a 100644 --- a/src/libopensc/card-esteid2018.c +++ b/src/libopensc/card-esteid2018.c @@ -32,6 +32,9 @@ /* Helping defines */ #define SIGNATURE_PAYLOAD_SIZE 0x30 +#define PIN1_REF 0x01 +#define PIN2_REF 0x85 +#define PUK_REF 0x02 static const struct sc_atr_table esteid_atrs[] = { {"3b:db:96:00:80:b1:fe:45:1f:83:00:12:23:3f:53:65:49:44:0f:90:00:f1", NULL, "EstEID 2018", SC_CARD_TYPE_ESTEID_2018, 0, NULL}, @@ -60,16 +63,11 @@ struct esteid_priv_data { } while (0) static int esteid_match_card(sc_card_t *card) { - int i = 0; - int r = 0; + int i = _sc_match_atr(card, esteid_atrs, &card->type); - i = _sc_match_atr(card, esteid_atrs, &card->type); - if (i >= 0) { - r = gp_select_aid(card, &IASECC_AID); - if (r == SC_SUCCESS) { - card->name = esteid_atrs[i].name; - return 1; - } + if (i >= 0 && gp_select_aid(card, &IASECC_AID) == SC_SUCCESS) { + card->name = esteid_atrs[i].name; + return 1; } return 0; } @@ -106,7 +104,7 @@ static int esteid_select(struct sc_card *card, unsigned char p1, unsigned char i static int esteid_select_file(struct sc_card *card, const struct sc_path *in_path, struct sc_file **file_out) { unsigned char pathbuf[SC_MAX_PATH_SIZE], *path = pathbuf; - int pathlen; + size_t pathlen; struct sc_file *file = NULL; LOG_FUNC_CALLED(card->ctx); @@ -122,7 +120,7 @@ static int esteid_select_file(struct sc_card *card, const struct sc_path *in_pat while (pathlen >= 2) { if (memcmp(path, "\x3F\x00", 2) == 0) { LOG_TEST_RET(card->ctx, esteid_select(card, 0x00, 0x3F, 0x00), "MF select failed"); - } else if (pathlen >= 2 && path[0] == 0xAD) { + } else if (path[0] == 0xAD) { LOG_TEST_RET(card->ctx, esteid_select(card, 0x01, path[0], path[1]), "DF select failed"); } else if (pathlen == 2) { LOG_TEST_RET(card->ctx, esteid_select(card, 0x02, path[0], path[1]), "EF select failed"); @@ -133,6 +131,7 @@ static int esteid_select_file(struct sc_card *card, const struct sc_path *in_pat if (file == NULL) LOG_FUNC_RETURN(card->ctx, SC_ERROR_OUT_OF_MEMORY); file->path = *in_path; + file->size = 1536; // Dummy size, to be above 1024 *file_out = file; } @@ -190,7 +189,8 @@ static int esteid_compute_signature(sc_card_t *card, const u8 *data, size_t data struct esteid_priv_data *priv = DRVDATA(card); struct sc_security_env *env = NULL; struct sc_apdu apdu; - u8 sbuf[SIGNATURE_PAYLOAD_SIZE] = {0}; + u8 sbuf[SIGNATURE_PAYLOAD_SIZE]; + int le = MIN(SC_MAX_APDU_RESP_SIZE, MIN(SIGNATURE_PAYLOAD_SIZE * 2, outlen)); LOG_FUNC_CALLED(card->ctx); if (data == NULL || out == NULL || datalen > SIGNATURE_PAYLOAD_SIZE) @@ -204,41 +204,43 @@ static int esteid_compute_signature(sc_card_t *card, const u8 *data, size_t data switch (env->key_ref[0]) { case 1: /* authentication key */ - sc_format_apdu_ex(card, &apdu, 0x88, 0, 0, sbuf, datalen, out, MIN(256, MIN(SIGNATURE_PAYLOAD_SIZE * 2, outlen))); + sc_format_apdu_ex(card, &apdu, 0x88, 0, 0, sbuf, datalen, out, le); break; default: - sc_format_apdu_ex(card, &apdu, 0x2A, 0x9E, 0x9A, sbuf, datalen, out, MIN(256, MIN(SIGNATURE_PAYLOAD_SIZE * 2, outlen))); + sc_format_apdu_ex(card, &apdu, 0x2A, 0x9E, 0x9A, sbuf, datalen, out, le); } SC_TRANSMIT_TEST_RET(card, apdu, "PSO CDS/INTERNAL AUTHENTICATE failed"); - LOG_FUNC_RETURN(card->ctx, apdu.resplen); + LOG_FUNC_RETURN(card->ctx, (int)apdu.resplen); } static int esteid_get_pin_remaining_tries(sc_card_t *card, int pin_reference) { unsigned char get_pin_info[] = {0x4D, 0x08, 0x70, 0x06, 0xBF, 0x81, 0xFF, 0x02, 0xA0, 0x80}; struct sc_apdu apdu; - unsigned char apdu_resp[SC_MAX_APDU_BUFFER_SIZE]; + unsigned char apdu_resp[SC_MAX_APDU_RESP_SIZE]; LOG_FUNC_CALLED(card->ctx); // We don't get the file information here, so we need to be ugly - if (pin_reference == 1 || pin_reference == 2) { + if (pin_reference == PIN1_REF || pin_reference == PUK_REF) { LOG_TEST_RET(card->ctx, esteid_select(card, 0x00, 0x3F, 0x00), "Cannot select MF"); - } else if (pin_reference == 0x85) { + } else if (pin_reference == PIN2_REF) { LOG_TEST_RET(card->ctx, esteid_select_file(card, &adf2, NULL), "Cannot select QSCD AID"); } else { LOG_FUNC_RETURN(card->ctx, SC_ERROR_INTERNAL); } get_pin_info[6] = pin_reference & 0x0F; // mask out local/global - // XXX: handling of default Le of 0x00 could be easier - sc_format_apdu_ex(card, &apdu, 0xCB, 0x3F, 0xFF, get_pin_info, sizeof(get_pin_info), apdu_resp, MIN(256, sizeof(apdu_resp))); + sc_format_apdu_ex(card, &apdu, 0xCB, 0x3F, 0xFF, get_pin_info, sizeof(get_pin_info), apdu_resp, sizeof(apdu_resp)); SC_TRANSMIT_TEST_RET(card, apdu, "GET DATA(pin info) failed"); + if (apdu.resplen < 32) { + LOG_FUNC_RETURN(card->ctx, SC_ERROR_INTERNAL); + } // XXX: sc_asn1_find_tag with the following payload (to get to tag 0x9B): // https://lapo.it/asn1js/#cB6_gQEaoBiaAQObAQOhEIwG8wAAc0MAnAbzAABzQwA - return apdu_resp[13]; + return (int)apdu_resp[13]; } static int esteid_pin_cmd(sc_card_t *card, struct sc_pin_cmd_data *data, int *tries_left) { @@ -260,17 +262,16 @@ static int esteid_pin_cmd(sc_card_t *card, struct sc_pin_cmd_data *data, int *tr // VERIFY memcpy(&tmp, data, sizeof(struct sc_pin_cmd_data)); tmp.cmd = SC_PIN_CMD_VERIFY; - tmp.pin_reference = 0x02; // hardcoded, ugly + tmp.pin_reference = PUK_REF; tmp.pin2.len = 0; r = iso_ops->pin_cmd(card, &tmp, tries_left); - sc_mem_clear(&tmp, sizeof(tmp)); LOG_TEST_RET(card->ctx, r, "VERIFY during unblock failed"); if (data->pin_reference == 0x85) { LOG_TEST_RET(card->ctx, esteid_select_file(card, &adf2, NULL), "Cannot select QSCD AID"); } // UNBLOCK - memcpy(&tmp, data, sizeof(struct sc_pin_cmd_data)); + tmp = *data; tmp.cmd = SC_PIN_CMD_UNBLOCK; tmp.pin1.len = 0; r = iso_ops->pin_cmd(card, &tmp, tries_left); diff --git a/src/libopensc/pkcs15-esteid2018.c b/src/libopensc/pkcs15-esteid2018.c index c542a541..af87550a 100644 --- a/src/libopensc/pkcs15-esteid2018.c +++ b/src/libopensc/pkcs15-esteid2018.c @@ -41,7 +41,7 @@ static void set_string(char **strp, const char *value) { static int sc_pkcs15emu_esteid2018_init(sc_pkcs15_card_t *p15card) { sc_card_t *card = p15card->card; - u8 buff[128]; + u8 buff[11]; int r, i; size_t field_length = 0, taglen, j; sc_path_t tmppath; @@ -49,11 +49,11 @@ static int sc_pkcs15emu_esteid2018_init(sc_pkcs15_card_t *p15card) { set_string(&p15card->tokeninfo->label, "ID-kaart"); set_string(&p15card->tokeninfo->manufacturer_id, "IDEMIA"); - /* Read docnr */ + /* Read documber number to be used as serial */ sc_format_path("3F00D003", &tmppath); LOG_TEST_RET(card->ctx, sc_select_file(card, &tmppath, NULL), "SELECT docnr"); r = sc_read_binary(card, 0, buff, 11, 0); - LOG_TEST_RET(card->ctx, r, "read docnr failed"); + LOG_TEST_RET(card->ctx, r, "read document number failed"); const unsigned char *tag = sc_asn1_find_tag(card->ctx, buff, (size_t)r, 0x04, &taglen); if (tag == NULL) LOG_FUNC_RETURN(card->ctx, SC_ERROR_INTERNAL); @@ -99,7 +99,7 @@ static int sc_pkcs15emu_esteid2018_init(sc_pkcs15_card_t *p15card) { if (cert->key->algorithm == SC_ALGORITHM_EC) field_length = cert->key->u.ec.params.field_length; - static const struct sc_object_id cn_oid = {{2, 5, 4, 3, -1}}; + const struct sc_object_id cn_oid = {{2, 5, 4, 3, -1}}; u8 *cn_name = NULL; size_t cn_len = 0; sc_pkcs15_get_name_from_dn(card->ctx, cert->subject, cert->subject_len, &cn_oid, &cn_name, &cn_len);