Skip to content

Commit

Permalink
Initial feedback pass
Browse files Browse the repository at this point in the history
  • Loading branch information
skmcgrail committed Feb 20, 2024
1 parent a949fbd commit b33bf02
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 25 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,10 @@ jobs:

- name: Run coverage
working-directory: ./aws-lc-rs
run: cargo llvm-cov --workspace --features unstable,ring-sig-verify,ring-io --no-fail-fast --ignore-filename-regex "aws-lc-(fips-)?sys/.*" --lcov --output-path ${{ runner.temp }}/lcov.info
run: cargo llvm-cov --workspace --features unstable --no-fail-fast --ignore-filename-regex "aws-lc-(fips-)?sys/.*" --lcov --output-path ${{ runner.temp }}/lcov.info
- name: Run FIPS coverage
working-directory: ./aws-lc-rs
run: cargo llvm-cov --workspace --features unstable,ring-sig-verify,ring-io,fips --no-fail-fast --ignore-filename-regex "aws-lc-(fips-)?sys/.*" --lcov --output-path ${{ runner.temp }}/lcov-fips.info
run: cargo llvm-cov --workspace --features unstable --no-fail-fast --ignore-filename-regex "aws-lc-(fips-)?sys/.*" --lcov --output-path ${{ runner.temp }}/lcov-fips.info
- name: Upload coverage reports to Codecov
uses: codecov/codecov-action@v3
env:
Expand Down
6 changes: 3 additions & 3 deletions aws-lc-rs/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ include ../Makefile

UNAME_S := $(shell uname -s)

AWS_LC_RS_COVERAGE_FEATURES := unstable,ring-sig-verify,ring-io
AWS_LC_RS_COV_EXTRA_FEATURES := unstable

asan:
# TODO: This build target produces linker error on Mac.
Expand All @@ -23,10 +23,10 @@ asan-fips:
RUST_BACKTRACE=1 ASAN_OPTIONS=detect_leaks=1 RUSTFLAGS=-Zsanitizer=address RUSTDOCFLAGS=-Zsanitizer=address cargo +nightly test --lib --bins --tests --examples --target `rustc -vV | sed -n 's|host: ||p'` --no-default-features --features fips,asan

coverage:
cargo llvm-cov --features "${AWS_LC_RS_COVERAGE_FEATURES}" --no-fail-fast --fail-under-lines 95 --ignore-filename-regex "aws-lc(-fips|)-sys/*" --lcov --output-path lcov.info
cargo llvm-cov --features "${AWS_LC_RS_COV_EXTRA_FEATURES}" --no-fail-fast --fail-under-lines 95 --ignore-filename-regex "aws-lc(-fips|)-sys/*" --lcov --output-path lcov.info

coverage-fips:
cargo llvm-cov --features "${AWS_LC_RS_COVERAGE_FEATURES},fips" --no-fail-fast --fail-under-lines 95 --ignore-filename-regex "aws-lc(-fips|)-sys/*" --lcov --output-path lcov.info
cargo llvm-cov --features "${AWS_LC_RS_COV_EXTRA_FEATURES},fips" --no-fail-fast --fail-under-lines 95 --ignore-filename-regex "aws-lc(-fips|)-sys/*" --lcov --output-path lcov.info

test:
cargo test --all-targets --features unstable
Expand Down
2 changes: 1 addition & 1 deletion aws-lc-rs/src/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ create_pointer!(EVP_AEAD_CTX, EVP_AEAD_CTX_free);

#[cfg(test)]
mod tests {
use crate::ptr::{ConstPointer, DetachablePointer, ManagedPointer};
use crate::ptr::{DetachablePointer, ManagedPointer};
use aws_lc::BIGNUM;

#[test]
Expand Down
48 changes: 48 additions & 0 deletions aws-lc-rs/src/rsa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,54 @@
// SPDX-License-Identifier: Apache-2.0 OR ISC

//! RSA Signature and Encryption Support.
//!
//! # OAEP Encryption / Decryption
//!
//! ```rust
//! # use std::error::Error;
//! # fn main() -> Result<(), Box<dyn Error>> {
//! use aws_lc_rs::{
//! encoding::{AsDer, Pkcs8V1Der, PublicKeyX509Der},
//! rsa::{KeySize, OAEP_SHA256_MGF1SHA256, PublicEncryptingKey, PrivateDecryptingKey}
//! };
//!
//! // Generate a RSA 2048-bit key.
//! let private_key = PrivateDecryptingKey::generate(KeySize::Rsa2048)?;
//!
//! // Serialize the RSA private key to DER encoded PKCS#8 format for later usage.
//! let private_key_der = AsDer::<Pkcs8V1Der>::as_der(&private_key)?;
//! let private_key_der_bytes = private_key_der.as_ref();
//!
//! // Load a RSA private key from DER encoded PKCS#8 document.
//! let private_key = PrivateDecryptingKey::from_pkcs8(private_key_der_bytes)?;
//!
//! // Retrieve the RSA public key
//! let public_key = private_key.public_key()?;
//!
//! // Serialize the RSA public key to DER encoded X.509 SubjectPublicKeyInfo for later usage.
//! let public_key_der = AsDer::<PublicKeyX509Der>::as_der(&public_key)?;
//! let public_key_der_bytes = public_key_der.as_ref();
//!
//! // Load a RSA public key from DER encoded X.509 SubjectPublicKeyInfo.
//! let public_key = PublicEncryptingKey::from_der(public_key_der_bytes)?;
//!
//! let message = b"hello world";
//! let mut ciphertext = vec![0u8; KeySize::Rsa2048.len()]; // Output will be the size of the RSA key length in bytes rounded up.
//!
//! // Encrypt a message with the public key.
//! let ciphertext = public_key.encrypt(&OAEP_SHA256_MGF1SHA256, message, &mut ciphertext)?;
//!
//! assert_ne!(message, ciphertext);
//!
//! // Decrypt a message with the private key.
//! let mut plaintext = vec![0u8; KeySize::Rsa2048.len()]; // Plaintext output will be at most RSA Key length bytes - 2 * HashLength − 2 bytes
//! let plaintext = private_key.decrypt(&OAEP_SHA256_MGF1SHA256, ciphertext, &mut plaintext)?;
//!
//! assert_eq!(message, plaintext);
//!
//! # Ok(())
//! # }
//! ```

// *R* and *r* in Montgomery math refer to different things, so we always use
// `R` to refer to *R* to avoid confusion, even when that's against the normal
Expand Down
36 changes: 21 additions & 15 deletions aws-lc-rs/src/rsa/encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: Apache-2.0 OR ISC

/// PKCS#8 Encoding Functions
pub(in super::super) mod pkcs8 {
pub(in crate::rsa) mod pkcs8 {
use crate::{
cbb::LcCBB,
error::{KeyRejected, Unspecified},
Expand All @@ -14,7 +14,7 @@ pub(in super::super) mod pkcs8 {
// rounded to an even 64-bit words (4678 + 1% + padding ≈ 4728).
const PKCS8_FIXED_CAPACITY_BUFFER: usize = 4728;

pub(in super::super) fn encode_v1_der(key: &LcPtr<EVP_PKEY>) -> Result<Vec<u8>, Unspecified> {
pub(in crate::rsa) fn encode_v1_der(key: &LcPtr<EVP_PKEY>) -> Result<Vec<u8>, Unspecified> {
let mut buffer = vec![0u8; PKCS8_FIXED_CAPACITY_BUFFER];
let out_len = {
let mut cbb = LcCBB::new_fixed(<&mut [u8; PKCS8_FIXED_CAPACITY_BUFFER]>::try_from(
Expand All @@ -33,18 +33,18 @@ pub(in super::super) mod pkcs8 {
}

// Supports v1 and v2 encodings through a single API entry-point.
pub(in super::super) fn decode_der(pkcs8: &[u8]) -> Result<LcPtr<EVP_PKEY>, KeyRejected> {
pub(in crate::rsa) fn decode_der(pkcs8: &[u8]) -> Result<LcPtr<EVP_PKEY>, KeyRejected> {
LcPtr::try_from(pkcs8)
}
}

/// [RFC 8017](https://www.rfc-editor.org/rfc/rfc8017.html)
///
/// PKCS #1: RSA Cryptography Specifications Version 2.2
pub(in super::super) mod rfc8017 {
pub(in crate::rsa) mod rfc8017 {
use crate::{
cbs,
error::Unspecified,
error::{KeyRejected, Unspecified},
ptr::{DetachableLcPtr, LcPtr},
};
use aws_lc::{
Expand All @@ -54,7 +54,7 @@ pub(in super::super) mod rfc8017 {
use std::ptr::null_mut;

/// DER encode a RSA public key to `RSAPublicKey` structure.
pub(in super::super) unsafe fn encode_public_key_der(
pub(in crate::rsa) unsafe fn encode_public_key_der(
pubkey: &LcPtr<EVP_PKEY>,
) -> Result<Box<[u8]>, ()> {
let mut pubkey_bytes = null_mut::<u8>();
Expand All @@ -74,17 +74,17 @@ pub(in super::super) mod rfc8017 {

/// Decode a DER encoded `RSAPublicKey` structure.
#[inline]
pub(in super::super) fn decode_public_key_der(
pub(in crate::rsa) fn decode_public_key_der(
public_key: &[u8],
) -> Result<LcPtr<EVP_PKEY>, Unspecified> {
) -> Result<LcPtr<EVP_PKEY>, KeyRejected> {
let mut cbs = unsafe { cbs::build_CBS(public_key) };

let rsa = DetachableLcPtr::new(unsafe { RSA_parse_public_key(&mut cbs) })?;

let pkey = LcPtr::new(unsafe { EVP_PKEY_new() })?;

if 1 != unsafe { EVP_PKEY_assign_RSA(*pkey, *rsa) } {
return Err(Unspecified);
return Err(KeyRejected::unspecified());
}

rsa.detach();
Expand All @@ -94,7 +94,7 @@ pub(in super::super) mod rfc8017 {

/// Decodes a DER encoded `RSAPrivateKey` structure.
#[inline]
pub(in super::super) fn decode_private_key_der(
pub(in crate::rsa) fn decode_private_key_der(
private_key: &[u8],
) -> Result<LcPtr<EVP_PKEY>, Unspecified> {
let mut cbs = unsafe { cbs::build_CBS(private_key) };
Expand All @@ -116,11 +116,17 @@ pub(in super::super) mod rfc8017 {
/// [RFC 5280](https://www.rfc-editor.org/rfc/rfc5280.html)
///
/// Encodings that use the `SubjectPublicKeyInfo` structure.
pub(in super::super) mod rfc5280 {
use crate::{cbb::LcCBB, cbs, encoding::PublicKeyX509Der, error::Unspecified, ptr::LcPtr};
pub(in crate::rsa) mod rfc5280 {
use crate::{
cbb::LcCBB,
cbs,
encoding::PublicKeyX509Der,
error::{KeyRejected, Unspecified},
ptr::LcPtr,
};
use aws_lc::{EVP_marshal_public_key, EVP_parse_public_key, EVP_PKEY};

pub(in super::super) fn encode_public_key_der(
pub(in crate::rsa) fn encode_public_key_der(
key: &LcPtr<EVP_PKEY>,
) -> Result<PublicKeyX509Der<'static>, Unspecified> {
let mut der = LcCBB::new(1024);
Expand All @@ -132,9 +138,9 @@ pub(in super::super) mod rfc5280 {
Ok(PublicKeyX509Der::from(der.into_buffer()?))
}

pub(in super::super) fn decode_public_key_der(
pub(in crate::rsa) fn decode_public_key_der(
value: &[u8],
) -> Result<LcPtr<EVP_PKEY>, Unspecified> {
) -> Result<LcPtr<EVP_PKEY>, KeyRejected> {
let mut der = unsafe { cbs::build_CBS(value) };
Ok(LcPtr::new(unsafe { EVP_parse_public_key(&mut der) })?)
}
Expand Down
4 changes: 1 addition & 3 deletions aws-lc-rs/src/rsa/encryption.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ use crate::{
fips::indicator_check,
ptr::LcPtr,
};
#[cfg(feature = "fips")]
use aws_lc::RSA_check_fips;
use aws_lc::{
EVP_PKEY_CTX_new, EVP_PKEY_CTX_set_rsa_mgf1_md, EVP_PKEY_CTX_set_rsa_oaep_md,
EVP_PKEY_CTX_set_rsa_padding, EVP_PKEY_decrypt, EVP_PKEY_decrypt_init, EVP_PKEY_encrypt,
Expand Down Expand Up @@ -254,7 +252,7 @@ impl PublicEncryptingKey {
///
/// # Errors
/// * `Unspecified` for any error that occurs deserializing from bytes.
pub fn from_der(value: &[u8]) -> Result<PublicEncryptingKey, Unspecified> {
pub fn from_der(value: &[u8]) -> Result<PublicEncryptingKey, KeyRejected> {
Ok(Self(RsaEvpPkey::from_rfc5280_public_key_der(
value,
UsageContext::Encryption,
Expand Down
1 change: 0 additions & 1 deletion aws-lc-rs/src/rsa/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ impl KeyPair {
///
/// # *ring* Compatibility
/// Our implementation ignores the `SecureRandom` parameter.
///
// # FIPS
// The following conditions must be met:
// * RSA Key Sizes: 2048, 3072, 4096
Expand Down

0 comments on commit b33bf02

Please sign in to comment.