Skip to content

Commit

Permalink
Remove TryFrom impls
Browse files Browse the repository at this point in the history
The `TryFrom` impl doesn't communicate that these bytes need to be DER-encoded. Forcing the user to use the constructor makes this obvious.
  • Loading branch information
thomaseizinger committed Oct 4, 2023
1 parent aaf4dd7 commit 345bc9b
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 43 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
## Unreleased

- Rename `RcGenError` to `Error` to avoid stuttering when used fully-qualified via `rcgen::`.
- Remove `TryFrom<[u8]>` and `TryFrom<Vec<u8>>` for `KeyPair` in favor of the more descriptive `KeyPair::from_der`.

## Release 0.11.3 - October 1, 2023

Expand Down
2 changes: 1 addition & 1 deletion examples/rsa-irc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
let bits = 2048;
let private_key = RsaPrivateKey::new(&mut rng, bits)?;
let private_key_der = private_key.to_pkcs8_der()?;
let key_pair = rcgen::KeyPair::try_from(private_key_der.as_bytes()).unwrap();
let key_pair = rcgen::KeyPair::from_der(private_key_der.as_bytes()).unwrap();
params.key_pair = Some(key_pair);

let cert = Certificate::from_params(params)?;
Expand Down
42 changes: 9 additions & 33 deletions src/key_pair.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use pem::Pem;
use ring::rand::SystemRandom;
use ring::signature::KeyPair as RingKeyPair;
use ring::signature::{self, EcdsaKeyPair, Ed25519KeyPair, RsaEncoding, RsaKeyPair};
use std::convert::TryFrom;
use std::fmt;
use yasna::DERWriter;

Expand Down Expand Up @@ -56,7 +55,7 @@ impl KeyPair {
///
/// Equivalent to using the [`TryFrom`] implementation.
pub fn from_der(der: &[u8]) -> Result<Self, Error> {
Ok(der.try_into()?)
Ok(KeyPair::from_raw(der)?)
}
/// Returns the key pair's signature algorithm
pub fn algorithm(&self) -> &'static SignatureAlgorithm {
Expand All @@ -67,7 +66,7 @@ impl KeyPair {
pub fn from_pem(pem_str: &str) -> Result<Self, Error> {
let private_key = pem::parse(pem_str)?;
let private_key_der: &[_] = private_key.contents();
Ok(private_key_der.try_into()?)
Ok(KeyPair::from_raw(private_key_der)?)
}

/// Obtains the key pair from a raw public key and a remote private key
Expand Down Expand Up @@ -143,9 +142,7 @@ impl KeyPair {
})
}

pub(crate) fn from_raw(
pkcs8: &[u8],
) -> Result<(KeyPairKind, &'static SignatureAlgorithm), Error> {
pub(crate) fn from_raw(pkcs8: &[u8]) -> Result<KeyPair, Error> {
let (kind, alg) = if let Ok(edkp) = Ed25519KeyPair::from_pkcs8_maybe_unchecked(pkcs8) {
(KeyPairKind::Ed(edkp), &PKCS_ED25519)
} else if let Ok(eckp) =
Expand All @@ -164,7 +161,12 @@ impl KeyPair {
} else {
return Err(Error::CouldNotParseKeyPair);
};
Ok((kind, alg))

Ok(KeyPair {
kind,
alg,
serialized_der: pkcs8.to_vec(),
})
}
}

Expand All @@ -183,32 +185,6 @@ pub trait RemoteKeyPair {
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> {
Expand Down
8 changes: 3 additions & 5 deletions tests/botan.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
#![cfg(feature = "x509-parser")]

use rcgen::DnValue;
use rcgen::{BasicConstraints, Certificate, CertificateParams, DnType, IsCa};
use rcgen::{
CertificateRevocationList, CertificateRevocationListParams, RevocationReason, RevokedCertParams,
};
use rcgen::{DnValue, KeyPair};
use rcgen::{KeyUsagePurpose, SerialNumber};
use time::{Duration, OffsetDateTime};

Expand Down Expand Up @@ -172,7 +172,6 @@ fn test_botan_separate_ca() {
#[cfg(feature = "x509-parser")]
#[test]
fn test_botan_imported_ca() {
use std::convert::TryInto;
let mut params = default_params();
params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained);
let ca_cert = Certificate::from_params(params).unwrap();
Expand All @@ -182,7 +181,7 @@ fn test_botan_imported_ca() {
ca_cert.serialize_private_key_der(),
);

let ca_key_pair = ca_key_der.as_slice().try_into().unwrap();
let ca_key_pair = KeyPair::from_der(ca_key_der.as_slice()).unwrap();
let imported_ca_cert_params =
CertificateParams::from_ca_cert_der(ca_cert_der.as_slice(), ca_key_pair).unwrap();
let imported_ca_cert = Certificate::from_params(imported_ca_cert_params).unwrap();
Expand All @@ -205,7 +204,6 @@ fn test_botan_imported_ca() {
#[cfg(feature = "x509-parser")]
#[test]
fn test_botan_imported_ca_with_printable_string() {
use std::convert::TryInto;
let mut params = default_params();
params.distinguished_name.push(
DnType::CountryName,
Expand All @@ -219,7 +217,7 @@ fn test_botan_imported_ca_with_printable_string() {
ca_cert.serialize_private_key_der(),
);

let ca_key_pair = ca_key_der.as_slice().try_into().unwrap();
let ca_key_pair = KeyPair::from_der(ca_key_der.as_slice()).unwrap();
let imported_ca_cert_params =
CertificateParams::from_ca_cert_der(ca_cert_der.as_slice(), ca_key_pair).unwrap();
let imported_ca_cert = Certificate::from_params(imported_ca_cert_params).unwrap();
Expand Down
6 changes: 2 additions & 4 deletions tests/webpki.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,6 @@ fn test_webpki_separate_ca_name_constraints() {
#[cfg(feature = "x509-parser")]
#[test]
fn test_webpki_imported_ca() {
use std::convert::TryInto;
let mut params = util::default_params();
params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained);
let ca_cert = Certificate::from_params(params).unwrap();
Expand All @@ -414,7 +413,7 @@ fn test_webpki_imported_ca() {
ca_cert.serialize_private_key_der(),
);

let ca_key_pair = ca_key_der.as_slice().try_into().unwrap();
let ca_key_pair = KeyPair::from_der(ca_key_der.as_slice()).unwrap();
let imported_ca_cert_params =
CertificateParams::from_ca_cert_der(ca_cert_der.as_slice(), ca_key_pair).unwrap();
let imported_ca_cert = Certificate::from_params(imported_ca_cert_params).unwrap();
Expand Down Expand Up @@ -443,7 +442,6 @@ fn test_webpki_imported_ca() {
#[cfg(feature = "x509-parser")]
#[test]
fn test_webpki_imported_ca_with_printable_string() {
use std::convert::TryInto;
let mut params = util::default_params();
params.distinguished_name.push(
DnType::CountryName,
Expand All @@ -457,7 +455,7 @@ fn test_webpki_imported_ca_with_printable_string() {
ca_cert.serialize_private_key_der(),
);

let ca_key_pair = ca_key_der.as_slice().try_into().unwrap();
let ca_key_pair = KeyPair::from_der(ca_key_der.as_slice()).unwrap();
let imported_ca_cert_params =
CertificateParams::from_ca_cert_der(ca_cert_der.as_slice(), ca_key_pair).unwrap();
let imported_ca_cert = Certificate::from_params(imported_ca_cert_params).unwrap();
Expand Down

0 comments on commit 345bc9b

Please sign in to comment.