From bc3d003e00be00b2155a6d816e5df48d234164e8 Mon Sep 17 00:00:00 2001 From: NIIBE Yutaka Date: Wed, 20 May 2020 15:15:44 +0900 Subject: [PATCH] ecc-sos: Avoid using gcry_mpi_print and gcry_mpi_scan. Signed-off-by: NIIBE Yutaka --- g10/ecdh.c | 65 +++++++++++++++++++++++++++++------------------- g10/pkglue.c | 38 ++++++++++++++++++++++------ g10/pkglue.h | 9 ++++--- g10/pubkey-enc.c | 5 +--- g10/seskey.c | 11 ++------ 5 files changed, 79 insertions(+), 49 deletions(-) diff --git a/g10/ecdh.c b/g10/ecdh.c index b4d93be7d..46ac140f5 100644 --- a/g10/ecdh.c +++ b/g10/ecdh.c @@ -83,16 +83,18 @@ pk_ecdh_default_params (unsigned int qbits) /* Encrypts/decrypts DATA using a key derived from the ECC shared - point SHARED_MPI using the FIPS SP 800-56A compliant method + point SHARED 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, + const byte *data, size_t ndata, + gcry_mpi_t *pkey, gcry_mpi_t *r_result) { gpg_error_t err; @@ -118,7 +120,10 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi, /* 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 = gcry_mpi_get_opaque (shared_mpi, NULL); + secret_x = xtrymalloc_secure (nshared); + if (!secret_x) + return gpg_error_from_syserror (); + memcpy (secret_x, shared, nshared); /* Expected size of the x component */ secret_x_size = (nbits+7)/8; @@ -133,7 +138,10 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi, returns X with no prefix of 40, so, nbytes == secret_x_size is allowed. */ if (nbytes < secret_x_size) - return gpg_error (GPG_ERR_BAD_DATA); + { + xfree (secret_x); + return gpg_error (GPG_ERR_BAD_DATA); + } /* Remove the prefix. */ if ((nbytes & 1)) @@ -166,6 +174,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); } @@ -181,12 +190,14 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi, && kdf_hash_algo != GCRY_MD_SHA384 && kdf_hash_algo != GCRY_MD_SHA512) { + xfree (secret_x); 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); } @@ -210,6 +221,7 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi, iobuf_close (obuf); if (err) { + xfree (secret_x); return err; } @@ -227,6 +239,7 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi, { log_error ("gcry_md_open failed for kdf_hash_algo %d: %s", kdf_hash_algo, gpg_strerror (err)); + xfree (secret_x); return err; } gcry_md_write(h, "\x00\x00\x00\x01", 4); /* counter = 1 */ @@ -255,7 +268,6 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi, /* And, finally, aeswrap with key secret_x. */ { gcry_cipher_hd_t hd; - size_t nbytes; byte *data_buf; int data_buf_size; @@ -267,10 +279,13 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi, { log_error ("ecdh failed to initialize AESWRAP: %s\n", gpg_strerror (err)); + xfree (secret_x); return err; } err = gcry_cipher_setkey (hd, secret_x, secret_x_size); + secret_x = NULL; + xfree (secret_x); if (err) { gcry_cipher_close (hd); @@ -279,7 +294,7 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi, return err; } - data_buf_size = (gcry_mpi_get_nbits(data)+7)/8; + data_buf_size = ndata; if ((data_buf_size & 7) != (is_encrypt ? 0 : 1)) { log_error ("can't use a shared secret of %d bytes for ecdh\n", @@ -301,15 +316,7 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi, /* Write data MPI into the end of data_buf. data_buf is size aeswrap data. */ - err = gcry_mpi_print (GCRYMPI_FMT_USG, in, - data_buf_size, &nbytes, data/*in*/); - if (err) - { - log_error ("ecdh failed to export DEK: %s\n", gpg_strerror (err)); - gcry_cipher_close (hd); - xfree (data_buf); - return err; - } + memcpy (in, data, ndata); if (DBG_CRYPTO) log_printhex (in, data_buf_size, "ecdh encrypting :"); @@ -345,17 +352,14 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi, else { byte *in; - const void *p; - p = gcry_mpi_get_opaque (data, &nbits); - nbytes = (nbits+7)/8; - if (!p || nbytes > data_buf_size || !nbytes) + if (!data || ndata > data_buf_size || !ndata) { xfree (data_buf); return gpg_error (GPG_ERR_BAD_MPI); } - memcpy (data_buf, p, nbytes); - if (data_buf[0] != nbytes-1) + memcpy (data_buf, data, ndata); + if (data_buf[0] != ndata-1) { log_error ("ecdh inconsistent size\n"); xfree (data_buf); @@ -459,11 +463,20 @@ 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) + gcry_mpi_t data, + const byte *frame, size_t nframe, gcry_mpi_t * skey) { + int r; + byte *p; + unsigned int nbits; + if (!data) return gpg_error (GPG_ERR_BAD_MPI); - return pk_ecdh_encrypt_with_shared_point (0 /*=decryption*/, shared, - sk_fp, data/*encr data as an MPI*/, - skey, result); + + p = gcry_mpi_get_opaque (data, &nbits);/*encr data as an MPI*/ + + r = pk_ecdh_encrypt_with_shared_point (0 /*=decryption*/, frame, nframe, + sk_fp, p, (nbits+7)/8, + skey, result); + return r; } diff --git a/g10/pkglue.c b/g10/pkglue.c index 0d18cb931..7dadeb44a 100644 --- a/g10/pkglue.c +++ b/g10/pkglue.c @@ -47,6 +47,26 @@ 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; + + 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 @@ -332,15 +352,15 @@ 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_OPAQUE); + shared = get_data_from_sexp (s_ciph, "s", &nshared); public = get_mpi_from_sexp (s_ciph, "e", GCRYMPI_FMT_OPAQUE); - gcry_sexp_release (s_ciph); - s_ciph = NULL; if (DBG_CRYPTO) { log_debug ("ECDH ephemeral key:"); @@ -353,9 +373,13 @@ 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, - fp, data, pkey, &result); - gcry_mpi_release (shared); + { + unsigned int nbits; + byte *p = gcry_mpi_get_opaque (data, &nbits); + rc = pk_ecdh_encrypt_with_shared_point (1 /*=encrypton*/, shared, nshared, + fp, p, (nbits+7)/8, pkey, &result); + } + xfree (shared); if (!rc) { resarr[0] = public; diff --git a/g10/pkglue.h b/g10/pkglue.h index 77a380191..438a0c542 100644 --- a/g10/pkglue.h +++ b/g10/pkglue.h @@ -36,15 +36,18 @@ 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, + const byte *data, size_t ndata, + gcry_mpi_t *pkey, gcry_mpi_t *out); 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 byte *frame, size_t nframe, + gcry_mpi_t * skey); #endif /*GNUPG_G10_PKGLUE_H*/ diff --git a/g10/pubkey-enc.c b/g10/pubkey-enc.c index f796f39b5..38353c812 100644 --- a/g10/pubkey-enc.c +++ b/g10/pubkey-enc.c @@ -278,14 +278,11 @@ 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. */ - shared_mpi = gcry_mpi_set_opaque_copy (NULL, frame, nframe * 8); 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; diff --git a/g10/seskey.c b/g10/seskey.c index fb71ad5cd..15c210b78 100644 --- a/g10/seskey.c +++ b/g10/seskey.c @@ -82,7 +82,6 @@ encode_session_key (int openpgp_pk_algo, DEK *dek, unsigned int nbits) byte *frame; int i,n; u16 csum; - gcry_mpi_t a; if (DBG_CRYPTO) log_debug ("encode_session_key: encoding %d byte DEK", dek->keylen); @@ -124,10 +123,7 @@ encode_session_key (int openpgp_pk_algo, DEK *dek, unsigned int nbits) (int) nframe, frame[0], frame[1], frame[2], frame[nframe-3], frame[nframe-2], frame[nframe-1]); - if (gcry_mpi_scan (&a, GCRYMPI_FMT_USG, frame, nframe, &nframe)) - BUG(); - xfree(frame); - return a; + return gcry_mpi_set_opaque (NULL, frame, 8*nframe); } /* The current limitation is that we can only use a session key @@ -195,10 +191,7 @@ encode_session_key (int openpgp_pk_algo, DEK *dek, unsigned int nbits) frame[n++] = csum >>8; frame[n++] = csum; log_assert (n == nframe); - if (gcry_mpi_scan( &a, GCRYMPI_FMT_USG, frame, n, &nframe)) - BUG(); - xfree (frame); - return a; + return gcry_mpi_set_opaque (NULL, frame, 8*n); }