[rsa] Add PKCS1v1.5 signing

Change-Id: I09c5e9d29f498238b521ad54f3753a4a601c009b
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2dd3990..805a678 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,11 @@
 
 ## [Unreleased]
 
+### Added
+- `public::rsa` now supports RSA-PKCS1v1.5 signing (behind the `rsa-pkcs1v15`
+  feature flag).
+
+### Fixed
 - `build.rs` no longer respects `$GOPATH`, instead it always uses the
   `go.mod` from the vendored boringssl.
 
diff --git a/Cargo.toml b/Cargo.toml
index 2a67dec..8278cbc 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -32,6 +32,7 @@
 insecure = []
 kdf = []
 rand-bytes = []
+rsa-pkcs1v15 = []
 experimental-sha512-ec = []
 # Run the `boringssl/test_symbol_conflict.sh` test during `cargo test`. This is
 # disabled by default because the test takes a long time to run.
diff --git a/boringssl/bindgen.sh b/boringssl/bindgen.sh
index 23fb135..a8262b8 100755
--- a/boringssl/bindgen.sh
+++ b/boringssl/bindgen.sh
@@ -98,9 +98,11 @@
 RSA_marshal_private_key|\
 RSA_new|\
 RSA_parse_private_key|\
+RSA_sign|\
 RSA_sign_pss_mgf1|\
 RSA_size|\
 RSA_up_ref|\
+RSA_verify|\
 RSA_verify_pss_mgf1|\
 SHA1_Init|\
 SHA1_Update|\
diff --git a/boringssl/boringssl.rs b/boringssl/boringssl.rs
index 14844a1..2617bae 100644
--- a/boringssl/boringssl.rs
+++ b/boringssl/boringssl.rs
@@ -1014,6 +1014,17 @@
     ) -> ::std::os::raw::c_int;
 }
 extern "C" {
+    #[link_name = "__RUST_MUNDANE_0_3_0_RSA_sign"]
+    pub fn RSA_sign(
+        hash_nid: ::std::os::raw::c_int,
+        in_: *const u8,
+        in_len: ::std::os::raw::c_uint,
+        out: *mut u8,
+        out_len: *mut ::std::os::raw::c_uint,
+        rsa: *mut RSA,
+    ) -> ::std::os::raw::c_int;
+}
+extern "C" {
     #[link_name = "__RUST_MUNDANE_0_3_0_RSA_sign_pss_mgf1"]
     pub fn RSA_sign_pss_mgf1(
         rsa: *mut RSA,
@@ -1028,6 +1039,17 @@
     ) -> ::std::os::raw::c_int;
 }
 extern "C" {
+    #[link_name = "__RUST_MUNDANE_0_3_0_RSA_verify"]
+    pub fn RSA_verify(
+        hash_nid: ::std::os::raw::c_int,
+        msg: *const u8,
+        msg_len: usize,
+        sig: *const u8,
+        sig_len: usize,
+        rsa: *mut RSA,
+    ) -> ::std::os::raw::c_int;
+}
+extern "C" {
     #[link_name = "__RUST_MUNDANE_0_3_0_RSA_verify_pss_mgf1"]
     pub fn RSA_verify_pss_mgf1(
         rsa: *mut RSA,
diff --git a/src/boringssl/mod.rs b/src/boringssl/mod.rs
index 70b6d12..4ae02f6 100644
--- a/src/boringssl/mod.rs
+++ b/src/boringssl/mod.rs
@@ -112,6 +112,8 @@
     RAND_bytes, RSA_bits, RSA_generate_key_ex, RSA_marshal_private_key, RSA_parse_private_key,
     RSA_sign_pss_mgf1, RSA_size, RSA_verify_pss_mgf1, SHA384_Init,
 };
+#[cfg(feature = "rsa-pkcs1v15")]
+use boringssl::raw::{RSA_sign, RSA_verify};
 
 impl CStackWrapper<BIGNUM> {
     /// The `BN_set_u64` function.
@@ -612,6 +614,45 @@
     }
 }
 
+/// The `RSA_sign` function.
+///
+/// # Panics
+///
+/// `rsa_sign` panics if `sig` is shorter than the minimum required signature
+/// size given by `rsa_size`.
+#[cfg(feature = "rsa-pkcs1v15")]
+pub fn rsa_sign(
+    hash_nid: c_int,
+    digest: &[u8],
+    sig: &mut [u8],
+    key: &CHeapWrapper<RSA>,
+) -> Result<usize, BoringError> {
+    unsafe {
+        // If we call RSA_sign with sig.len() < min_size, it will invoke UB.
+        let min_size = key.rsa_size().unwrap_abort();
+        assert_abort!(sig.len() >= min_size.get());
+
+        let mut sig_len: c_uint = 0;
+        RSA_sign(
+            hash_nid,
+            digest.as_ptr(),
+            digest.len().try_into().unwrap_abort(),
+            sig.as_mut_ptr(),
+            &mut sig_len,
+            // RSA_sign does not mutate its argument but, for
+            // backwards-compatibility reasons, continues to take a normal
+            // (non-const) pointer.
+            key.as_const() as *mut _,
+        )?;
+
+        // RSA_sign guarantees that it only needs RSA_size bytes for the
+        // signature.
+        let sig_len = sig_len.try_into().unwrap_abort();
+        assert_abort!(sig_len <= min_size.get());
+        Ok(sig_len)
+    }
+}
+
 /// The `rsa_sign_pss_mgf1` function.
 #[must_use]
 pub fn rsa_sign_pss_mgf1(
@@ -647,6 +688,25 @@
     }
 }
 
+/// The `RSA_verify` function.
+#[must_use]
+#[cfg(feature = "rsa-pkcs1v15")]
+pub fn rsa_verify(hash_nid: c_int, digest: &[u8], sig: &[u8], key: &CHeapWrapper<RSA>) -> bool {
+    unsafe {
+        RSA_verify(
+            hash_nid,
+            digest.as_ptr(),
+            digest.len(),
+            sig.as_ptr(),
+            sig.len(),
+            // RSA_verify does not mutate its argument but, for
+            // backwards-compatibility reasons, continues to take a normal
+            // (non-const) pointer.
+            key.as_const() as *mut _,
+        )
+    }
+}
+
 /// The `RSA_verify_pss_mgf1` function.
 #[must_use]
 pub fn rsa_verify_pss_mgf1(
diff --git a/src/boringssl/raw.rs b/src/boringssl/raw.rs
index eb0ac02..9ac8fd7 100644
--- a/src/boringssl/raw.rs
+++ b/src/boringssl/raw.rs
@@ -392,6 +392,20 @@
     ptr_or_err("RSA_parse_private_key", ffi::RSA_parse_private_key(cbs))
 }
 
+#[cfg(feature = "rsa-pkcs1v15")]
+#[allow(non_snake_case)]
+#[must_use]
+pub unsafe fn RSA_sign(
+    hash_nid: c_int,
+    in_: *const u8,
+    in_len: c_uint,
+    out: *mut u8,
+    out_len: *mut c_uint,
+    key: *mut RSA,
+) -> Result<(), BoringError> {
+    one_or_err("RSA_sign", ffi::RSA_sign(hash_nid, in_, in_len, out, out_len, key))
+}
+
 #[allow(non_snake_case)]
 #[must_use]
 pub unsafe fn RSA_sign_pss_mgf1(
@@ -418,6 +432,25 @@
         .ok_or_else(|| BoringError::consume_stack("RSA_size"))
 }
 
+#[cfg(feature = "rsa-pkcs1v15")]
+#[allow(non_snake_case)]
+#[must_use]
+pub unsafe fn RSA_verify(
+    hash_nid: c_int,
+    msg: *const u8,
+    msg_len: usize,
+    sig: *const u8,
+    sig_len: usize,
+    rsa: *mut RSA,
+) -> bool {
+    match ffi::RSA_verify(hash_nid, msg, msg_len, sig, sig_len, rsa) {
+        0 => false,
+        1 => true,
+        // RSA_verify promises to only return 0 or 1
+        _ => unreachable_abort!(),
+    }
+}
+
 #[allow(non_snake_case)]
 #[must_use]
 pub unsafe fn RSA_verify_pss_mgf1(
diff --git a/src/insecure.rs b/src/insecure.rs
index ef255b3..e4384ef 100644
--- a/src/insecure.rs
+++ b/src/insecure.rs
@@ -7,8 +7,8 @@
 //! WARNING: INSECURE CRYPTOGRAPHIC OPERATIONS.
 //!
 //! This module contains cryptographic operations which are considered insecure.
-//! These operations should only be used for compatibility with legacy systems,
-//! but never in new systems!
+//! These operations should only be used for compatibility with legacy systems -
+//! never in new systems!
 
 #![deprecated(note = "insecure cryptographic operations")]
 
diff --git a/src/lib.rs b/src/lib.rs
index de94f73..62583f4 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -26,17 +26,18 @@
 //!
 //! **Features**
 //!
-//! | Name         | Description              |
-//! | ------------ | ------------------------ |
-//! | `kdf`        | Key derivation functions |
-//! | `rand-bytes` | Generate random bytes    |
+//! | Name           | Description              |
+//! | -------------- | ------------------------ |
+//! | `kdf`          | Key derivation functions |
+//! | `rand-bytes`   | Generate random bytes    |
+//! | `rsa-pkcs1v15` | RSA-PKCS1v1.5 signatures |
 //!
 //! # Insecure Operations
 //!
 //! Mundane supports one additional feature not listed in the previous section:
 //! `insecure`. This enables some cryptographic primitives which are today
 //! considered insecure. These should only be used for compatibility with legacy
-//! systems, but never in new systems! When the `insecure` feature is used, an
+//! systems - never in new systems! When the `insecure` feature is used, an
 //! `insecure` module is added to the crate root. All insecure primitives are
 //! exposed through this module.
 
diff --git a/src/public/rsa/mod.rs b/src/public/rsa/mod.rs
index 991db95..e32d0c2 100644
--- a/src/public/rsa/mod.rs
+++ b/src/public/rsa/mod.rs
@@ -359,15 +359,49 @@
 /// An `RsaSignatureScheme` defines how to compute an RSA signature. The primary
 /// detail defined by a signature scheme is how to perform padding.
 ///
-/// `RsaSignatureScheme` is implemented by [`RsaPss`].
+/// `RsaSignatureScheme` is implemented by [`RsaPss`] and, if the `rsa-pkcs1v15`
+/// feature is enabled, [`RsaPkcs1v15`].
 pub trait RsaSignatureScheme:
     Sized + Copy + Clone + Default + Display + Debug + self::inner::RsaSignatureScheme
 {
 }
 
-/// The RSA-PSS signature scheme.
+/// The RSA-PKCS1v1.5 signature scheme.
+///
+/// This signature scheme is old, and considered less secure than RSA-PSS. It
+/// should only be used for compatibility with legacy systems - never in new
+/// systems!
+#[cfg(feature = "rsa-pkcs1v15")]
 #[derive(Copy, Clone, Default, Debug, Eq, PartialEq, Hash)]
-pub struct RsaPss;
+pub struct RsaPkcs1v15;
+
+#[cfg(feature = "rsa-pkcs1v15")]
+impl Display for RsaPkcs1v15 {
+    fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> {
+        write!(f, "RSA-PKCS1v1.5")
+    }
+}
+
+#[cfg(feature = "rsa-pkcs1v15")]
+impl Sealed for RsaPkcs1v15 {}
+#[cfg(feature = "rsa-pkcs1v15")]
+impl RsaSignatureScheme for RsaPkcs1v15 {}
+
+#[cfg(feature = "rsa-pkcs1v15")]
+impl self::inner::RsaSignatureScheme for RsaPkcs1v15 {
+    fn sign<B: RsaKeyBits, H: Hasher>(
+        rsa: &RsaKey<B>,
+        digest: &[u8],
+        sig: &mut [u8],
+    ) -> Result<usize, BoringError> {
+        // NOTE: rsa_sign will panic if sig is not large enough to hold the
+        // largest possible signature, as RSA_sign has this as a precondition.
+        boringssl::rsa_sign(H::nid(), digest, sig, &rsa.key)
+    }
+    fn verify<B: RsaKeyBits, H: Hasher>(rsa: &RsaKey<B>, digest: &[u8], sig: &[u8]) -> bool {
+        boringssl::rsa_verify(H::nid(), digest, sig, &rsa.key)
+    }
+}
 
 impl Display for RsaPss {
     fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> {
@@ -375,6 +409,10 @@
     }
 }
 
+/// The RSA-PSS signature scheme.
+#[derive(Copy, Clone, Default, Debug, Eq, PartialEq, Hash)]
+pub struct RsaPss;
+
 impl Sealed for RsaPss {}
 impl RsaSignatureScheme for RsaPss {}
 
@@ -384,9 +422,12 @@
         digest: &[u8],
         sig: &mut [u8],
     ) -> Result<usize, BoringError> {
-        // This is not needed for memory safety, but is an important security
-        // and correctness check, as passing a too-short signature would result
-        // in the signature being truncated.
+        // We assert here (and not in the RSA-PKCS1v1.5 implementation) because,
+        // while our rsa_sign wrapper (which implements PKCS1v1.5) performs this
+        // assertion itself (for safety reasons), our rsa_sign_pss_mgf1 wrapper
+        // does not. This assertion is an important security and correctness
+        // check as well, as passing a too-short signature would result in the
+        // signature being truncated.
         assert!(sig.len() >= rsa.key.rsa_size().unwrap().get());
         // A salt_len of -1 means to use a salt of the same length as the hash
         // output. This is a reasonable default and, for bit lengths larger than
@@ -567,6 +608,13 @@
                 unwrap_pub_any(RsaPubKeyAnyBits::parse_from_der(&pubkey.marshal_to_der()).unwrap());
 
             fn sign_and_verify<B: RsaKeyBits>(privkey: &RsaPrivKey<B>, pubkey: &RsaPubKey<B>) {
+                #[cfg(feature = "rsa-pkcs1v15")]
+                {
+                    let sig =
+                        RsaSignature::<B, RsaPkcs1v15, Sha256>::sign(&privkey, MESSAGE).unwrap();
+                    assert!(RsaSignature::<B, RsaPkcs1v15, Sha256>::from_bytes(sig.bytes())
+                        .is_valid(&pubkey, MESSAGE));
+                }
                 let sig = RsaSignature::<B, RsaPss, Sha256>::sign(&privkey, MESSAGE).unwrap();
                 assert!(RsaSignature::<B, RsaPss, Sha256>::from_bytes(sig.bytes())
                     .is_valid(&pubkey, MESSAGE));
@@ -745,6 +793,12 @@
         fn test<B: RsaKeyBits>() {
             use public::testutil::test_signature_smoke;
             let key = generate_rsa_key::<B>();
+            #[cfg(feature = "rsa-pkcs1v15")]
+            test_signature_smoke(
+                &key,
+                RsaSignature::<_, RsaPkcs1v15, Sha256>::from_bytes,
+                RsaSignature::bytes,
+            );
             test_signature_smoke(
                 &key,
                 RsaSignature::<_, RsaPss, Sha256>::from_bytes,
@@ -766,6 +820,11 @@
             assert!(!sig.is_valid_format());
             assert!(!sig.is_valid(&generate_rsa_key::<B2048>().public(), &[],));
         }
+        #[cfg(feature = "rsa-pkcs1v15")]
+        {
+            test_is_invalid::<RsaPkcs1v15>(&RsaSignature::from_bytes(&[0; MAX_SIGNATURE_LEN + 1]));
+            test_is_invalid::<RsaPkcs1v15>(&RsaSignature::from_bytes(&[]));
+        }
         test_is_invalid::<RsaPss>(&RsaSignature::from_bytes(&[0; MAX_SIGNATURE_LEN + 1]));
         test_is_invalid::<RsaPss>(&RsaSignature::from_bytes(&[]));
     }
diff --git a/test.sh b/test.sh
index 3058f7b..c8831f6 100755
--- a/test.sh
+++ b/test.sh
@@ -8,7 +8,7 @@
 
 set -eu
 
-FEATURES="insecure kdf rand-bytes experimental-sha512-ec"
+FEATURES="insecure kdf rand-bytes rsa-pkcs1v15 experimental-sha512-ec"
 
 # Test with each feature individually
 for features in $FEATURES run-symbol-conflict-test; do