Skip to content

Tighten up string type representations to prevent illegal values #214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions rcgen/examples/sign-leaf-with-ca.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ fn main() {
}

fn new_ca() -> Certificate {
let mut params = CertificateParams::new(Vec::default());
let mut params =
CertificateParams::new(Vec::default()).expect("empty subject alt name can't produce error");
let (yesterday, tomorrow) = validity_period();
params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained);
params
.distinguished_name
.push(DnType::CountryName, PrintableString("BR".into()));
params.distinguished_name.push(
DnType::CountryName,
PrintableString("BR".try_into().unwrap()),
);
params
.distinguished_name
.push(DnType::OrganizationName, "Crab widgits SE");
Expand All @@ -39,7 +41,7 @@ fn new_ca() -> Certificate {

fn new_end_entity() -> Certificate {
let name = "entity.other.host";
let mut params = CertificateParams::new(vec![name.into()]);
let mut params = CertificateParams::new(vec![name.into()]).expect("we know the name is valid");
let (yesterday, tomorrow) = validity_period();
params.distinguished_name.push(DnType::CommonName, name);
params.use_authority_key_identifier_extension = true;
Expand Down
4 changes: 2 additions & 2 deletions rcgen/examples/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
.distinguished_name
.push(DnType::CommonName, "Master Cert");
params.subject_alt_names = vec![
SanType::DnsName("crabs.crabs".to_string()),
SanType::DnsName("localhost".to_string()),
SanType::DnsName("crabs.crabs".try_into()?),
SanType::DnsName("localhost".try_into()?),
];

let key_pair = KeyPair::generate()?;
Expand Down
33 changes: 33 additions & 0 deletions rcgen/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ pub enum Error {
#[cfg(feature = "x509-parser")]
/// Invalid subject alternative name type
InvalidNameType,
/// Invalid ASN.1 string
InvalidAsn1String(InvalidAsn1String),
/// An IP address was provided as a byte array, but the byte array was an invalid length.
InvalidIpAddressOctetLength(usize),
/// There is no support for generating
Expand Down Expand Up @@ -55,6 +57,7 @@ impl fmt::Display for Error {
CouldNotParseKeyPair => write!(f, "Could not parse key pair")?,
#[cfg(feature = "x509-parser")]
InvalidNameType => write!(f, "Invalid subject alternative name type")?,
InvalidAsn1String(e) => write!(f, "{}", e)?,
InvalidIpAddressOctetLength(actual) => {
write!(f, "Invalid IP address octet length of {actual} bytes")?
},
Expand Down Expand Up @@ -90,6 +93,36 @@ impl fmt::Display for Error {

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

/// Invalid ASN.1 string type
#[derive(Debug, PartialEq, Eq)]
#[non_exhaustive]
pub enum InvalidAsn1String {
/// Invalid PrintableString type
PrintableString(String),
/// Invalid UniversalString type
UniversalString(String),
/// Invalid Ia5String type
Ia5String(String),
/// Invalid TeletexString type
TeletexString(String),
/// Invalid BmpString type
BmpString(String),
}

impl fmt::Display for InvalidAsn1String {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use InvalidAsn1String::*;
match self {
PrintableString(s) => write!(f, "Invalid PrintableString: '{}'", s)?,
Ia5String(s) => write!(f, "Invalid IA5String: '{}'", s)?,
BmpString(s) => write!(f, "Invalid BMPString: '{}'", s)?,
UniversalString(s) => write!(f, "Invalid UniversalString: '{}'", s)?,
TeletexString(s) => write!(f, "Invalid TeletexString: '{}'", s)?,
};
Ok(())
}
}

/// 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
Expand Down
80 changes: 47 additions & 33 deletions rcgen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,13 @@ pub use crate::crl::{
CrlIssuingDistributionPoint, CrlScope, RevocationReason, RevokedCertParams,
};
pub use crate::csr::{CertificateSigningRequestParams, PublicKey};
pub use crate::error::Error;
pub use crate::error::{Error, InvalidAsn1String};
use crate::key_pair::PublicKeyData;
pub use crate::key_pair::{KeyPair, RemoteKeyPair};
use crate::oid::*;
pub use crate::sign_algo::algo::*;
pub use crate::sign_algo::SignatureAlgorithm;
pub use crate::string_types::*;

/// Type-alias for the old name of [`Error`].
#[deprecated(
Expand Down Expand Up @@ -118,7 +119,7 @@ pub fn generate_simple_self_signed(
) -> Result<CertifiedKey, Error> {
let key_pair = KeyPair::generate()?;
let cert =
Certificate::generate_self_signed(CertificateParams::new(subject_alt_names), &key_pair)?;
Certificate::generate_self_signed(CertificateParams::new(subject_alt_names)?, &key_pair)?;
Ok(CertifiedKey { cert, key_pair })
}

Expand All @@ -131,6 +132,7 @@ mod key_pair;
mod oid;
mod ring_like;
mod sign_algo;
mod string_types;

// Example certs usable as reference:
// Uses ECDSA: https://crt.sh/?asn1=607203242
Expand All @@ -150,9 +152,9 @@ const ENCODE_CONFIG: pem::EncodeConfig = {
/// The type of subject alt name
pub enum SanType {
/// Also known as E-Mail address
Rfc822Name(String),
DnsName(String),
URI(String),
Rfc822Name(Ia5String),
DnsName(Ia5String),
URI(Ia5String),
IpAddress(IpAddr),
}

Expand All @@ -172,10 +174,12 @@ impl SanType {
fn try_from_general(name: &x509_parser::extensions::GeneralName<'_>) -> Result<Self, Error> {
Ok(match name {
x509_parser::extensions::GeneralName::RFC822Name(name) => {
SanType::Rfc822Name((*name).into())
SanType::Rfc822Name((*name).try_into()?)
},
x509_parser::extensions::GeneralName::DNSName(name) => {
SanType::DnsName((*name).try_into()?)
},
x509_parser::extensions::GeneralName::DNSName(name) => SanType::DnsName((*name).into()),
x509_parser::extensions::GeneralName::URI(name) => SanType::URI((*name).into()),
x509_parser::extensions::GeneralName::URI(name) => SanType::URI((*name).try_into()?),
x509_parser::extensions::GeneralName::IPAddress(octets) => {
SanType::IpAddress(ip_addr_from_octets(octets)?)
},
Expand Down Expand Up @@ -376,15 +380,15 @@ impl DnType {
#[non_exhaustive]
pub enum DnValue {
/// A string encoded using UCS-2
BmpString(Vec<u8>),
BmpString(BmpString),
/// An ASCII string.
Ia5String(String),
Ia5String(Ia5String),
/// An ASCII string containing only A-Z, a-z, 0-9, '()+,-./:=? and `<SPACE>`
PrintableString(String),
PrintableString(PrintableString),
/// A string of characters from the T.61 character set
TeletexString(Vec<u8>),
TeletexString(TeletexString),
/// A string encoded using UTF-32
UniversalString(Vec<u8>),
UniversalString(UniversalString),
/// A string encoded using UTF-8
Utf8String(String),
}
Expand Down Expand Up @@ -444,9 +448,9 @@ impl DistinguishedName {
/// # use rcgen::{DistinguishedName, DnType, DnValue};
/// let mut dn = DistinguishedName::new();
/// dn.push(DnType::OrganizationName, "Crab widgits SE");
/// dn.push(DnType::CommonName, DnValue::PrintableString("Master Cert".to_string()));
/// dn.push(DnType::CommonName, DnValue::PrintableString("Master Cert".try_into().unwrap()));
/// assert_eq!(dn.get(&DnType::OrganizationName), Some(&DnValue::Utf8String("Crab widgits SE".to_string())));
/// assert_eq!(dn.get(&DnType::CommonName), Some(&DnValue::PrintableString("Master Cert".to_string())));
/// assert_eq!(dn.get(&DnType::CommonName), Some(&DnValue::PrintableString("Master Cert".try_into().unwrap())));
/// ```
pub fn push(&mut self, ty: DnType, s: impl Into<DnValue>) {
if !self.entries.contains_key(&ty) {
Expand Down Expand Up @@ -490,11 +494,13 @@ impl DistinguishedName {
let try_str =
|data| std::str::from_utf8(data).map_err(|_| Error::CouldNotParseCertificate);
let dn_value = match attr.attr_value().header.tag() {
Tag::BmpString => DnValue::BmpString(data.into()),
Tag::Ia5String => DnValue::Ia5String(try_str(data)?.to_owned()),
Tag::PrintableString => DnValue::PrintableString(try_str(data)?.to_owned()),
Tag::T61String => DnValue::TeletexString(data.into()),
Tag::UniversalString => DnValue::UniversalString(data.into()),
Tag::BmpString => DnValue::BmpString(BmpString::from_utf16be(data.to_vec())?),
Tag::Ia5String => DnValue::Ia5String(try_str(data)?.try_into()?),
Tag::PrintableString => DnValue::PrintableString(try_str(data)?.try_into()?),
Tag::T61String => DnValue::TeletexString(try_str(data)?.try_into()?),
Tag::UniversalString => {
DnValue::UniversalString(UniversalString::from_utf32be(data.to_vec())?)
},
Tag::Utf8String => DnValue::Utf8String(try_str(data)?.to_owned()),
_ => return Err(Error::CouldNotParseCertificate),
};
Expand Down Expand Up @@ -578,19 +584,21 @@ impl Default for CertificateParams {

impl CertificateParams {
/// Generate certificate parameters with reasonable defaults
pub fn new(subject_alt_names: impl Into<Vec<String>>) -> Self {
pub fn new(subject_alt_names: impl Into<Vec<String>>) -> Result<Self, Error> {
let subject_alt_names = subject_alt_names
.into()
.into_iter()
.map(|s| match s.parse() {
Ok(ip) => SanType::IpAddress(ip),
Err(_) => SanType::DnsName(s),
.map(|s| {
Ok(match IpAddr::from_str(&s) {
Ok(ip) => SanType::IpAddress(ip),
Err(_) => SanType::DnsName(s.try_into()?),
})
})
.collect::<Vec<_>>();
CertificateParams {
.collect::<Result<Vec<_>, _>>()?;
Ok(CertificateParams {
subject_alt_names,
..Default::default()
}
})
}

/// Parses an existing ca certificate from the ASCII PEM format.
Expand Down Expand Up @@ -850,7 +858,7 @@ impl CertificateParams {
|writer| match san {
SanType::Rfc822Name(name)
| SanType::DnsName(name)
| SanType::URI(name) => writer.write_ia5_string(name),
| SanType::URI(name) => writer.write_ia5_string(name.as_str()),
SanType::IpAddress(IpAddr::V4(addr)) => {
writer.write_bytes(&addr.octets())
},
Expand Down Expand Up @@ -1450,18 +1458,24 @@ fn write_distinguished_name(writer: DERWriter, dn: &DistinguishedName) {
match content {
DnValue::BmpString(s) => writer
.next()
.write_tagged_implicit(TAG_BMPSTRING, |writer| writer.write_bytes(s)),
DnValue::Ia5String(s) => writer.next().write_ia5_string(s),
DnValue::PrintableString(s) => writer.next().write_printable_string(s),
.write_tagged_implicit(TAG_BMPSTRING, |writer| {
writer.write_bytes(s.as_bytes())
}),

DnValue::Ia5String(s) => writer.next().write_ia5_string(s.as_str()),

DnValue::PrintableString(s) => {
writer.next().write_printable_string(s.as_str())
},
DnValue::TeletexString(s) => writer
.next()
.write_tagged_implicit(TAG_TELETEXSTRING, |writer| {
writer.write_bytes(s)
writer.write_bytes(s.as_bytes())
}),
DnValue::UniversalString(s) => writer
.next()
.write_tagged_implicit(TAG_UNIVERSALSTRING, |writer| {
writer.write_bytes(s)
writer.write_bytes(s.as_bytes())
}),
DnValue::Utf8String(s) => writer.next().write_utf8_string(s),
}
Expand Down
Loading