Skip to content

Commit

Permalink
RSA signature verification: Avoid wasteful key re-serialization.
Browse files Browse the repository at this point in the history
When we added `rsa::PublicKey` we changed the `ring::signature` RSA
implementation to construct an `rsa::PublicKey` and then verify the
signature using it. Unfortunately for backward compatibility with old
uses of `RsaKeyPair`, `rsa::PublicKey` constructor constructs (and
allocates) a copy of the ASN.1-serialized public key. This is not
acceptable for users who are using `ring::signature` to verify a
single signature. Refactor `PublicKey` so that it can be bypassed
by the `ring::signature` implementation.

This is a step towards implementing allocation-free RSA signature
verification.
  • Loading branch information
briansmith committed Nov 3, 2023
1 parent 6920c4f commit 8ed4860
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 32 deletions.
10 changes: 5 additions & 5 deletions src/rsa/keypair.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ impl KeyPair {
cpu_features,
)?;

let n = public_key.n().value();
let n = public_key.inner().n().value();

// 6.4.1.4.3 says to skip 6.4.1.2.1 Step 2.

Expand All @@ -323,7 +323,7 @@ impl KeyPair {
// TODO: First, stop if `p < (√2) * 2**((nBits/2) - 1)`.
//
// Second, stop if `p > 2**(nBits/2) - 1`.
let half_n_bits = public_key.n().len_bits().half_rounded_up();
let half_n_bits = public_key.inner().n().len_bits().half_rounded_up();
if p_bits != half_n_bits {
return Err(KeyRejected::inconsistent_components());
}
Expand Down Expand Up @@ -580,7 +580,7 @@ impl KeyPair {

// Use the output buffer as the scratch space for the signature to
// reduce the required stack space.
padding_alg.encode(m_hash, signature, self.public().n().len_bits(), rng)?;
padding_alg.encode(m_hash, signature, self.public().inner().n().len_bits(), rng)?;

// RFC 8017 Section 5.1.2: RSADP, using the Chinese Remainder Theorem
// with Garner's algorithm.
Expand All @@ -607,7 +607,7 @@ impl KeyPair {
// RFC 8017 Section 5.1.2: RSADP, using the Chinese Remainder Theorem
// with Garner's algorithm.

let n = self.public.n().value();
let n = self.public.inner().n().value();

// Step 1. The value zero is also rejected.
let base = bigint::Elem::from_be_bytes_padded(untrusted::Input::from(base), n)?;
Expand Down Expand Up @@ -648,7 +648,7 @@ impl KeyPair {
// minimum value, since the relationship of `e` to `d`, `p`, and `q` is
// not verified during `KeyPair` construction.
{
let verify = self.public.exponentiate_elem(m.clone());
let verify = self.public.inner().exponentiate_elem(m.clone());
bigint::elem_verify_equal_consttime(&verify, &c)?;
}

Expand Down
80 changes: 57 additions & 23 deletions src/rsa/public_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ use core::num::NonZeroU64;
/// An RSA Public Key.
#[derive(Clone)]
pub struct PublicKey {
n: PublicModulus,
e: PublicExponent,
inner: Inner,
serialized: Box<[u8]>,
}

Expand All @@ -41,9 +40,64 @@ impl PublicKey {
e_min_value: PublicExponent,
cpu_features: cpu::Features,
) -> Result<Self, error::KeyRejected> {
let inner = Inner::from_modulus_and_exponent(
n,
e,
n_min_bits,
n_max_bits,
e_min_value,
cpu_features,
)?;

let n_bytes = n;
let e_bytes = e;

// TODO: Remove this re-parsing, and stop allocating this here.
// Instead we should serialize on demand without allocation, from
// `Modulus::be_bytes()` and `Exponent::be_bytes()`. Once this is
// fixed, merge `Inner` back into `PublicKey`.
let n_bytes = io::Positive::from_be_bytes(n_bytes)
.map_err(|_: error::Unspecified| error::KeyRejected::unexpected_error())?;
let e_bytes = io::Positive::from_be_bytes(e_bytes)
.map_err(|_: error::Unspecified| error::KeyRejected::unexpected_error())?;
let serialized = der_writer::write_all(der::Tag::Sequence, &|output| {
der_writer::write_positive_integer(output, &n_bytes);
der_writer::write_positive_integer(output, &e_bytes);
});

Ok(Self { inner, serialized })
}

/// The length, in bytes, of the public modulus.
///
/// The modulus length is rounded up to a whole number of bytes if its
/// bit length isn't a multiple of 8.
pub fn modulus_len(&self) -> usize {
self.inner.n().len_bits().as_usize_bytes_rounded_up()
}

pub(super) fn inner(&self) -> &Inner {
&self.inner
}
}

/// `PublicKey` but without any superfluous allocations, optimized for one-shot
/// RSA signature verification.
#[derive(Clone)]
pub(crate) struct Inner {
n: PublicModulus,
e: PublicExponent,
}

impl Inner {
pub(super) fn from_modulus_and_exponent(
n: untrusted::Input,
e: untrusted::Input,
n_min_bits: bits::BitLength,
n_max_bits: bits::BitLength,
e_min_value: PublicExponent,
cpu_features: cpu::Features,
) -> Result<Self, error::KeyRejected> {
// This is an incomplete implementation of NIST SP800-56Br1 Section
// 6.4.2.2, "Partial Public-Key Validation for RSA." That spec defers
// to NIST SP800-89 Section 5.3.3, "(Explicit) Partial Public Key
Expand All @@ -64,27 +118,7 @@ impl PublicKey {
// XXX: Steps 4 & 5 / Steps d, e, & f are not implemented. This is also the
// case in most other commonly-used crypto libraries.

// TODO: Remove this re-parsing, and stop allocating this here.
// Instead we should serialize on demand without allocation, from
// `Modulus::be_bytes()` and `Exponent::be_bytes()`.
let n_bytes = io::Positive::from_be_bytes(n_bytes)
.map_err(|_: error::Unspecified| error::KeyRejected::unexpected_error())?;
let e_bytes = io::Positive::from_be_bytes(e_bytes)
.map_err(|_: error::Unspecified| error::KeyRejected::unexpected_error())?;
let serialized = der_writer::write_all(der::Tag::Sequence, &|output| {
der_writer::write_positive_integer(output, &n_bytes);
der_writer::write_positive_integer(output, &e_bytes);
});

Ok(Self { n, e, serialized })
}

/// The length, in bytes, of the public modulus.
///
/// The modulus length is rounded up to a whole number of bytes if its
/// bit length isn't a multiple of 8.
pub fn modulus_len(&self) -> usize {
self.n().len_bits().as_usize_bytes_rounded_up()
Ok(Self { n, e })
}

/// The public modulus.
Expand Down
4 changes: 2 additions & 2 deletions src/rsa/public_key_components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ where
{
fn from(public_key: &PublicKey) -> Self {
Self {
n: public_key.n().be_bytes().collect(),
e: public_key.e().be_bytes().collect(),
n: public_key.inner().n().be_bytes().collect(),
e: public_key.inner().e().be_bytes().collect(),
}
}
}
4 changes: 2 additions & 2 deletions src/rsa/verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
//! Verification of RSA signatures.
use super::{
parse_public_key, PublicExponent, PublicKey, RsaParameters, PUBLIC_KEY_PUBLIC_MODULUS_MAX_LEN,
parse_public_key, public_key, PublicExponent, RsaParameters, PUBLIC_KEY_PUBLIC_MODULUS_MAX_LEN,
};
use crate::{bits, cpu, digest, error, sealed, signature};

Expand Down Expand Up @@ -201,7 +201,7 @@ pub(crate) fn verify_rsa_(
// exponent value is 2**16 + 1, but it isn't clear if this is just for
// signing or also for verification. We support exponents of 3 and larger
// for compatibility with other commonly-used crypto libraries.
let key = PublicKey::from_modulus_and_exponent(
let key = public_key::Inner::from_modulus_and_exponent(
n,
e,
params.min_bits,
Expand Down

0 comments on commit 8ed4860

Please sign in to comment.