Skip to content

Commit

Permalink
Fix Supported curves
Browse files Browse the repository at this point in the history
  • Loading branch information
@cedelavergne-ledger
cedelavergne-ledger committed Feb 29, 2024
1 parent f60feb0 commit 2996197
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 40 deletions.
6 changes: 3 additions & 3 deletions doc/user/app-openpgp.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]_):

Expand Down Expand Up @@ -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*
Expand Down
53 changes: 36 additions & 17 deletions src/gpg_data.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 ----------------- */
Expand Down Expand Up @@ -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:
Expand All @@ -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;
}
Expand All @@ -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;

Expand Down
14 changes: 7 additions & 7 deletions src/gpg_gen.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,31 +230,31 @@ 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);
gpg_io_mark();
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;

Expand Down
3 changes: 1 addition & 2 deletions src/gpg_ux.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/gpg_ux_nanos.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/gpg_ux_nanox.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 7 additions & 7 deletions src/gpg_ux_nbgl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Expand All @@ -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) {
Expand Down Expand Up @@ -260,15 +260,15 @@ 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;
break;
}
break;
case KEY_ID_ECDSA:
token = TOKEN_TYPE_SECP256K1;
token = TOKEN_TYPE_SECP256R1;
break;
case KEY_ID_EDDSA:
token = TOKEN_TYPE_Ed25519;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 2996197

Please sign in to comment.