From 29961974e140d68c69d530f47c70d2624739540f Mon Sep 17 00:00:00 2001 From: Charles-Edouard de la Vergne Date: Thu, 29 Feb 2024 16:54:23 +0100 Subject: [PATCH] Fix Supported curves --- doc/user/app-openpgp.rst | 6 ++--- src/gpg_data.c | 53 +++++++++++++++++++++++++++------------- src/gpg_gen.c | 14 +++++------ src/gpg_ux.h | 3 +-- src/gpg_ux_nanos.c | 4 +-- src/gpg_ux_nanox.c | 4 +-- src/gpg_ux_nbgl.c | 14 +++++------ 7 files changed, 58 insertions(+), 40 deletions(-) diff --git a/doc/user/app-openpgp.rst b/doc/user/app-openpgp.rst index cfd7339..73e1c96 100644 --- a/doc/user/app-openpgp.rst +++ b/doc/user/app-openpgp.rst @@ -51,9 +51,9 @@ This specification is available in doc directory at [G10CODE]_. The application supports: - RSA with key up to 3072 bits -- ECDSA with secp256k1, secp256r1, brainpool 256r1 and brainpool 256t1 curves +- ECDSA with secp256R1 - EDDSA with Ed25519 curve -- ECDH with secp256k1, secp256r1, brainpool 256r1, brainpool 256t1 and curve25519 curves +- ECDH with secp256R1 and curve25519 curves This release has known missing parts (see also [GPGADD]_): @@ -154,7 +154,7 @@ The full menu layout is: | Choose Type... | RSA 2048 | RSA 3072 - | NIST P256 + | SECP 256R1 | ED25519 | Set Template | Seed mode *ON/OFF* diff --git a/src/gpg_data.c b/src/gpg_data.c index a2ef9de..6b24683 100644 --- a/src/gpg_data.c +++ b/src/gpg_data.c @@ -231,6 +231,8 @@ int gpg_apdu_put_data(unsigned int ref) { void *pkey = NULL; cx_aes_key_t aes_key = {0}; cx_err_t error = CX_INTERNAL_ERROR; + unsigned int pkey_size = 0; + unsigned int ksz, curve; G_gpg_vstate.DO_current = ref; @@ -326,7 +328,7 @@ int gpg_apdu_put_data(unsigned int ref) { /* ----------------- Extended Header list -----------------*/ case 0x3FFF: { unsigned int len_e, len_p, len_q; - unsigned int endof, ksz, reset_cnt; + unsigned int endof, reset_cnt; gpg_key_t *keygpg = NULL; // fecth 4D gpg_io_fetch_tl(&t, &l); @@ -400,7 +402,6 @@ int gpg_apdu_put_data(unsigned int ref) { unsigned char *p, *q, *pq; cx_rsa_public_key_t *rsa_pub; cx_rsa_private_key_t *rsa_priv; - unsigned int pkey_size = 0; // check length ksz = U2BE(keygpg->attributes.value, 1) >> 3; rsa_pub = (cx_rsa_public_key_t *) &G_gpg_vstate.work.rsa.public; @@ -476,12 +477,9 @@ int gpg_apdu_put_data(unsigned int ref) { nvm_write(&G_gpg_vstate.kslot->sig_count, &reset_cnt, sizeof(unsigned int)); } sw = SW_OK; - } - else if ((keygpg->attributes.value[0] == KEY_ID_ECDH) || - (keygpg->attributes.value[0] == KEY_ID_ECDSA) || - (keygpg->attributes.value[0] == KEY_ID_EDDSA)) { - unsigned int curve; - + } else if ((keygpg->attributes.value[0] == KEY_ID_ECDH) || + (keygpg->attributes.value[0] == KEY_ID_ECDSA) || + (keygpg->attributes.value[0] == KEY_ID_EDDSA)) { curve = gpg_oid2curve(&keygpg->attributes.value[1], keygpg->attributes.length - 1); if (curve == CX_CURVE_NONE) { sw = SW_WRONG_DATA; @@ -637,9 +635,35 @@ int gpg_apdu_put_data(unsigned int ref) { sw = SW_WRONG_LENGTH; break; } - nvm_write(ptr_v, G_gpg_vstate.work.io_buffer, G_gpg_vstate.io_length); - nvm_write(ptr_l, &G_gpg_vstate.io_length, sizeof(unsigned int)); - sw = SW_OK; + switch (G_gpg_vstate.work.io_buffer[0]) { + case KEY_ID_RSA: + ksz = U2BE(G_gpg_vstate.work.io_buffer, 1); + if ((ksz != 2048) && (ksz != 3072)) { + sw = SW_WRONG_DATA; + } else { + sw = SW_OK; + } + break; + case KEY_ID_ECDH: + case KEY_ID_ECDSA: + case KEY_ID_EDDSA: + curve = + gpg_oid2curve(G_gpg_vstate.work.io_buffer + 1, G_gpg_vstate.io_length - 1); + if (curve == CX_CURVE_NONE) { + sw = SW_WRONG_DATA; + } else { + sw = SW_OK; + } + break; + default: + sw = SW_WRONG_DATA; + break; + } + + if (sw == SW_OK) { + nvm_write(ptr_v, G_gpg_vstate.work.io_buffer, G_gpg_vstate.io_length); + nvm_write(ptr_l, &G_gpg_vstate.io_length, sizeof(unsigned int)); + } break; /* ----------------- PWS status ----------------- */ @@ -961,7 +985,6 @@ int gpg_apdu_put_key_data(unsigned int ref) { break; } offset = G_gpg_vstate.io_offset; - ksz = U2BE(G_gpg_vstate.mse_dec->attributes.value, 1) >> 3; switch (ksz) { case 2048 / 8: @@ -981,12 +1004,10 @@ int gpg_apdu_put_key_data(unsigned int ref) { } if ((key == NULL) || (key->size != ksz)) { - PRINTF("[DATA] - put_key_data: Wrong key len: %d / %d\n", ksz, key->size); sw = SW_CONDITIONS_NOT_SATISFIED; break; } if (len != GPG_IO_BUFFER_LENGTH) { - PRINTF("[DATA] - put_key_data: Wrong buffer len: %d / %d\n", len, GPG_IO_BUFFER_LENGTH); sw = SW_CONDITIONS_NOT_SATISFIED; break; } @@ -1004,9 +1025,7 @@ int gpg_apdu_put_key_data(unsigned int ref) { sw = SW_WRONG_DATA; break; } - nvm_write((unsigned char *) key, - G_gpg_vstate.work.io_buffer, - len); + nvm_write((unsigned char *) key, G_gpg_vstate.work.io_buffer, len); sw = SW_OK; break; diff --git a/src/gpg_gen.c b/src/gpg_gen.c index c3b8986..9c85e3f 100644 --- a/src/gpg_gen.c +++ b/src/gpg_gen.c @@ -230,7 +230,7 @@ static int gpg_read_ecc_kyey(gpg_key_t *keygpg) { uint32_t i, len; cx_err_t error = CX_INTERNAL_ERROR; - if (keygpg->pub_key.ecfp256.W_len == 0) { + if (keygpg->pub_key.ecfp.W_len == 0) { return SW_REFERENCED_DATA_NOT_FOUND; } gpg_io_discard(1); @@ -238,23 +238,23 @@ static int gpg_read_ecc_kyey(gpg_key_t *keygpg) { curve = gpg_oid2curve(keygpg->attributes.value + 1, keygpg->attributes.length - 1); if (curve == CX_CURVE_Ed25519) { memmove(G_gpg_vstate.work.io_buffer + 128, - keygpg->pub_key.ecfp256.W, - keygpg->pub_key.ecfp256.W_len); + keygpg->pub_key.ecfp.W, + keygpg->pub_key.ecfp.W_len); CX_CHECK(cx_edwards_compress_point_no_throw(CX_CURVE_Ed25519, G_gpg_vstate.work.io_buffer + 128, 65)); gpg_io_insert_tlv(0x86, 32, G_gpg_vstate.work.io_buffer + 129); // 129: discard 02 } else if (curve == CX_CURVE_Curve25519) { - len = keygpg->pub_key.ecfp256.W_len - 1; + len = keygpg->pub_key.ecfp.W_len - 1; for (i = 0; i <= len; i++) { - G_gpg_vstate.work.io_buffer[128 + i] = keygpg->pub_key.ecfp256.W[len - i]; + G_gpg_vstate.work.io_buffer[128 + i] = keygpg->pub_key.ecfp.W[len - i]; } gpg_io_insert_tlv(0x86, 32, G_gpg_vstate.work.io_buffer + 128); } else { gpg_io_insert_tlv(0x86, - keygpg->pub_key.ecfp256.W_len, - (unsigned char *) &keygpg->pub_key.ecfp256.W); + keygpg->pub_key.ecfp.W_len, + (unsigned char *) &keygpg->pub_key.ecfp.W); } return SW_OK; diff --git a/src/gpg_ux.h b/src/gpg_ux.h index 207643d..fc7a63d 100644 --- a/src/gpg_ux.h +++ b/src/gpg_ux.h @@ -29,8 +29,7 @@ #define LABEL_RSA2048 "RSA 2048" #define LABEL_RSA3072 "RSA 3072" #define LABEL_RSA4096 "RSA 4096" -#define LABEL_NISTP256 "NIST P256" -#define LABEL_SECP256K1 "SECP 256K1" +#define LABEL_SECP256R1 "SECP 256R1" #define LABEL_Ed25519 "Ed25519" void ui_CCID_reset(void); diff --git a/src/gpg_ux_nanos.c b/src/gpg_ux_nanos.c index a5391b7..9dbdc7f 100644 --- a/src/gpg_ux_nanos.c +++ b/src/gpg_ux_nanos.c @@ -539,7 +539,7 @@ const ux_menu_entry_t ui_menu_tmpl_type[] = { #ifdef WITH_SUPPORT_RSA4096 {NULL, ui_menu_tmpl_type_action, 4096, NULL, LABEL_RSA4096, NULL, 0, 0}, #endif - {NULL, ui_menu_tmpl_type_action, CX_CURVE_SECP256R1, NULL, LABEL_NISTP256, NULL, 0, 0}, + {NULL, ui_menu_tmpl_type_action, CX_CURVE_SECP256R1, NULL, LABEL_SECP256R1, NULL, 0, 0}, {NULL, ui_menu_tmpl_type_action, CX_CURVE_Ed25519, NULL, LABEL_Ed25519, NULL, 0, 0}, {ui_menu_template, NULL, 0, &C_icon_back, "Back", NULL, 61, 40}, UX_MENU_END}; @@ -578,7 +578,7 @@ const bagl_element_t *ui_menu_template_predisplay(const ux_menu_entry_t *entry, break; #endif case CX_CURVE_SECP256R1: - snprintf(G_gpg_vstate.menu, sizeof(G_gpg_vstate.menu), " %s", LABEL_NISTP256); + snprintf(G_gpg_vstate.menu, sizeof(G_gpg_vstate.menu), " %s", LABEL_SECP256R1); break; case CX_CURVE_Ed25519: snprintf(G_gpg_vstate.menu, sizeof(G_gpg_vstate.menu), " %s", LABEL_Ed25519); diff --git a/src/gpg_ux_nanox.c b/src/gpg_ux_nanox.c index bb9dd71..efdaf45 100644 --- a/src/gpg_ux_nanox.c +++ b/src/gpg_ux_nanox.c @@ -477,7 +477,7 @@ const char *const tmpl_type_getter_values[] = {LABEL_RSA2048, #ifdef WITH_SUPPORT_RSA4096 LABEL_RSA4096, #endif - LABEL_SECP256K1, + LABEL_SECP256R1, LABEL_Ed25519}; const unsigned int tmpl_type_getter_values_map[] = {2048, @@ -577,7 +577,7 @@ void ui_menu_template_predisplay() { break; #endif case CX_CURVE_SECP256R1: - snprintf(KEY_TYPE, sizeof(KEY_TYPE), " %s", LABEL_SECP256K1); + snprintf(KEY_TYPE, sizeof(KEY_TYPE), " %s", LABEL_SECP256R1); break; case CX_CURVE_Ed25519: snprintf(KEY_TYPE, sizeof(KEY_TYPE), " %s", LABEL_Ed25519); diff --git a/src/gpg_ux_nbgl.c b/src/gpg_ux_nbgl.c index a587fee..b118995 100644 --- a/src/gpg_ux_nbgl.c +++ b/src/gpg_ux_nbgl.c @@ -207,7 +207,7 @@ enum { #ifdef WITH_SUPPORT_RSA4096 TOKEN_TYPE_RSA4096, #endif - TOKEN_TYPE_SECP256K1, + TOKEN_TYPE_SECP256R1, TOKEN_TYPE_Ed25519, TOKEN_TYPE_BACK }; @@ -217,7 +217,7 @@ static const char* const keyTypeTexts[] = {LABEL_RSA2048, #ifdef WITH_SUPPORT_RSA4096 LABEL_RSA4096, #endif - LABEL_SECP256K1, + LABEL_SECP256R1, LABEL_Ed25519}; static uint32_t _getKeyType(const uint8_t key) { @@ -260,7 +260,7 @@ static uint32_t _getKeyType(const uint8_t key) { tag = attributes[1]; switch (tag) { case 0x2A: - token = TOKEN_TYPE_SECP256K1; + token = TOKEN_TYPE_SECP256R1; break; case 0x2B: token = TOKEN_TYPE_Ed25519; @@ -268,7 +268,7 @@ static uint32_t _getKeyType(const uint8_t key) { } break; case KEY_ID_ECDSA: - token = TOKEN_TYPE_SECP256K1; + token = TOKEN_TYPE_SECP256R1; break; case KEY_ID_EDDSA: token = TOKEN_TYPE_Ed25519; @@ -315,7 +315,7 @@ static void template_key_cb(int token, uint8_t index) { oid_len = 6; break; - case TOKEN_TYPE_SECP256K1: + case TOKEN_TYPE_SECP256R1: if (G_gpg_vstate.ux_key == TOKEN_TEMPLATE_DEC) { attributes.value[0] = KEY_ID_ECDH; } else { @@ -409,8 +409,8 @@ static void ui_settings_template(void) { bar.subText = PIC(LABEL_RSA4096); break; #endif - case TOKEN_TYPE_SECP256K1: - bar.subText = PIC(LABEL_SECP256K1); + case TOKEN_TYPE_SECP256R1: + bar.subText = PIC(LABEL_SECP256R1); break; case TOKEN_TYPE_Ed25519: bar.subText = PIC(LABEL_Ed25519);