From 813f8d1b8e4b6c4365f0bd2a5305bdbe1e049d05 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 11 Apr 2024 15:56:21 +0200 Subject: [PATCH] gpg: Changed internal data format for Kyber. * g10/packet.h (PKT_pubkey_enc): Add field seskey_algo. (struct pubkey_enc_list): Ditto. * g10/misc.c (pubkey_get_nenc): Change value for Kyber from 4 to 3. * g10/parse-packet.c (parse_pubkeyenc): Store the Kyber algo in the new field and adjust data. Do not store the length byte in data[2]. * g10/build-packet.c (do_pubkey_enc): Take the session algo for Kyber from the new field. * g10/encrypt.c (write_pubkey_enc): Ses the seskey_algo. * g10/mainproc.c (proc_pubkey_enc): Copy it. * g10/pubkey-enc.c (get_it): Support Kyber decryption. * g10/seskey.c (encode_session_key): Handle Kyber different from ECDH. -- Having always the single byte in the packet data than to store and retrieve it from an MPI is much easier. Thus this patch changes the original internal format. With this chnages decryption of the slighly modified test data works now. See the bug tracker for test data. GnuPG-bug-id: 6815 --- g10/build-packet.c | 5 +++ g10/encrypt.c | 1 + g10/mainproc.c | 1 + g10/misc.c | 2 +- g10/packet.h | 3 ++ g10/parse-packet.c | 20 ++++++---- g10/pubkey-enc.c | 94 +++++++++++++++++++++++++++++++--------------- g10/seskey.c | 20 ++++++++-- 8 files changed, 104 insertions(+), 42 deletions(-) diff --git a/g10/build-packet.c b/g10/build-packet.c index 78828c8dc..fd315b313 100644 --- a/g10/build-packet.c +++ b/g10/build-packet.c @@ -982,6 +982,11 @@ do_pubkey_enc( IOBUF out, int ctb, PKT_pubkey_enc *enc ) for (i=0; i < n && !rc ; i++ ) { + /* For Kyber we need to insert the algo before the final data + * element because it is not stored in the data array. */ + if (enc->pubkey_algo == PUBKEY_ALGO_KYBER && i == 2) + iobuf_put (a, enc->seskey_algo); + if (enc->pubkey_algo == PUBKEY_ALGO_ECDH && i == 1) rc = gpg_mpi_write_opaque_nohdr (a, enc->data[i]); else if (enc->pubkey_algo == PUBKEY_ALGO_ECDH) diff --git a/g10/encrypt.c b/g10/encrypt.c index aa0c3c6dd..f416cde53 100644 --- a/g10/encrypt.c +++ b/g10/encrypt.c @@ -1122,6 +1122,7 @@ write_pubkey_enc (ctrl_t ctrl, enc->pubkey_algo = pk->pubkey_algo; keyid_from_pk( pk, enc->keyid ); enc->throw_keyid = throw_keyid; + enc->seskey_algo = dek->algo; /* (Used only by PUBKEY_ALGO_KYBER.) */ /* Okay, what's going on: We have the session key somewhere in * the structure DEK and want to encode this session key in an diff --git a/g10/mainproc.c b/g10/mainproc.c index 6f3254e88..48bc463c5 100644 --- a/g10/mainproc.c +++ b/g10/mainproc.c @@ -529,6 +529,7 @@ proc_pubkey_enc (CTX c, PACKET *pkt) x->keyid[1] = enc->keyid[1]; x->pubkey_algo = enc->pubkey_algo; x->result = -1; + x->seskey_algo = enc->seskey_algo; x->data[0] = x->data[1] = x->data[2] = x->data[3] = NULL; if (enc->data[0]) { diff --git a/g10/misc.c b/g10/misc.c index ce0c04b3b..1359987d2 100644 --- a/g10/misc.c +++ b/g10/misc.c @@ -1786,7 +1786,7 @@ pubkey_get_nenc (pubkey_algo_t algo) case PUBKEY_ALGO_ECDSA: return 0; case PUBKEY_ALGO_ELGAMAL: return 2; case PUBKEY_ALGO_EDDSA: return 0; - case PUBKEY_ALGO_KYBER: return 4; + case PUBKEY_ALGO_KYBER: return 3; default: return 0; } } diff --git a/g10/packet.h b/g10/packet.h index 40e5051c0..459e38dda 100644 --- a/g10/packet.h +++ b/g10/packet.h @@ -137,6 +137,8 @@ typedef struct { byte version; /* The algorithm used for the public key encryption scheme. */ byte pubkey_algo; + /* The session key algorithm used by some pubkey algos. */ + byte seskey_algo; /* Whether to hide the key id. This value is not directly serialized. */ byte throw_keyid; @@ -151,6 +153,7 @@ struct pubkey_enc_list struct pubkey_enc_list *next; u32 keyid[2]; int pubkey_algo; + int seskey_algo; int result; gcry_mpi_t data[PUBKEY_MAX_NENC]; }; diff --git a/g10/parse-packet.c b/g10/parse-packet.c index c55bb1b71..8bd283b4b 100644 --- a/g10/parse-packet.c +++ b/g10/parse-packet.c @@ -1513,7 +1513,7 @@ parse_pubkeyenc (IOBUF inp, int pkttype, unsigned long pktlen, } else if (k->pubkey_algo == PUBKEY_ALGO_KYBER) { - log_assert (ndata == 4); + log_assert (ndata == 3); /* Get the ephemeral public key. */ n = pktlen; k->data[0] = sos_read (inp, &n, 0); @@ -1527,12 +1527,16 @@ parse_pubkeyenc (IOBUF inp, int pkttype, unsigned long pktlen, rc = read_octet_string (inp, &pktlen, 4, 0, 0, k->data + 1); if (rc) goto leave; - /* Get the algorithm id. */ - rc = read_octet_string (inp, &pktlen, 0, 1, 0, k->data + 2); - if (rc) - goto leave; - /* Get the wrapped symmetric key. */ - rc = read_sized_octet_string (inp, &pktlen, k->data + 3); + /* Get the algorithm id for the session key. */ + if (!pktlen) + { + rc = gpg_error (GPG_ERR_INV_PACKET); + goto leave; + } + k->seskey_algo = iobuf_get_noeof (inp); + pktlen--; + /* Get the encrypted symmetric key. */ + rc = read_octet_string (inp, &pktlen, 1, 0, 0, k->data + 2); if (rc) goto leave; } @@ -1551,6 +1555,8 @@ parse_pubkeyenc (IOBUF inp, int pkttype, unsigned long pktlen, } if (list_mode) { + if (k->seskey_algo) + es_fprintf (listfp, "\tsession key algo: %d\n", k->seskey_algo); for (i = 0; i < ndata; i++) { es_fprintf (listfp, "\tdata: "); diff --git a/g10/pubkey-enc.c b/g10/pubkey-enc.c index 873b864b5..da32ebc7b 100644 --- a/g10/pubkey-enc.c +++ b/g10/pubkey-enc.c @@ -194,7 +194,7 @@ get_it (ctrl_t ctrl, { gpg_error_t err; byte *frame = NULL; - unsigned int n; + unsigned int frameidx; size_t nframe; u16 csum, csum2; int padding; @@ -240,13 +240,15 @@ get_it (ctrl_t ctrl, } else if (sk->pubkey_algo == PUBKEY_ALGO_KYBER) { - if (!enc->data[0] || !enc->data[1] || !enc->data[2] || !enc->data[3]) + log_debug ("seskey_algo: %d\n", enc->seskey_algo); + if (!enc->data[0] || !enc->data[1] || !enc->data[2]) err = gpg_error (GPG_ERR_BAD_MPI); else err = gcry_sexp_build (&s_data, NULL, - "(enc-val(pqc(e%m)(k%m)(s%m)(fixed-info%s)))", - enc->data[0], enc->data[1], enc->data[3], - "\x1d" /*PUBKEY_ALGO_KYBER*/); + "(enc-val(pqc(e%m)(k%m)(s%m)(c%d)(fixed-info%s)))", + enc->data[0], enc->data[1], enc->data[2], + enc->seskey_algo, + "\x69"); } else err = gpg_error (GPG_ERR_BUG); @@ -287,9 +289,22 @@ get_it (ctrl_t ctrl, */ if (DBG_CRYPTO) log_printhex (frame, nframe, "DEK frame:"); - n = 0; + frameidx = 0; - if (sk->pubkey_algo == PUBKEY_ALGO_ECDH) + if (sk->pubkey_algo == PUBKEY_ALGO_KYBER) + { + /* We expect a 32 byte session key. We should not see this + * error here because due to the KEM mode the agent_pkdecrypt + * should have already failed. */ + if (nframe != 32) + { + err = gpg_error (GPG_ERR_WRONG_SECKEY); + goto leave; + } + dek->keylen = nframe; + dek->algo = enc->seskey_algo; + } + else if (sk->pubkey_algo == PUBKEY_ALGO_ECDH) { gcry_mpi_t decoded; @@ -313,13 +328,21 @@ get_it (ctrl_t ctrl, goto leave; } nframe -= frame[nframe-1]; /* Remove padding. */ - log_assert (!n); /* (used just below) */ + if (4 > nframe) + { + err = gpg_error (GPG_ERR_WRONG_SECKEY); + goto leave; + } + + dek->keylen = nframe - 3; + dek->algo = frame[0]; + frameidx = 1; } else { if (padding) { - if (n + 7 > nframe) + if (7 > nframe) { err = gpg_error (GPG_ERR_WRONG_SECKEY); goto leave; @@ -331,34 +354,38 @@ get_it (ctrl_t ctrl, * using a Smartcard we are doing it the right way and * therefore we have to skip the zero. This should be fixed * in gpg-agent of course. */ - if (!frame[n]) - n++; + frameidx = 0; + if (!frame[frameidx]) + frameidx++; - if (frame[n] == 1 && frame[nframe - 1] == 2) + if (frame[frameidx] == 1 && frame[nframe - 1] == 2) { log_info (_("old encoding of the DEK is not supported\n")); err = gpg_error (GPG_ERR_CIPHER_ALGO); goto leave; } - if (frame[n] != 2) /* Something went wrong. */ + if (frame[frameidx] != 2) /* Something went wrong. */ { err = gpg_error (GPG_ERR_WRONG_SECKEY); goto leave; } - for (n++; n < nframe && frame[n]; n++) /* Skip the random bytes. */ + /* Skip the random bytes. */ + for (frameidx++; frameidx < nframe && frame[frameidx]; frameidx++) ; - n++; /* Skip the zero byte. */ + frameidx++; /* Skip the zero byte. */ } + + if (frameidx + 4 > nframe) + { + err = gpg_error (GPG_ERR_WRONG_SECKEY); + goto leave; + } + + dek->keylen = nframe - (frameidx + 1) - 2; + dek->algo = frame[frameidx++]; } - if (n + 4 > nframe) - { - err = gpg_error (GPG_ERR_WRONG_SECKEY); - goto leave; - } - - dek->keylen = nframe - (n + 1) - 2; - dek->algo = frame[n++]; + /* Check whether we support the ago. */ err = openpgp_cipher_test_algo (dek->algo); if (err) { @@ -377,16 +404,21 @@ get_it (ctrl_t ctrl, goto leave; } - /* Copy the key to DEK and compare the checksum. */ - csum = buf16_to_u16 (frame+nframe-2); - memcpy (dek->key, frame + n, dek->keylen); - for (csum2 = 0, n = 0; n < dek->keylen; n++) - csum2 += dek->key[n]; - if (csum != csum2) + /* Copy the key to DEK and compare the checksum if needed. */ + /* We use the frameidx as flag for the need of a checksum. */ + memcpy (dek->key, frame + frameidx, dek->keylen); + if (frameidx) { - err = gpg_error (GPG_ERR_WRONG_SECKEY); - goto leave; + csum = buf16_to_u16 (frame+nframe-2); + for (csum2 = 0, frameidx = 0; frameidx < dek->keylen; frameidx++) + csum2 += dek->key[frameidx]; + if (csum != csum2) + { + err = gpg_error (GPG_ERR_WRONG_SECKEY); + goto leave; + } } + if (DBG_CLOCK) log_clock ("decryption ready"); if (DBG_CRYPTO) diff --git a/g10/seskey.c b/g10/seskey.c index e5397080d..2fe8e9de7 100644 --- a/g10/seskey.c +++ b/g10/seskey.c @@ -86,15 +86,29 @@ encode_session_key (int openpgp_pk_algo, DEK *dek, unsigned int nbits) if (DBG_CRYPTO) log_debug ("encode_session_key: encoding %d byte DEK", dek->keylen); + if (openpgp_pk_algo == PUBKEY_ALGO_KYBER) + { + /* Straightforward encoding w/o extra checksum as used by ECDH. */ + nframe = dek->keylen; + log_assert (nframe > 4); /*(for the log_debug)*/ + frame = xmalloc_secure (nframe); + memcpy (frame, dek->key, nframe); + if (DBG_CRYPTO) + log_debug ("encode_session_key: " + "[%d] %02x %02x %02x ... %02x %02x %02x\n", + (int) dek->keylen, frame[0], frame[1], frame[2], + frame[nframe-3], frame[nframe-2], frame[nframe-1]); + + return gcry_mpi_set_opaque (NULL, frame, 8*nframe); + } + csum = 0; for (p = dek->key, i=0; i < dek->keylen; i++) csum += *p++; /* Shortcut for ECDH. It's padding is minimal to simply make the output be a multiple of 8 bytes. */ - /* FIXME: We use the ECDH also for Kyber for now. */ - if (openpgp_pk_algo == PUBKEY_ALGO_ECDH - || openpgp_pk_algo == PUBKEY_ALGO_KYBER) + if (openpgp_pk_algo == PUBKEY_ALGO_ECDH) { /* Pad to 8 byte granularity; the padding byte is the number of * padded bytes.