Skip to content

Commit

Permalink
Unwind multiprime RSA support.
Browse files Browse the repository at this point in the history 
FIPS is not compatible with multiprime RSA. Any multiprime RSA private
keys will fail to parse after this change.

Change-Id: I8d969d668bf0be4f66c66a30e56f0e7f6795f3e9
Reviewed-on: https://boringssl-review.googlesource.com/14984
Reviewed-by: Adam Langley <agl@google.com>
  • Loading branch information
@davidben @agl
davidben authored and agl committed Apr 12, 2017
1 parent fb8b763 commit 82b2b85
Show file tree
Hide file tree
Showing 12 changed files with 14 additions and 680 deletions.
31 changes: 0 additions & 31 deletions crypto/evp/print.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,6 @@ static int do_rsa_print(BIO *out, const RSA *rsa, int off,
update_buflen(rsa->dmp1, &buf_len);
update_buflen(rsa->dmq1, &buf_len);
update_buflen(rsa->iqmp, &buf_len);

if (rsa->additional_primes != NULL) {
for (size_t i = 0;
i < sk_RSA_additional_prime_num(rsa->additional_primes); i++) {
const RSA_additional_prime *ap =
sk_RSA_additional_prime_value(rsa->additional_primes, i);
update_buflen(ap->prime, &buf_len);
update_buflen(ap->exp, &buf_len);
update_buflen(ap->coeff, &buf_len);
}
}
}

m = (uint8_t *)OPENSSL_malloc(buf_len + 10);
Expand Down Expand Up @@ -204,26 +193,6 @@ static int do_rsa_print(BIO *out, const RSA *rsa, int off,
!bn_print(out, "coefficient:", rsa->iqmp, m, off)) {
goto err;
}

if (rsa->additional_primes != NULL &&
sk_RSA_additional_prime_num(rsa->additional_primes) > 0) {
if (BIO_printf(out, "otherPrimeInfos:\n") <= 0) {
goto err;
}
for (size_t i = 0;
i < sk_RSA_additional_prime_num(rsa->additional_primes); i++) {
const RSA_additional_prime *ap =
sk_RSA_additional_prime_value(rsa->additional_primes, i);

if (BIO_printf(out, "otherPrimeInfo (prime %u):\n",
(unsigned)(i + 3)) <= 0 ||
!bn_print(out, "prime:", ap->prime, m, off) ||
!bn_print(out, "exponent:", ap->exp, m, off) ||
!bn_print(out, "coeff:", ap->coeff, m, off)) {
goto err;
}
}
}
}
ret = 1;

Expand Down
20 changes: 0 additions & 20 deletions crypto/rsa/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,26 +117,6 @@ int RSA_private_transform(RSA *rsa, uint8_t *out, const uint8_t *in,
size_t len);


/* RSA_additional_prime contains information about the third, forth etc prime
* in a multi-prime RSA key. */
typedef struct RSA_additional_prime_st {
BIGNUM *prime;
/* exp is d^{prime-1} mod prime */
BIGNUM *exp;
/* coeff is such that r×coeff ≡ 1 mod prime. */
BIGNUM *coeff;

/* Values below here are not in the ASN.1 serialisation. */

/* r is the product of all primes (including p and q) prior to this one. */
BIGNUM *r;
/* mont is a |BN_MONT_CTX| modulo |prime|. */
BN_MONT_CTX *mont;
} RSA_additional_prime;

void RSA_additional_prime_free(RSA_additional_prime *ap);


/* The following utility functions are exported for test purposes. */

extern const BN_ULONG kBoringSSLRSASqrtTwo[];
Expand Down
41 changes: 1 addition & 40 deletions crypto/rsa/rsa.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,19 +110,6 @@ RSA *RSA_new_method(const ENGINE *engine) {
return rsa;
}

void RSA_additional_prime_free(RSA_additional_prime *ap) {
if (ap == NULL) {
return;
}

BN_clear_free(ap->prime);
BN_clear_free(ap->exp);
BN_clear_free(ap->coeff);
BN_clear_free(ap->r);
BN_MONT_CTX_free(ap->mont);
OPENSSL_free(ap);
}

void RSA_free(RSA *rsa) {
unsigned u;

Expand Down Expand Up @@ -157,10 +144,6 @@ void RSA_free(RSA *rsa) {
}
OPENSSL_free(rsa->blindings);
OPENSSL_free(rsa->blindings_inuse);
if (rsa->additional_primes != NULL) {
sk_RSA_additional_prime_pop_free(rsa->additional_primes,
RSA_additional_prime_free);
}
CRYPTO_MUTEX_cleanup(&rsa->lock);
OPENSSL_free(rsa);
}
Expand Down Expand Up @@ -584,23 +567,6 @@ int RSA_check_key(const RSA *key) {
goto out;
}

size_t num_additional_primes = 0;
if (key->additional_primes != NULL) {
num_additional_primes = sk_RSA_additional_prime_num(key->additional_primes);
}

for (size_t i = 0; i < num_additional_primes; i++) {
const RSA_additional_prime *ap =
sk_RSA_additional_prime_value(key->additional_primes, i);
if (!BN_mul(&n, &n, ap->prime, ctx) ||
!BN_sub(&pm1, ap->prime, BN_value_one()) ||
!BN_mul(&lcm, &lcm, &pm1, ctx) ||
!BN_gcd(&gcd, &gcd, &pm1, ctx)) {
OPENSSL_PUT_ERROR(RSA, ERR_LIB_BN);
goto out;
}
}

if (!BN_div(&lcm, NULL, &lcm, &gcd, ctx) ||
!BN_gcd(&gcd, &pm1, &qm1, ctx) ||
/* de = d*e mod lcm(prime-1, for all primes). */
Expand All @@ -626,7 +592,7 @@ int RSA_check_key(const RSA *key) {
goto out;
}

if (has_crt_values && num_additional_primes == 0) {
if (has_crt_values) {
if (/* dmp1 = d mod (p-1) */
!BN_mod(&dmp1, key->d, &pm1, ctx) ||
/* dmq1 = d mod (q-1) */
Expand Down Expand Up @@ -734,11 +700,6 @@ int RSA_recover_crt_params(RSA *rsa) {
return 0;
}

if (rsa->additional_primes != NULL) {
OPENSSL_PUT_ERROR(RSA, RSA_R_CANNOT_RECOVER_MULTI_PRIME_KEY);
return 0;
}

/* This uses the algorithm from section 9B of the RSA paper:
* http://people.csail.mit.edu/rivest/Rsapaper.pdf */

Expand Down
114 changes: 6 additions & 108 deletions crypto/rsa/rsa_asn1.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,36 +169,9 @@ int RSA_public_key_to_bytes(uint8_t **out_bytes, size_t *out_len,
return 1;
}

/* kVersionTwoPrime and kVersionMulti are the supported values of the version
* field of an RSAPrivateKey structure (RFC 3447). */
/* kVersionTwoPrime is the value of the version field for a two-prime
* RSAPrivateKey structure (RFC 3447). */
static const uint64_t kVersionTwoPrime = 0;
static const uint64_t kVersionMulti = 1;

/* rsa_parse_additional_prime parses a DER-encoded OtherPrimeInfo from |cbs| and
* advances |cbs|. It returns a newly-allocated |RSA_additional_prime| on
* success or NULL on error. The |r| and |mont| fields of the result are set to
* NULL. */
static RSA_additional_prime *rsa_parse_additional_prime(CBS *cbs) {
RSA_additional_prime *ret = OPENSSL_malloc(sizeof(RSA_additional_prime));
if (ret == NULL) {
OPENSSL_PUT_ERROR(RSA, ERR_R_MALLOC_FAILURE);
return 0;
}
OPENSSL_memset(ret, 0, sizeof(RSA_additional_prime));

CBS child;
if (!CBS_get_asn1(cbs, &child, CBS_ASN1_SEQUENCE) ||
!parse_integer(&child, &ret->prime) ||
!parse_integer(&child, &ret->exp) ||
!parse_integer(&child, &ret->coeff) ||
CBS_len(&child) != 0) {
OPENSSL_PUT_ERROR(RSA, RSA_R_BAD_ENCODING);
RSA_additional_prime_free(ret);
return NULL;
}

return ret;
}

RSA *RSA_parse_private_key(CBS *cbs) {
BN_CTX *ctx = NULL;
Expand All @@ -216,7 +189,7 @@ RSA *RSA_parse_private_key(CBS *cbs) {
goto err;
}

if (version != kVersionTwoPrime && version != kVersionMulti) {
if (version != kVersionTwoPrime) {
OPENSSL_PUT_ERROR(RSA, RSA_R_BAD_VERSION);
goto err;
}
Expand All @@ -232,50 +205,6 @@ RSA *RSA_parse_private_key(CBS *cbs) {
goto err;
}

if (version == kVersionMulti) {
/* Although otherPrimeInfos is written as OPTIONAL in RFC 3447, it later
* says "[otherPrimeInfos] shall be omitted if version is 0 and shall
* contain at least one instance of OtherPrimeInfo if version is 1." The
* OPTIONAL is just so both versions share a single definition. */
CBS other_prime_infos;
if (!CBS_get_asn1(&child, &other_prime_infos, CBS_ASN1_SEQUENCE) ||
CBS_len(&other_prime_infos) == 0) {
OPENSSL_PUT_ERROR(RSA, RSA_R_BAD_ENCODING);
goto err;
}
ret->additional_primes = sk_RSA_additional_prime_new_null();
if (ret->additional_primes == NULL) {
OPENSSL_PUT_ERROR(RSA, ERR_R_MALLOC_FAILURE);
goto err;
}

ctx = BN_CTX_new();
product_of_primes_so_far = BN_new();
if (ctx == NULL ||
product_of_primes_so_far == NULL ||
!BN_mul(product_of_primes_so_far, ret->p, ret->q, ctx)) {
goto err;
}

while (CBS_len(&other_prime_infos) > 0) {
RSA_additional_prime *ap = rsa_parse_additional_prime(&other_prime_infos);
if (ap == NULL) {
goto err;
}
if (!sk_RSA_additional_prime_push(ret->additional_primes, ap)) {
OPENSSL_PUT_ERROR(RSA, ERR_R_MALLOC_FAILURE);
RSA_additional_prime_free(ap);
goto err;
}
ap->r = BN_dup(product_of_primes_so_far);
if (ap->r == NULL ||
!BN_mul(product_of_primes_so_far, product_of_primes_so_far,
ap->prime, ctx)) {
goto err;
}
}
}

if (CBS_len(&child) != 0) {
OPENSSL_PUT_ERROR(RSA, RSA_R_BAD_ENCODING);
goto err;
Expand Down Expand Up @@ -310,49 +239,18 @@ RSA *RSA_private_key_from_bytes(const uint8_t *in, size_t in_len) {
}

int RSA_marshal_private_key(CBB *cbb, const RSA *rsa) {
const int is_multiprime =
sk_RSA_additional_prime_num(rsa->additional_primes) > 0;

CBB child;
if (!CBB_add_asn1(cbb, &child, CBS_ASN1_SEQUENCE) ||
!CBB_add_asn1_uint64(&child,
is_multiprime ? kVersionMulti : kVersionTwoPrime) ||
!CBB_add_asn1_uint64(&child, kVersionTwoPrime) ||
!marshal_integer(&child, rsa->n) ||
!marshal_integer(&child, rsa->e) ||
!marshal_integer(&child, rsa->d) ||
!marshal_integer(&child, rsa->p) ||
!marshal_integer(&child, rsa->q) ||
!marshal_integer(&child, rsa->dmp1) ||
!marshal_integer(&child, rsa->dmq1) ||
!marshal_integer(&child, rsa->iqmp)) {
OPENSSL_PUT_ERROR(RSA, RSA_R_ENCODE_ERROR);
return 0;
}

CBB other_prime_infos;
if (is_multiprime) {
if (!CBB_add_asn1(&child, &other_prime_infos, CBS_ASN1_SEQUENCE)) {
OPENSSL_PUT_ERROR(RSA, RSA_R_ENCODE_ERROR);
return 0;
}
for (size_t i = 0; i < sk_RSA_additional_prime_num(rsa->additional_primes);
i++) {
RSA_additional_prime *ap =
sk_RSA_additional_prime_value(rsa->additional_primes, i);
CBB other_prime_info;
if (!CBB_add_asn1(&other_prime_infos, &other_prime_info,
CBS_ASN1_SEQUENCE) ||
!marshal_integer(&other_prime_info, ap->prime) ||
!marshal_integer(&other_prime_info, ap->exp) ||
!marshal_integer(&other_prime_info, ap->coeff) ||
!CBB_flush(&other_prime_infos)) {
OPENSSL_PUT_ERROR(RSA, RSA_R_ENCODE_ERROR);
return 0;
}
}
}

if (!CBB_flush(cbb)) {
!marshal_integer(&child, rsa->iqmp) ||
!CBB_flush(cbb)) {
OPENSSL_PUT_ERROR(RSA, RSA_R_ENCODE_ERROR);
return 0;
}
Expand Down
34 changes: 0 additions & 34 deletions crypto/rsa/rsa_impl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -651,11 +651,6 @@ static int mod_exp(BIGNUM *r0, const BIGNUM *I, RSA *rsa, BN_CTX *ctx) {

BIGNUM *r1, *m1, *vrfy;
int ret = 0;
size_t i, num_additional_primes = 0;

if (rsa->additional_primes != NULL) {
num_additional_primes = sk_RSA_additional_prime_num(rsa->additional_primes);
}

BN_CTX_start(ctx);
r1 = BN_CTX_get(ctx);
Expand Down Expand Up @@ -733,31 +728,6 @@ static int mod_exp(BIGNUM *r0, const BIGNUM *I, RSA *rsa, BN_CTX *ctx) {
goto err;
}

for (i = 0; i < num_additional_primes; i++) {
/* multi-prime RSA. */
RSA_additional_prime *ap =
sk_RSA_additional_prime_value(rsa->additional_primes, i);

/* c will already point to a BIGNUM with the correct flags. */
if (!BN_mod(r1, I, ap->prime, ctx)) {
goto err;
}

if (!BN_MONT_CTX_set_locked(&ap->mont, &rsa->lock, ap->prime, ctx) ||
!BN_mod_exp_mont_consttime(m1, r1, ap->exp, ap->prime, ctx, ap->mont)) {
goto err;
}

if (!BN_sub(m1, m1, r0) ||
!BN_mul(m1, m1, ap->coeff, ctx) ||
!BN_mod(m1, m1, ap->prime, ctx) ||
(BN_is_negative(m1) && !BN_add(m1, m1, ap->prime)) ||
!BN_mul(m1, m1, ap->r, ctx) ||
!BN_add(r0, r0, m1)) {
goto err;
}
}

ret = 1;

err:
Expand Down Expand Up @@ -1032,10 +1002,6 @@ int rsa_default_keygen(RSA *rsa, int bits, BIGNUM *e_value, BN_GENCB *cb) {
goto bn_err;
}

sk_RSA_additional_prime_pop_free(rsa->additional_primes,
RSA_additional_prime_free);
rsa->additional_primes = NULL;

/* The key generation process is complex and thus error-prone. It could be
* disastrous to generate and then use a bad key so double-check that the key
* makes sense. */
Expand Down
Loading

0 comments on commit 82b2b85

Please sign in to comment.