Skip to content

Commit

Permalink
x509-cert: make SKI optional in leaf certificate (#1028)
Browse files Browse the repository at this point in the history
[RFC5280 Section 4.2.1.2] recommends the SKI to be included but other
specifications (IEEE 801.1AR Section 8.10.2 subjectKeyIdentifier) says
it should not be included.

This introduces a tunable in the Leaf profile under the `hazmat` feature
not to include it.

[RFC5280 Section 4.2.1.2]: https://www.rfc-editor.org/rfc/rfc5280#section-4.2.1.2
  • Loading branch information
baloo authored Apr 25, 2023
1 parent 3c1060e commit f005791
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 7 deletions.
17 changes: 16 additions & 1 deletion x509-cert/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,14 @@ pub enum Profile {
enable_key_agreement: bool,
/// should the key encipherment flag of KeyUsage be enabled
enable_key_encipherment: bool,
/// should the subject key identifier extension be included
///
/// From [RFC 5280 Section 4.2.1.2]:
/// For end entity certificates, subject key identifiers SHOULD be
/// derived from the public key. Two common methods for generating key
/// identifiers from the public key are identified above.
#[cfg(feature = "hazmat")]
include_subject_key_identifier: bool,
},
#[cfg(feature = "hazmat")]
/// Opt-out of the default extensions
Expand Down Expand Up @@ -128,7 +136,14 @@ impl Profile {

let mut extensions: vec::Vec<Extension> = vec::Vec::new();

extensions.push(SubjectKeyIdentifier::try_from(spk)?.to_extension(tbs)?);
match self {
#[cfg(feature = "hazmat")]
Profile::Leaf {
include_subject_key_identifier: false,
..
} => {}
_ => extensions.push(SubjectKeyIdentifier::try_from(spk)?.to_extension(tbs)?),
}

// Build Authority Key Identifier
match self {
Expand Down
46 changes: 40 additions & 6 deletions x509-cert/tests/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,27 +90,35 @@ fn leaf_certificate() {
let issuer =
Name::from_str("CN=World domination corporation,O=World domination Inc,C=US").unwrap();
let profile = Profile::Leaf {
issuer,
issuer: issuer.clone(),
enable_key_agreement: false,
enable_key_encipherment: false,
#[cfg(feature = "hazmat")]
include_subject_key_identifier: true,
};

let subject = Name::from_str("CN=service.domination.world").unwrap();
let pub_key =
SubjectPublicKeyInfoOwned::try_from(RSA_2048_DER_EXAMPLE).expect("get rsa pub key");

let signer = ecdsa_signer();
let builder =
CertificateBuilder::new(profile, serial_number, validity, subject, pub_key, &signer)
.expect("Create certificate");
let builder = CertificateBuilder::new(
profile,
serial_number.clone(),
validity.clone(),
subject.clone(),
pub_key.clone(),
&signer,
)
.expect("Create certificate");

let certificate = builder.build::<ecdsa::Signature<NistP256>>().unwrap();

let pem = certificate.to_pem(LineEnding::LF).expect("generate pem");
println!("{}", openssl::check_certificate(pem.as_bytes()));

// TODO(baloo): not too sure we should tackle those in this API.
let ignored = &[
let ignored = vec![
"e_sub_cert_aia_missing",
"e_sub_cert_crl_distribution_points_missing",
"w_sub_cert_aia_does_not_contain_issuing_ca_url",
Expand All @@ -126,7 +134,31 @@ fn leaf_certificate() {
"e_sub_cert_eku_missing",
];

zlint::check_certificate(pem.as_bytes(), ignored);
zlint::check_certificate(pem.as_bytes(), &ignored);

#[cfg(feature = "hazmat")]
{
let profile = Profile::Leaf {
issuer,
enable_key_agreement: false,
enable_key_encipherment: false,
include_subject_key_identifier: false,
};
let builder =
CertificateBuilder::new(profile, serial_number, validity, subject, pub_key, &signer)
.expect("Create certificate");

let certificate = builder.build::<ecdsa::Signature<NistP256>>().unwrap();

let pem = certificate.to_pem(LineEnding::LF).expect("generate pem");
println!("{}", openssl::check_certificate(pem.as_bytes()));

// Ignore warning about leaf not having SKI extension (this is a warning not a fail, as
// denoted by the `w_` prefix.
let mut ignored = ignored;
ignored.push("w_ext_subject_key_identifier_missing_sub_cert");
zlint::check_certificate(pem.as_bytes(), &ignored);
}
}

#[test]
Expand All @@ -140,6 +172,8 @@ fn pss_certificate() {
issuer,
enable_key_agreement: false,
enable_key_encipherment: false,
#[cfg(feature = "hazmat")]
include_subject_key_identifier: true,
};

let subject = Name::from_str("CN=service.domination.world").unwrap();
Expand Down

0 comments on commit f005791

Please sign in to comment.