Skip to content

Commit

Permalink
Add external types CI check + config (#183)
Browse files Browse the repository at this point in the history
Having types from dependent crates appear in rcgen's public API makes
taking semver incompatible updates to those dependencies tricky: we need
to treat it as a semver incompatible change to rcgen itself. To make
sure whenever this type leak happens that it was done because of a
deliberate choice this branch adds
[cargo-check-external-types](https://github.com/awslabs/cargo-check-external-types)
to CI.

Three existing types that leaked into the API are fixed:
`ring::error::KeyRejected`, `ring::error::Unspecified`, and
`pem::PemError`. In each case it's simple to translate the specific
error types from the dependency into our own `Error` variants that don't
expose the type.

Two types are allow-listed: `time::offset_date_time::OffsetDateTime` and
`zeroize::Zeroize`.

The first, `OffsetDateTime` is used throughout the public API. It isn't
clear to me if we want to keep that as part of the public API and treat
`time` updates carefully, or if we should pursue using a more generic
representation. I've left this out for now so it can be discussed.

The second, `Zeroize`, could probably be avoided by implementing `Drop`
and calling `zeroize` on the sensitive fields manually. That's the
approach Rustls implemented
(rustls/rustls#1492). I've left this out for in
case there was a reason to prefer implementing the trait on the public
types.
  • Loading branch information
cpu authored Dec 9, 2023
1 parent 1a579ca commit 8798e81
Show file tree
Hide file tree
Showing 6 changed files with 138 additions and 86 deletions.
30 changes: 30 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,36 @@ jobs:
env:
RUSTDOCFLAGS: ${{ matrix.rust_channel == 'nightly' && '-Dwarnings --cfg=docsrs' || '-Dwarnings' }}

check-external-types:
name: Validate external types appearing in public API
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v4
with:
persist-credentials: false
- name: Install rust toolchain
uses: dtolnay/rust-toolchain@master
with:
toolchain: nightly-2023-10-10
# ^ sync with https://github.com/awslabs/cargo-check-external-types/blob/main/rust-toolchain.toml
- run: cargo install --locked cargo-check-external-types
- name: run cargo-check-external-types for rcgen/
working-directory: rcgen/
run: cargo check-external-types --all-features

semver:
name: Check semver compatibility
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v4
with:
persist-credentials: false

- name: Check semver
uses: obi1kenobi/cargo-semver-checks-action@v2

build-windows:
runs-on: windows-latest
env:
Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion rcgen/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "rcgen"
version = "0.11.3"
version = "0.12.0"
documentation = "https://docs.rs/rcgen"
description.workspace = true
repository.workspace = true
Expand Down Expand Up @@ -35,6 +35,12 @@ default = ["pem"]
[package.metadata.docs.rs]
features = ["x509-parser"]

[package.metadata.cargo_check_external_types]
allowed_external_types = [
"time::offset_date_time::OffsetDateTime",
"zeroize::Zeroize"
]

[dev-dependencies]
openssl = "0.10"
x509-parser = { version = "0.15", features = ["verify"] }
Expand Down
25 changes: 7 additions & 18 deletions rcgen/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub enum Error {
Time,
#[cfg(feature = "pem")]
/// Error from the pem crate
PemError(pem::PemError),
PemError(String),
/// Error generated by a remote key operation
RemoteKeyError,
/// Unsupported field when generating a CSR
Expand Down Expand Up @@ -98,21 +98,10 @@ impl fmt::Display for Error {

impl std::error::Error for Error {}

impl From<ring::error::Unspecified> for Error {
fn from(_unspecified: ring::error::Unspecified) -> Self {
Error::RingUnspecified
}
}

impl From<ring::error::KeyRejected> for Error {
fn from(err: ring::error::KeyRejected) -> Self {
Error::RingKeyRejected(err.to_string())
}
}

#[cfg(feature = "pem")]
impl From<pem::PemError> for Error {
fn from(e: pem::PemError) -> Self {
Error::PemError(e)
}
/// A trait describing an error that can be converted into an `rcgen::Error`.
///
/// We use this trait to avoid leaking external error types into the public API
/// through a `From<x> for Error` implementation.
pub(crate) trait ExternalError<T>: Sized {
fn _err(self) -> Result<T, Error>;
}
155 changes: 91 additions & 64 deletions rcgen/src/key_pair.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::convert::TryFrom;
use std::fmt;
use yasna::DERWriter;

use crate::error::ExternalError;
use crate::sign_algo::algo::*;
use crate::sign_algo::SignAlgo;
#[cfg(feature = "pem")]
Expand Down Expand Up @@ -58,14 +59,16 @@ impl KeyPair {
pub fn from_der(der: &[u8]) -> Result<Self, Error> {
Ok(der.try_into()?)
}

/// Returns the key pair's signature algorithm
pub fn algorithm(&self) -> &'static SignatureAlgorithm {
self.alg
}

/// Parses the key pair from the ASCII PEM format
#[cfg(feature = "pem")]
pub fn from_pem(pem_str: &str) -> Result<Self, Error> {
let private_key = pem::parse(pem_str)?;
let private_key = pem::parse(pem_str)._err()?;
let private_key_der: &[_] = private_key.contents();
Ok(private_key_der.try_into()?)
}
Expand All @@ -88,7 +91,7 @@ impl KeyPair {
pem_str: &str,
alg: &'static SignatureAlgorithm,
) -> Result<Self, Error> {
let private_key = pem::parse(pem_str)?;
let private_key = pem::parse(pem_str)._err()?;
let private_key_der: &[_] = private_key.contents();
Ok(Self::from_der_and_sign_algo(private_key_der, alg)?)
}
Expand All @@ -110,30 +113,28 @@ impl KeyPair {
let pkcs8_vec = pkcs8.to_vec();

let kind = if alg == &PKCS_ED25519 {
KeyPairKind::Ed(Ed25519KeyPair::from_pkcs8_maybe_unchecked(pkcs8)?)
KeyPairKind::Ed(Ed25519KeyPair::from_pkcs8_maybe_unchecked(pkcs8)._err()?)
} else if alg == &PKCS_ECDSA_P256_SHA256 {
KeyPairKind::Ec(EcdsaKeyPair::from_pkcs8(
&signature::ECDSA_P256_SHA256_ASN1_SIGNING,
pkcs8,
rng,
)?)
KeyPairKind::Ec(
EcdsaKeyPair::from_pkcs8(&signature::ECDSA_P256_SHA256_ASN1_SIGNING, pkcs8, rng)
._err()?,
)
} else if alg == &PKCS_ECDSA_P384_SHA384 {
KeyPairKind::Ec(EcdsaKeyPair::from_pkcs8(
&signature::ECDSA_P384_SHA384_ASN1_SIGNING,
pkcs8,
rng,
)?)
KeyPairKind::Ec(
EcdsaKeyPair::from_pkcs8(&signature::ECDSA_P384_SHA384_ASN1_SIGNING, pkcs8, rng)
._err()?,
)
} else if alg == &PKCS_RSA_SHA256 {
let rsakp = RsaKeyPair::from_pkcs8(pkcs8)?;
let rsakp = RsaKeyPair::from_pkcs8(pkcs8)._err()?;
KeyPairKind::Rsa(rsakp, &signature::RSA_PKCS1_SHA256)
} else if alg == &PKCS_RSA_SHA384 {
let rsakp = RsaKeyPair::from_pkcs8(pkcs8)?;
let rsakp = RsaKeyPair::from_pkcs8(pkcs8)._err()?;
KeyPairKind::Rsa(rsakp, &signature::RSA_PKCS1_SHA384)
} else if alg == &PKCS_RSA_SHA512 {
let rsakp = RsaKeyPair::from_pkcs8(pkcs8)?;
let rsakp = RsaKeyPair::from_pkcs8(pkcs8)._err()?;
KeyPairKind::Rsa(rsakp, &signature::RSA_PKCS1_SHA512)
} else if alg == &PKCS_RSA_PSS_SHA256 {
let rsakp = RsaKeyPair::from_pkcs8(pkcs8)?;
let rsakp = RsaKeyPair::from_pkcs8(pkcs8)._err()?;
KeyPairKind::Rsa(rsakp, &signature::RSA_PSS_SHA256)
} else {
panic!("Unknown SignatureAlgorithm specified!");
Expand Down Expand Up @@ -170,57 +171,14 @@ impl KeyPair {
};
Ok((kind, alg))
}
}

/// A private key that is not directly accessible, but can be used to sign messages
///
/// Trait objects based on this trait can be passed to the [`KeyPair::from_remote`] function for generating certificates
/// from a remote and raw private key, for example an HSM.
pub trait RemoteKeyPair {
/// Returns the public key of this key pair in the binary format as in [`KeyPair::public_key_raw`]
fn public_key(&self) -> &[u8];

/// Signs `msg` using the selected algorithm
fn sign(&self, msg: &[u8]) -> Result<Vec<u8>, Error>;

/// Reveals the algorithm to be used when calling `sign()`
fn algorithm(&self) -> &'static SignatureAlgorithm;
}

impl TryFrom<&[u8]> for KeyPair {
type Error = Error;

fn try_from(pkcs8: &[u8]) -> Result<KeyPair, Error> {
let (kind, alg) = KeyPair::from_raw(pkcs8)?;
Ok(KeyPair {
kind,
alg,
serialized_der: pkcs8.to_vec(),
})
}
}

impl TryFrom<Vec<u8>> for KeyPair {
type Error = Error;

fn try_from(pkcs8: Vec<u8>) -> Result<KeyPair, Error> {
let (kind, alg) = KeyPair::from_raw(pkcs8.as_slice())?;
Ok(KeyPair {
kind,
alg,
serialized_der: pkcs8,
})
}
}

impl KeyPair {
/// Generate a new random key pair for the specified signature algorithm
pub fn generate(alg: &'static SignatureAlgorithm) -> Result<Self, Error> {
let rng = &SystemRandom::new();

match alg.sign_alg {
SignAlgo::EcDsa(sign_alg) => {
let key_pair_doc = EcdsaKeyPair::generate_pkcs8(sign_alg, rng)?;
let key_pair_doc = EcdsaKeyPair::generate_pkcs8(sign_alg, rng)._err()?;
let key_pair_serialized = key_pair_doc.as_ref().to_vec();

let key_pair =
Expand All @@ -232,7 +190,7 @@ impl KeyPair {
})
},
SignAlgo::EdDsa(_sign_alg) => {
let key_pair_doc = Ed25519KeyPair::generate_pkcs8(rng)?;
let key_pair_doc = Ed25519KeyPair::generate_pkcs8(rng)._err()?;
let key_pair_serialized = key_pair_doc.as_ref().to_vec();

let key_pair = Ed25519KeyPair::from_pkcs8(&&key_pair_doc.as_ref()).unwrap();
Expand All @@ -248,6 +206,7 @@ impl KeyPair {
SignAlgo::Rsa() => Err(Error::KeyGenerationUnavailable),
}
}

/// Get the raw public key of this key pair
///
/// The key is in raw format, as how [`ring::signature::KeyPair::public_key`]
Expand All @@ -256,20 +215,23 @@ impl KeyPair {
pub fn public_key_raw(&self) -> &[u8] {
self.raw_bytes()
}

/// Check if this key pair can be used with the given signature algorithm
pub fn is_compatible(&self, signature_algorithm: &SignatureAlgorithm) -> bool {
self.alg == signature_algorithm
}

/// Returns (possibly multiple) compatible [`SignatureAlgorithm`]'s
/// that the key can be used with
pub fn compatible_algs(&self) -> impl Iterator<Item = &'static SignatureAlgorithm> {
std::iter::once(self.alg)
}

pub(crate) fn sign(&self, msg: &[u8], writer: DERWriter) -> Result<(), Error> {
match &self.kind {
KeyPairKind::Ec(kp) => {
let system_random = SystemRandom::new();
let signature = kp.sign(&system_random, msg)?;
let signature = kp.sign(&system_random, msg)._err()?;
let sig = &signature.as_ref();
writer.write_bitvec_bytes(&sig, &sig.len() * 8);
},
Expand All @@ -281,7 +243,8 @@ impl KeyPair {
KeyPairKind::Rsa(kp, padding_alg) => {
let system_random = SystemRandom::new();
let mut signature = vec![0; kp.public().modulus_len()];
kp.sign(*padding_alg, &system_random, msg, &mut signature)?;
kp.sign(*padding_alg, &system_random, msg, &mut signature)
._err()?;
let sig = &signature.as_ref();
writer.write_bitvec_bytes(&sig, &sig.len() * 8);
},
Expand All @@ -292,6 +255,7 @@ impl KeyPair {
}
Ok(())
}

/// Return the key pair's public key in DER format
///
/// The key is formatted according to the SubjectPublicKeyInfo struct of
Expand All @@ -300,6 +264,7 @@ impl KeyPair {
pub fn public_key_der(&self) -> Vec<u8> {
yasna::construct_der(|writer| self.serialize_public_key_der(writer))
}

/// Return the key pair's public key in PEM format
///
/// The returned string can be interpreted with `openssl pkey --inform PEM -pubout -pubin -text`
Expand All @@ -309,6 +274,7 @@ impl KeyPair {
let p = Pem::new("PUBLIC KEY", contents);
pem::encode_config(&p, ENCODE_CONFIG)
}

/// Serializes the key pair (including the private key) in PKCS#8 format in DER
///
/// Panics if called on a remote key pair.
Expand Down Expand Up @@ -350,6 +316,32 @@ impl KeyPair {
}
}

impl TryFrom<&[u8]> for KeyPair {
type Error = Error;

fn try_from(pkcs8: &[u8]) -> Result<KeyPair, Error> {
let (kind, alg) = KeyPair::from_raw(pkcs8)?;
Ok(KeyPair {
kind,
alg,
serialized_der: pkcs8.to_vec(),
})
}
}

impl TryFrom<Vec<u8>> for KeyPair {
type Error = Error;

fn try_from(pkcs8: Vec<u8>) -> Result<KeyPair, Error> {
let (kind, alg) = KeyPair::from_raw(pkcs8.as_slice())?;
Ok(KeyPair {
kind,
alg,
serialized_der: pkcs8,
})
}
}

impl PublicKeyData for KeyPair {
fn alg(&self) -> &SignatureAlgorithm {
self.alg
Expand All @@ -364,9 +356,44 @@ impl PublicKeyData for KeyPair {
}
}

/// A private key that is not directly accessible, but can be used to sign messages
///
/// Trait objects based on this trait can be passed to the [`KeyPair::from_remote`] function for generating certificates
/// from a remote and raw private key, for example an HSM.
pub trait RemoteKeyPair {
/// Returns the public key of this key pair in the binary format as in [`KeyPair::public_key_raw`]
fn public_key(&self) -> &[u8];

/// Signs `msg` using the selected algorithm
fn sign(&self, msg: &[u8]) -> Result<Vec<u8>, Error>;

/// Reveals the algorithm to be used when calling `sign()`
fn algorithm(&self) -> &'static SignatureAlgorithm;
}

impl<T> ExternalError<T> for Result<T, ring::error::KeyRejected> {
fn _err(self) -> Result<T, Error> {
self.map_err(|e| Error::RingKeyRejected(e.to_string()))
}
}

impl<T> ExternalError<T> for Result<T, ring::error::Unspecified> {
fn _err(self) -> Result<T, Error> {
self.map_err(|_| Error::RingUnspecified)
}
}

impl<T> ExternalError<T> for Result<T, pem::PemError> {
fn _err(self) -> Result<T, Error> {
self.map_err(|e| Error::PemError(e.to_string()))
}
}

pub(crate) trait PublicKeyData {
fn alg(&self) -> &SignatureAlgorithm;

fn raw_bytes(&self) -> &[u8];

fn serialize_public_key_der(&self, writer: DERWriter) {
writer.write_sequence(|writer| {
self.alg().write_oids_sign_alg(writer.next());
Expand Down
Loading

0 comments on commit 8798e81

Please sign in to comment.