Tighten up string type representations to prevent illegal values (#214)

Fix #181

Mainly inspired by the
[der](https://docs.rs/der/0.7.8/der/asn1/index.html) and the
[asn1_rs](https://github.com/rusticata/asn1-rs/tree/d05f860d07998b4e7f152005ec9a923bb284a708)
crates.

Author: Tudyx

rcgen/examples/sign-leaf-with-ca.rs

@@ -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");
@@ -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;

rcgen/examples/simple.rs

@@ -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()?;

rcgen/src/error.rs

@@ -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
@@ -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")?
 			},
@@ -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

rcgen/src/lib.rs

@@ -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(
@@ -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 })
 }
 
@@ -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
@@ -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),
 }
 
@@ -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)?)
 			},
@@ -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),
 }
@@ -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) {
@@ -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),
 			};
@@ -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.
@@ -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())
 							},
@@ -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),
 					}