From a38d9a4ef98f028dc939413e5a803a61e1f72e7b Mon Sep 17 00:00:00 2001
From: Brian Smith <brian@briansmith.org>
Date: Mon, 21 Nov 2016 18:44:50 -1000
Subject: [PATCH] Verify RSA PKCS#1 1.5 signatures by encode-then-check.

When we first implemented PKCS#1 1.5 signature verification we had not
implemented signature generation, so we implemented verification by
parsing the padding. Now that we have generation we can save some code
and arguably make verification safer. Also, this is the way RFC 3447
recommends to do it.
---
 src/polyfill.rs    |  5 +++
 src/rsa/padding.rs | 97 +++++++++++++++++-----------------------------
 2 files changed, 41 insertions(+), 61 deletions(-)

diff --git a/src/polyfill.rs b/src/polyfill.rs
index 1f81d3c34..5ae33f470 100644
--- a/src/polyfill.rs
+++ b/src/polyfill.rs
@@ -17,6 +17,11 @@
 
 use core;
 
+// A better name for the `&*` idiom for removing the mutability from a
+// reference.
+#[inline(always)]
+pub fn ref_from_mut_ref<'a, T: ?Sized>(x: &'a mut T) -> &'a T { x }
+
 #[inline(always)]
 pub fn u64_from_usize(x: usize) -> u64 { x as u64 }
 
diff --git a/src/rsa/padding.rs b/src/rsa/padding.rs
index 4cde3aeed..63febb7f6 100644
--- a/src/rsa/padding.rs
+++ b/src/rsa/padding.rs
@@ -53,81 +53,56 @@ impl ::private::Private for PKCS1 { }
 
 #[cfg(feature ="rsa_signing")]
 impl RSAEncoding for PKCS1 {
-    // Implement padding procedure per EMSA-PKCS1-v1_5,
-    // https://tools.ietf.org/html/rfc3447#section-9.2.
     fn encode(&self, msg: &[u8], m_out: &mut [u8], _mod_bits: bits::BitLength,
               _rng: &rand::SecureRandom) -> Result<(), error::Unspecified> {
-        let em = m_out;
-
-        let digest_len = self.digestinfo_prefix.len() +
-                         self.digest_alg.output_len;
-
-        // Require at least 8 bytes of padding. Since we disallow keys smaller
-        // than 2048 bits, this should never happen anyway.
-        debug_assert!(em.len() >= digest_len + 11);
-        let pad_len = em.len() - digest_len - 3;
-        em[0] = 0;
-        em[1] = 1;
-        for i in 0..pad_len {
-            em[2 + i] = 0xff;
-        }
-        em[2 + pad_len] = 0;
-
-        let (digest_prefix, digest_dst) = em[3 + pad_len..]
-            .split_at_mut(self.digestinfo_prefix.len());
-        digest_prefix.copy_from_slice(self.digestinfo_prefix);
-        digest_dst.copy_from_slice(
-            digest::digest(self.digest_alg, msg).as_ref());
+        pkcs1_encode(&self, msg, m_out);
         Ok(())
     }
 }
 
 impl RSAVerification for PKCS1 {
     fn verify(&self, msg: untrusted::Input, m: &mut untrusted::Reader,
-              _mod_bits: bits::BitLength) -> Result<(), error::Unspecified> {
-        let em = m;
-
-        if try!(em.read_byte()) != 0 ||
-           try!(em.read_byte()) != 1 {
+              mod_bits: bits::BitLength) -> Result<(), error::Unspecified> {
+        // `mod_bits.as_usize_bytes_rounded_up() <=
+        //      PUBLIC_KEY_PUBLIC_MODULUS_MAX_LEN` is ensured by `verify_rsa()`.
+        let mut calculated = [0u8; PUBLIC_KEY_PUBLIC_MODULUS_MAX_LEN];
+        let calculated =
+            &mut calculated[..mod_bits.as_usize_bytes_rounded_up()];
+        pkcs1_encode(&self, msg.as_slice_less_safe(), calculated);
+        if m.skip_to_end() != polyfill::ref_from_mut_ref(calculated) {
             return Err(error::Unspecified);
         }
-
-        let mut ps_len = 0;
-        loop {
-            match try!(em.read_byte()) {
-                0xff => {
-                    ps_len += 1;
-                },
-                0x00 => {
-                    break;
-                },
-                _ => {
-                    return Err(error::Unspecified);
-                },
-            }
-        }
-        if ps_len < 8 {
-            return Err(error::Unspecified);
-        }
-
-        let em_digestinfo_prefix = try!(em.skip_and_get_input(
-                    self.digestinfo_prefix.len()));
-        if em_digestinfo_prefix != self.digestinfo_prefix {
-            return Err(error::Unspecified);
-        }
-
-        let digest_alg = self.digest_alg;
-        let decoded_digest =
-            try!(em.skip_and_get_input(digest_alg.output_len));
-        let digest = digest::digest(digest_alg, msg.as_slice_less_safe());
-        if decoded_digest != digest.as_ref() {
-            return Err(error::Unspecified);
-        }
-
         Ok(())
     }
 }
 
+// Implement padding procedure per EMSA-PKCS1-v1_5,
+// https://tools.ietf.org/html/rfc3447#section-9.2. This is used by both
+// verification and signing so it needs to be able to handle moduli of the
+// minimum and maximum sizes for both operations.
+fn pkcs1_encode(pkcs1: &PKCS1, msg: &[u8], m_out: &mut [u8]) {
+    let em = m_out;
+
+    let digest_len =
+        pkcs1.digestinfo_prefix.len() + pkcs1.digest_alg.output_len;
+
+    // The specification requires at least 8 bytes of padding. Since we
+    // disallow keys smaller than 2048 bits, this should always be true.
+    assert!(em.len() >= digest_len + 11);
+    let pad_len = em.len() - digest_len - 3;
+    em[0] = 0;
+    em[1] = 1;
+    for i in 0..pad_len {
+        em[2 + i] = 0xff;
+    }
+    em[2 + pad_len] = 0;
+
+    let (digest_prefix, digest_dst) = em[3 + pad_len..]
+        .split_at_mut(pkcs1.digestinfo_prefix.len());
+    digest_prefix.copy_from_slice(pkcs1.digestinfo_prefix);
+    digest_dst.copy_from_slice(digest::digest(pkcs1.digest_alg, msg).as_ref());
+}
+
 macro_rules! rsa_pkcs1_padding {
     ( $PADDING_ALGORITHM:ident, $digest_alg:expr, $digestinfo_prefix:expr,
       $doc_str:expr ) => {