fix(native-certgen): address Phase 3 validation findings

Remove ENCRYPT/VERIFY from KeyUsage mapping to match Kotlin behavior.
Document BasicConstraints and SKI suppression via rcgen NoCa default.
Fix rotating log off-by-one that kept one extra backup file. Handle
BMPString (UTF-16BE) and VisibleString in X.500 DN parser.
This commit is contained in:
Enginex0
2026-03-09 16:08:07 +01:00
parent e76f5115e0
commit 76b18706f1
2 changed files with 24 additions and 19 deletions
+18 -12
View File
@@ -87,8 +87,9 @@ fn build_leaf_params(
timestamp_to_datetime(params.cert_not_after)? timestamp_to_datetime(params.cert_not_after)?
}; };
// Suppress AKI/SKI — Android attestation leaf certs don't include these // rcgen 0.13.2: IsCa::NoCa (the default) emits neither BasicConstraints nor SKI
cp.key_identifier_method = rcgen::KeyIdMethod::PreSpecified(vec![]); // extension. This matches real Android attestation leaf certs which include neither.
// No explicit suppression needed — NoCa is a no-op in the extension writer.
// KeyUsage from purposes // KeyUsage from purposes
cp.key_usages = map_key_usages(&params.purposes); cp.key_usages = map_key_usages(&params.purposes);
@@ -101,22 +102,19 @@ fn build_leaf_params(
Ok(cp) Ok(cp)
} }
/// Maps KeyPurpose values to X.509 KeyUsage bits per KeyCreationResult.aidl spec.
/// Only SIGN, DECRYPT, WRAP_KEY, AGREE_KEY, and ATTEST_KEY produce KeyUsage bits.
/// ENCRYPT and VERIFY are intentionally excluded (matches Kotlin CertificateGenerator).
fn map_key_usages(purposes: &[i32]) -> Vec<KeyUsagePurpose> { fn map_key_usages(purposes: &[i32]) -> Vec<KeyUsagePurpose> {
let mut usages = Vec::new(); let mut usages = Vec::new();
for &purpose in purposes { for &purpose in purposes {
match purpose { match purpose {
2 | 3 => { 2 => {
// SIGN or VERIFY -> digitalSignature // SIGN -> digitalSignature
if !usages.contains(&KeyUsagePurpose::DigitalSignature) { if !usages.contains(&KeyUsagePurpose::DigitalSignature) {
usages.push(KeyUsagePurpose::DigitalSignature); usages.push(KeyUsagePurpose::DigitalSignature);
} }
} }
0 => {
// ENCRYPT -> keyEncipherment
if !usages.contains(&KeyUsagePurpose::KeyEncipherment) {
usages.push(KeyUsagePurpose::KeyEncipherment);
}
}
1 => { 1 => {
// DECRYPT -> dataEncipherment // DECRYPT -> dataEncipherment
if !usages.contains(&KeyUsagePurpose::DataEncipherment) { if !usages.contains(&KeyUsagePurpose::DataEncipherment) {
@@ -224,10 +222,18 @@ fn extract_string_from_der_any(der: &[u8]) -> String {
let content = &der[header_len..end]; let content = &der[header_len..end];
match tag { match tag {
0x0C | 0x13 | 0x16 => { 0x0C | 0x13 | 0x16 | 0x1A => {
// UTF8String (0x0C), PrintableString (0x13), IA5String (0x16) // UTF8String (0x0C), PrintableString (0x13), IA5String (0x16), VisibleString (0x1A)
String::from_utf8_lossy(content).into_owned() String::from_utf8_lossy(content).into_owned()
} }
0x1E => {
// BMPString (UTF-16BE)
let chars: Vec<u16> = content
.chunks_exact(2)
.map(|c| u16::from_be_bytes([c[0], c[1]]))
.collect();
String::from_utf16_lossy(&chars)
}
_ => String::from_utf8_lossy(content).into_owned(), _ => String::from_utf8_lossy(content).into_owned(),
} }
} }
+6 -7
View File
@@ -53,7 +53,12 @@ fn rotate(state: &mut RotatingState) {
state.current.take(); state.current.take();
let dir = &state.dir; let dir = &state.dir;
// Shift older files up: .{max-1} is deleted, .{N} -> .{N+1} // Delete the oldest rotated file before shifting
let oldest = dir.join(format!("certgen.log.{}", state.max_files));
if oldest.exists() {
let _ = fs::remove_file(&oldest);
}
// Shift older files up: .{N} -> .{N+1}
for i in (1..state.max_files).rev() { for i in (1..state.max_files).rev() {
let from = dir.join(format!("certgen.log.{}", i)); let from = dir.join(format!("certgen.log.{}", i));
let to = dir.join(format!("certgen.log.{}", i + 1)); let to = dir.join(format!("certgen.log.{}", i + 1));
@@ -68,12 +73,6 @@ fn rotate(state: &mut RotatingState) {
let _ = fs::rename(&current_path, &first_rotated); let _ = fs::rename(&current_path, &first_rotated);
} }
// Delete excess files beyond max_files
let excess = dir.join(format!("certgen.log.{}", state.max_files + 1));
if excess.exists() {
let _ = fs::remove_file(&excess);
}
let (file, size) = open_current_log(dir); let (file, size) = open_current_log(dir);
state.current = file; state.current = file;
state.current_size = size; state.current_size = size;