Skip to content

Commit 665a9ff

Browse files
committed
rsa: Validate public key befrore allocating memory.
1 parent 8a7249c commit 665a9ff

File tree

5 files changed

+49
-40
lines changed

5 files changed

+49
-40
lines changed

src/arithmetic/bigint/modulusvalue.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,11 @@ impl<'a> ValidatedInput<'a> {
108108
self.len_bits
109109
}
110110

111-
pub(crate) fn build_value<M>(self) -> OwnedModulusValue<M> {
111+
pub fn input(&self) -> untrusted::Input<'_> {
112+
self.input
113+
}
114+
115+
pub(crate) fn build_value<M>(&self) -> OwnedModulusValue<M> {
112116
let limbs = Uninit::new_less_safe(self.num_limbs)
113117
.write_from_be_byes_padded(self.input)
114118
.unwrap_or_else(|LenMismatchError { .. }| unreachable!());

src/rsa/keypair.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
use super::{
1616
padding::{self, RsaEncoding},
17-
KeyPairComponents, PublicExponent, PublicKey, PublicKeyComponents, N,
17+
public_key, KeyPairComponents, PublicExponent, PublicKey, PublicKeyComponents, N,
1818
};
1919

2020
/// RSA PKCS#1 1.5 signatures.
@@ -274,15 +274,16 @@ impl KeyPair {
274274
// https://www.mail-archive.com/[email protected]/msg44759.html.
275275
// Also, this limit might help with memory management decisions later.
276276

277-
// Step 1.c. We validate e >= 65537.
278-
let public_key = PublicKey::new(
277+
let public_key = public_key::ValidatedInput::try_from_be_bytes(
279278
public_key,
280279
BitLength::from_bits(2048),
281280
super::PRIVATE_KEY_PUBLIC_MODULUS_MAX_BITS,
282281
PublicExponent::_65537,
283-
cpu_features,
284282
)?;
285283

284+
// Step 1.c. We validate e >= 65537.
285+
let public_key = PublicKey::new(public_key, cpu_features)?;
286+
286287
let n_one = public_key.inner().n().oneRR();
287288
let n = &public_key.inner().n().value(cpu_features);
288289

src/rsa/public_key.rs

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -37,22 +37,13 @@ derive_debug_self_as_ref_hex_bytes!(PublicKey);
3737

3838
impl PublicKey {
3939
pub(super) fn new(
40-
components: PublicKeyComponents<&[u8]>,
41-
n_min_bits: bits::BitLength,
42-
n_max_bits: bits::BitLength,
43-
e_min_value: PublicExponent,
40+
input: ValidatedInput<'_>,
4441
cpu_features: cpu::Features,
4542
) -> Result<Self, error::KeyRejected> {
46-
let inner = Inner::new(
47-
components,
48-
n_min_bits,
49-
n_max_bits,
50-
e_min_value,
51-
cpu_features,
52-
)?;
43+
let inner = input.build(cpu_features);
5344

54-
let n_bytes = untrusted::Input::from(components.n);
55-
let e_bytes = untrusted::Input::from(components.e);
45+
let n_bytes = input.n.input();
46+
let e_bytes = input.e_input;
5647

5748
// TODO: Remove this re-parsing, and stop allocating this here.
5849
// Instead we should serialize on demand without allocation, from
@@ -92,14 +83,27 @@ pub(crate) struct Inner {
9283
e: PublicExponent,
9384
}
9485

95-
impl Inner {
96-
pub(super) fn new(
97-
components: PublicKeyComponents<&[u8]>,
86+
pub struct ValidatedInput<'a> {
87+
n: public_modulus::ValidatedInput<'a>,
88+
e: PublicExponent,
89+
e_input: untrusted::Input<'a>,
90+
}
91+
92+
impl<'a> ValidatedInput<'a> {
93+
pub(super) fn try_from_be_bytes(
94+
components: PublicKeyComponents<&'a [u8]>,
9895
n_min_bits: bits::BitLength,
9996
n_max_bits: bits::BitLength,
10097
e_min_value: PublicExponent,
101-
cpu_features: cpu::Features,
10298
) -> Result<Self, error::KeyRejected> {
99+
let n =
100+
public_modulus::ValidatedInput::from_be_bytes(components.n, n_min_bits..=n_max_bits)?;
101+
let e_input = components.e.into();
102+
let e = PublicExponent::from_be_bytes(e_input, e_min_value)?;
103+
Ok(Self { n, e, e_input })
104+
}
105+
106+
pub(super) fn build(&self, cpu_features: cpu::Features) -> Inner {
103107
// This is an incomplete implementation of NIST SP800-56Br1 Section
104108
// 6.4.2.2, "Partial Public-Key Validation for RSA." That spec defers
105109
// to NIST SP800-89 Section 5.3.3, "(Explicit) Partial Public Key
@@ -109,13 +113,7 @@ impl Inner {
109113
// and one set lettered. TODO: Document this in the end-user
110114
// documentation for RSA keys.
111115

112-
let n = public_modulus::ValidatedInput::from_be_bytes(
113-
components.n.into(),
114-
n_min_bits..=n_max_bits,
115-
)?;
116-
let e = PublicExponent::from_be_bytes(components.e.into(), e_min_value)?;
117-
118-
let n = n.build(cpu_features);
116+
let n = self.n.build(cpu_features);
119117

120118
// If `n` is less than `e` then somebody has probably accidentally swapped
121119
// them. The largest acceptable `e` is smaller than the smallest acceptable
@@ -124,9 +122,11 @@ impl Inner {
124122
// XXX: Steps 4 & 5 / Steps d, e, & f are not implemented. This is also the
125123
// case in most other commonly-used crypto libraries.
126124

127-
Ok(Self { n, e })
125+
Inner { n, e: self.e }
128126
}
127+
}
129128

129+
impl Inner {
130130
/// The public modulus.
131131
#[inline]
132132
pub(super) fn n(&self) -> &PublicModulus {

src/rsa/public_modulus.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ pub struct ValidatedInput<'a> {
4141

4242
impl<'a> ValidatedInput<'a> {
4343
pub fn from_be_bytes(
44-
n: untrusted::Input<'a>,
44+
n: &'a [u8],
4545
allowed_bit_lengths: RangeInclusive<bits::BitLength>,
4646
) -> Result<Self, error::KeyRejected> {
4747
// See `PublicKey::from_modulus_and_exponent` for background on the step
@@ -56,7 +56,7 @@ impl<'a> ValidatedInput<'a> {
5656
const MIN_BITS: bits::BitLength = bits::BitLength::from_bits(1024);
5757

5858
// Step 3 / Step c for `n` (out of order).
59-
let input = bigint::modulus::ValidatedInput::try_from_be_bytes(n)?;
59+
let input = bigint::modulus::ValidatedInput::try_from_be_bytes(n.into())?;
6060
let bits = input.len_bits();
6161

6262
// Step 1 / Step a. XXX: SP800-56Br1 and SP800-89 require the length of
@@ -75,7 +75,11 @@ impl<'a> ValidatedInput<'a> {
7575
Ok(Self { input })
7676
}
7777

78-
pub(super) fn build(self, cpu_features: cpu::Features) -> PublicModulus {
78+
pub fn input(&self) -> untrusted::Input<'_> {
79+
self.input.input()
80+
}
81+
82+
pub(super) fn build(&self, cpu_features: cpu::Features) -> PublicModulus {
7983
let value = self.input.build_value().into_modulus();
8084
let m = value.modulus(cpu_features);
8185
let oneRR = m.alloc_uninit();

src/rsa/verification.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -196,19 +196,19 @@ fn verify(
196196
let max_bits: bits::BitLength =
197197
bits::BitLength::from_byte_len(PUBLIC_KEY_PUBLIC_MODULUS_MAX_LEN)
198198
.map_err(error::erase::<InputTooLongError>)?;
199-
200-
// XXX: FIPS 186-4 seems to indicate that the minimum
201-
// exponent value is 2**16 + 1, but it isn't clear if this is just for
202-
// signing or also for verification. We support exponents of 3 and larger
203-
// for compatibility with other commonly-used crypto libraries.
204-
let key = public_key::Inner::new(
199+
let validated = public_key::ValidatedInput::try_from_be_bytes(
205200
components,
206201
params.min_bits,
207202
max_bits,
208203
PublicExponent::_3,
209-
cpu_features,
210204
)?;
211205

206+
// XXX: FIPS 186-4 seems to indicate that the minimum
207+
// exponent value is 2**16 + 1, but it isn't clear if this is just for
208+
// signing or also for verification. We support exponents of 3 and larger
209+
// for compatibility with other commonly-used crypto libraries.
210+
let key = validated.build(cpu_features);
211+
212212
// RFC 8017 Section 5.2.2: RSAVP1.
213213
let mut decoded = [0u8; PUBLIC_KEY_PUBLIC_MODULUS_MAX_LEN];
214214
let decoded = key.exponentiate(signature, &mut decoded, cpu_features)?;

0 commit comments

Comments
 (0)