From b203325ce112c223a5164081cecd14744a01ff69 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 4 May 2021 11:51:34 +0200 Subject: [PATCH] gpg: Allow ECDH with a smartcard returning just the x-coordinate. * g10/ecdh.c (pk_ecdh_encrypt_with_shared_point): Factor extraction part out to ... (extract_secret_x): new. Allow for x-only coordinate. (pk_ecdh_encrypt_with_shared_point): Change arg shared_mpi to (shared,nshared). Move param check to the top. Add extra safety check. (pk_ecdh_decrypt): Adjust for change. * g10/pkglue.c (get_data_from_sexp): New. (pk_encrypt): Use it for "s" and adjusted for changed pk_ecdh_encrypt_with_shared_point. * g10/pubkey-enc.c (get_it): Remove conversion to an MPI and call pk_ecdh_decrypt with the frame buffer. -- Backported-from-master: f129b0e97730b47d62482fba9599db39b526f3d2) Signed-off-by: Werner Koch --- g10/ecdh.c | 187 ++++++++++++++++++++++++++--------------------- g10/pkglue.c | 39 +++++++++- g10/pkglue.h | 5 +- g10/pubkey-enc.c | 12 +-- 4 files changed, 143 insertions(+), 100 deletions(-) diff --git a/g10/ecdh.c b/g10/ecdh.c index 5bbea96c0..97d483838 100644 --- a/g10/ecdh.c +++ b/g10/ecdh.c @@ -82,15 +82,73 @@ pk_ecdh_default_params (unsigned int qbits) } +/* Extract x-component from the point (SHARED,NSHARED) and strore it + * in a new buffer at R_SECRET_X. POINT_NBYTES is the size to + * represent an EC point which is determined by the public key. + * SECRET_X_SIZE is the size of x component to represent an integer + * which is determined by the curve. */ +static gpg_error_t +extract_secret_x (byte **r_secret_x, + const char *shared, size_t nshared, + size_t point_nbytes, size_t secret_x_size) +{ + byte *secret_x; + + *r_secret_x = NULL; + + /* Extract X from the result. It must be in the format of: + 04 || X || Y + 40 || X + 41 || X + + Since it may come with the prefix, the size of point is larger + than or equals to the size of an integer X. We also better check + that the provided shared point is not larger than the size needed + to represent the point. */ + if (point_nbytes < secret_x_size) + return gpg_error (GPG_ERR_BAD_DATA); + if (point_nbytes < nshared) + return gpg_error (GPG_ERR_BAD_DATA); + + /* Extract x component of the shared point: this is the actual + shared secret. */ + secret_x = xtrymalloc_secure (point_nbytes); + if (!secret_x) + return gpg_error_from_syserror (); + + memcpy (secret_x, shared, nshared); + + /* Wrangle the provided point unless only the x-component w/o any + * prefix was provided. */ + if (nshared != secret_x_size) + { + /* Remove the prefix. */ + if ((point_nbytes & 1)) + memmove (secret_x, secret_x+1, secret_x_size); + + /* Clear the rest of data. */ + if (point_nbytes - secret_x_size) + memset (secret_x+secret_x_size, 0, point_nbytes-secret_x_size); + } + + if (DBG_CRYPTO) + log_printhex (secret_x, secret_x_size, "ECDH shared secret X is:"); + + *r_secret_x = secret_x; + return 0; +} + + /* Encrypts/decrypts DATA using a key derived from the ECC shared - point SHARED_MPI using the FIPS SP 800-56A compliant method + point (SHARED,NSHARED) using the FIPS SP 800-56A compliant method key_derivation+key_wrapping. If IS_ENCRYPT is true the function encrypts; if false, it decrypts. PKEY is the public key and PK_FP the fingerprint of this public key. On success the result is stored at R_RESULT; on failure NULL is stored at R_RESULT and an error code returned. */ gpg_error_t -pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi, +pk_ecdh_encrypt_with_shared_point (int is_encrypt, + const char *shared, size_t nshared, const byte pk_fp[MAX_FINGERPRINT_LEN], gcry_mpi_t data, gcry_mpi_t *pkey, gcry_mpi_t *r_result) @@ -103,78 +161,15 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi, size_t kek_params_size; int kdf_hash_algo; int kdf_encr_algo; + size_t kek_size; unsigned char message[256]; size_t message_size; *r_result = NULL; - nbits = pubkey_nbits (PUBKEY_ALGO_ECDH, pkey); - if (!nbits) - return gpg_error (GPG_ERR_TOO_SHORT); - - { - size_t nbytes; - - /* Extract x component of the shared point: this is the actual - shared secret. */ - nbytes = (mpi_get_nbits (pkey[1] /* public point */)+7)/8; - secret_x = xtrymalloc_secure (nbytes); - if (!secret_x) - return gpg_error_from_syserror (); - - err = gcry_mpi_print (GCRYMPI_FMT_USG, secret_x, nbytes, - &nbytes, shared_mpi); - if (err) - { - xfree (secret_x); - log_error ("ECDH ephemeral export of shared point failed: %s\n", - gpg_strerror (err)); - return err; - } - - /* Expected size of the x component */ - secret_x_size = (nbits+7)/8; - - /* Extract X from the result. It must be in the format of: - 04 || X || Y - 40 || X - 41 || X - - Since it always comes with the prefix, it's larger than X. In - old experimental version of libgcrypt, there is a case where it - returns X with no prefix of 40, so, nbytes == secret_x_size - is allowed. */ - if (nbytes < secret_x_size) - { - xfree (secret_x); - return gpg_error (GPG_ERR_BAD_DATA); - } - - /* Remove the prefix. */ - if ((nbytes & 1)) - memmove (secret_x, secret_x+1, secret_x_size); - - /* Clear the rest of data. */ - if (nbytes - secret_x_size) - memset (secret_x+secret_x_size, 0, nbytes-secret_x_size); - - if (DBG_CRYPTO) - log_printhex (secret_x, secret_x_size, "ECDH shared secret X is:"); - } - - /*** We have now the shared secret bytes in secret_x. ***/ - - /* At this point we are done with PK encryption and the rest of the - * function uses symmetric key encryption techniques to protect the - * input DATA. The following two sections will simply replace - * current secret_x with a value derived from it. This will become - * a KEK. - */ if (!gcry_mpi_get_flag (pkey[2], GCRYMPI_FLAG_OPAQUE)) - { - xfree (secret_x); - return gpg_error (GPG_ERR_BUG); - } + return gpg_error (GPG_ERR_BUG); + kek_params = gcry_mpi_get_opaque (pkey[2], &nbits); kek_params_size = (nbits+7)/8; @@ -183,10 +178,7 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi, /* Expect 4 bytes 03 01 hash_alg symm_alg. */ if (kek_params_size != 4 || kek_params[0] != 3 || kek_params[1] != 1) - { - xfree (secret_x); - return gpg_error (GPG_ERR_BAD_PUBKEY); - } + return gpg_error (GPG_ERR_BAD_PUBKEY); kdf_hash_algo = kek_params[2]; kdf_encr_algo = kek_params[3]; @@ -199,17 +191,43 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi, if (kdf_hash_algo != GCRY_MD_SHA256 && kdf_hash_algo != GCRY_MD_SHA384 && kdf_hash_algo != GCRY_MD_SHA512) - { - xfree (secret_x); - return gpg_error (GPG_ERR_BAD_PUBKEY); - } + return gpg_error (GPG_ERR_BAD_PUBKEY); + if (kdf_encr_algo != CIPHER_ALGO_AES && kdf_encr_algo != CIPHER_ALGO_AES192 && kdf_encr_algo != CIPHER_ALGO_AES256) - { - xfree (secret_x); - return gpg_error (GPG_ERR_BAD_PUBKEY); - } + return gpg_error (GPG_ERR_BAD_PUBKEY); + + kek_size = gcry_cipher_get_algo_keylen (kdf_encr_algo); + if (kek_size > gcry_md_get_algo_dlen (kdf_hash_algo)) + return gpg_error (GPG_ERR_BAD_PUBKEY); + + + nbits = pubkey_nbits (PUBKEY_ALGO_ECDH, pkey); + if (!nbits) + return gpg_error (GPG_ERR_TOO_SHORT); + + /* Expected size of the x component */ + secret_x_size = (nbits+7)/8; + + if (kek_size > secret_x_size) + return gpg_error (GPG_ERR_BAD_PUBKEY); + + err = extract_secret_x (&secret_x, shared, nshared, + (mpi_get_nbits (pkey[1] /* public point */)+7)/8, + secret_x_size); + if (err) + return err; + + /*** We have now the shared secret bytes in secret_x. ***/ + + /* At this point we are done with PK encryption and the rest of the + * function uses symmetric key encryption techniques to protect the + * input DATA. The following two sections will simply replace + * current secret_x with a value derived from it. This will become + * a KEK. + */ + /* Build kdf_params. */ { @@ -484,12 +502,15 @@ pk_ecdh_generate_ephemeral_key (gcry_mpi_t *pkey, gcry_mpi_t *r_k) /* Perform ECDH decryption. */ int -pk_ecdh_decrypt (gcry_mpi_t * result, const byte sk_fp[MAX_FINGERPRINT_LEN], - gcry_mpi_t data, gcry_mpi_t shared, gcry_mpi_t * skey) +pk_ecdh_decrypt (gcry_mpi_t *result, const byte sk_fp[MAX_FINGERPRINT_LEN], + gcry_mpi_t data, + const char *shared, size_t nshared, + gcry_mpi_t *skey) { if (!data) return gpg_error (GPG_ERR_BAD_MPI); - return pk_ecdh_encrypt_with_shared_point (0 /*=decryption*/, shared, + return pk_ecdh_encrypt_with_shared_point (0 /*=decryption*/, + shared, nshared, sk_fp, data/*encr data as an MPI*/, skey, result); } diff --git a/g10/pkglue.c b/g10/pkglue.c index 8021a94db..e053657ee 100644 --- a/g10/pkglue.c +++ b/g10/pkglue.c @@ -47,6 +47,28 @@ get_mpi_from_sexp (gcry_sexp_t sexp, const char *item, int mpifmt) } +static byte * +get_data_from_sexp (gcry_sexp_t sexp, const char *item, size_t *r_size) +{ + gcry_sexp_t list; + size_t valuelen; + const char *value; + byte *v; + + if (DBG_CRYPTO) + log_printsexp ("get_data_from_sexp:", sexp); + + list = gcry_sexp_find_token (sexp, item, 0); + log_assert (list); + value = gcry_sexp_nth_data (list, 1, &valuelen); + log_assert (value); + v = xtrymalloc (valuelen); + memcpy (v, value, valuelen); + gcry_sexp_release (list); + *r_size = valuelen; + return v; +} + /**************** * Emulate our old PK interface here - sometime in the future we might @@ -309,12 +331,19 @@ pk_encrypt (pubkey_algo_t algo, gcry_mpi_t *resarr, gcry_mpi_t data, ; else if (algo == PUBKEY_ALGO_ECDH) { - gcry_mpi_t shared, public, result; + gcry_mpi_t public, result; byte fp[MAX_FINGERPRINT_LEN]; size_t fpn; + byte *shared; + size_t nshared; /* Get the shared point and the ephemeral public key. */ - shared = get_mpi_from_sexp (s_ciph, "s", GCRYMPI_FMT_USG); + shared = get_data_from_sexp (s_ciph, "s", &nshared); + if (!shared) + { + rc = gpg_error_from_syserror (); + goto leave; + } public = get_mpi_from_sexp (s_ciph, "e", GCRYMPI_FMT_USG); gcry_sexp_release (s_ciph); s_ciph = NULL; @@ -330,9 +359,10 @@ pk_encrypt (pubkey_algo_t algo, gcry_mpi_t *resarr, gcry_mpi_t data, if (fpn != 20) rc = gpg_error (GPG_ERR_INV_LENGTH); else - rc = pk_ecdh_encrypt_with_shared_point (1 /*=encrypton*/, shared, + rc = pk_ecdh_encrypt_with_shared_point (1 /*=encrypton*/, + shared, nshared, fp, data, pkey, &result); - gcry_mpi_release (shared); + xfree (shared); if (!rc) { resarr[0] = public; @@ -352,6 +382,7 @@ pk_encrypt (pubkey_algo_t algo, gcry_mpi_t *resarr, gcry_mpi_t data, resarr[1] = get_mpi_from_sexp (s_ciph, "b", GCRYMPI_FMT_USG); } + leave: gcry_sexp_release (s_ciph); return rc; } diff --git a/g10/pkglue.h b/g10/pkglue.h index 77a380191..5780e2356 100644 --- a/g10/pkglue.h +++ b/g10/pkglue.h @@ -36,7 +36,7 @@ int pk_check_secret_key (pubkey_algo_t algo, gcry_mpi_t *skey); gcry_mpi_t pk_ecdh_default_params (unsigned int qbits); gpg_error_t pk_ecdh_generate_ephemeral_key (gcry_mpi_t *pkey, gcry_mpi_t *r_k); gpg_error_t pk_ecdh_encrypt_with_shared_point -/* */ (int is_encrypt, gcry_mpi_t shared_mpi, +/* */ (int is_encrypt, const char *shared, size_t nshared, const byte pk_fp[MAX_FINGERPRINT_LEN], gcry_mpi_t data, gcry_mpi_t *pkey, gcry_mpi_t *out); @@ -44,7 +44,8 @@ gpg_error_t pk_ecdh_encrypt_with_shared_point int pk_ecdh_encrypt (gcry_mpi_t *resarr, const byte pk_fp[MAX_FINGERPRINT_LEN], gcry_mpi_t data, gcry_mpi_t * pkey); int pk_ecdh_decrypt (gcry_mpi_t *result, const byte sk_fp[MAX_FINGERPRINT_LEN], - gcry_mpi_t data, gcry_mpi_t shared, gcry_mpi_t * skey); + gcry_mpi_t data, const char *shared, size_t nshared, + gcry_mpi_t *skey); #endif /*GNUPG_G10_PKGLUE_H*/ diff --git a/g10/pubkey-enc.c b/g10/pubkey-enc.c index 30a4bc099..91dfb7798 100644 --- a/g10/pubkey-enc.c +++ b/g10/pubkey-enc.c @@ -264,20 +264,10 @@ get_it (ctrl_t ctrl, if (sk->pubkey_algo == PUBKEY_ALGO_ECDH) { - gcry_mpi_t shared_mpi; gcry_mpi_t decoded; - /* At the beginning the frame are the bytes of shared point MPI. */ - err = gcry_mpi_scan (&shared_mpi, GCRYMPI_FMT_USG, frame, nframe, NULL); - if (err) - { - err = gpg_error (GPG_ERR_WRONG_SECKEY); - goto leave; - } - err = pk_ecdh_decrypt (&decoded, fp, enc->data[1]/*encr data as an MPI*/, - shared_mpi, sk->pkey); - mpi_release (shared_mpi); + frame, nframe, sk->pkey); if(err) goto leave;