From 83edfc355b0dfc21cb4bf74cde92db346f189c18 Mon Sep 17 00:00:00 2001 From: LucaCappelletti94 Date: Thu, 8 Oct 2026 17:18:21 +0200 Subject: [PATCH] Issue certificate serials as positive minimal integers (R74) --- crates/connetto-ca/src/lib.rs | 17 +++-- crates/connetto-client/src/enrolment/tests.rs | 30 +++++--- crates/connetto-client/tests/it/enrolment.rs | 17 ++++- .../src/device_cert/authority.rs | 9 +-- crates/connetto-core/src/device_cert/mod.rs | 2 + .../connetto-core/src/device_cert/serial.rs | 46 ++++++++++++ crates/connetto-core/src/device_cert/tests.rs | 71 ++++++++++++++++--- .../src/device_cert/attestation.rs | 10 ++- .../src/device_cert/enrolment.rs | 39 +++++++--- crates/connetto-server/src/device_cert/mod.rs | 2 +- .../connetto-server/src/device_cert/tests.rs | 43 ++++++++++- .../tests/it/device_identity.rs | 4 +- crates/connetto-server/tests/it/enrolment.rs | 62 ++++++++++++++-- crates/connetto-server/tests/it/revocation.rs | 4 +- plans/master-implementation-plan.md | 2 +- 15 files changed, 303 insertions(+), 55 deletions(-) create mode 100644 crates/connetto-core/src/device_cert/serial.rs diff --git a/crates/connetto-ca/src/lib.rs b/crates/connetto-ca/src/lib.rs index 1f15fb77..89b1405e 100644 --- a/crates/connetto-ca/src/lib.rs +++ b/crates/connetto-ca/src/lib.rs @@ -1,5 +1,6 @@ #![doc = include_str!("../README.md")] +use std::convert::Infallible; use std::fs::OpenOptions; use std::io::Write as _; use std::path::{Path, PathBuf}; @@ -9,8 +10,8 @@ use connetto_core::device_cert::layout::{ ISSUER_CERTIFICATE, ISSUER_KEY, ROOT_CERTIFICATE, ROOT_LIST, }; use connetto_core::device_cert::{ - DeploymentId, IssuerError, ListError, RevocationList, Revoked, RootCa, RootError, - certificate_serial, verify_signer, + CertificateSerial, DeploymentId, IssuerError, ListError, RevocationList, Revoked, RootCa, + RootError, certificate_serial, verify_signer, }; use pkcs8::{EncryptedPrivateKeyInfo, PrivateKeyInfo}; use rand_core::{OsRng, RngCore as _}; @@ -99,6 +100,11 @@ pub fn init(dir: &Path, passphrase: &str, now: SystemTime) -> Result Result<(), Infallible> { + OsRng.fill_bytes(bytes); + Ok(()) + }) + .expect("the system source draws a serial"); let certificate = root.sign_issuer( &issuer_key.subject_public_key_info(), now, diff --git a/crates/connetto-client/src/enrolment/tests.rs b/crates/connetto-client/src/enrolment/tests.rs index c3532821..d2e98b96 100644 --- a/crates/connetto-client/src/enrolment/tests.rs +++ b/crates/connetto-client/src/enrolment/tests.rs @@ -7,9 +7,9 @@ use std::time::{Instant, SystemTime}; use base64::Engine as _; use connetto_core::device_cert::{ - AttestationLevel, CertificateRequest, CertificateSigner, DeploymentId, DeviceCertificate, - DeviceIssuer, DeviceKey, DeviceKeyError, KeyHome, RevocationList, Revoked, RootCa, - public_key_info, + AttestationLevel, CertificateRequest, CertificateSerial, CertificateSigner, DeploymentId, + DeviceCertificate, DeviceIssuer, DeviceKey, DeviceKeyError, KeyHome, RevocationList, Revoked, + RootCa, public_key_info, }; use connetto_core::messages::{ControlMessage, DeviceAttestation, EnrolRefusal, SyncStatus}; use diesel::Connection as _; @@ -84,7 +84,7 @@ async fn held(not_before: SystemTime, lifetime: Duration) -> Held { &public_key_info(&issuer_key.key), not_before - 10 * HOUR, 400 * 24 * HOUR, - [2; 16], + CertificateSerial::new([2; 16]).expect("the serial is positive"), ) .expect("issuer"); let issuer = DeviceIssuer::from_pkcs8(issuer_der.clone(), &issuer_pkcs8, root.certificate()) @@ -98,7 +98,7 @@ async fn held(not_before: SystemTime, lifetime: Duration) -> Held { "alice", not_before, lifetime, - [3; 16], + CertificateSerial::new([3; 16]).expect("the serial is positive"), AttestationLevel::Unproven, ) .expect("issue"); @@ -220,7 +220,7 @@ async fn a_lower_number_never_replaces_a_higher_and_an_equal_one_with_other_cont &public_key_info(&issuer_key.key), start - HOUR, 400 * 24 * HOUR, - [4; 16], + CertificateSerial::new([4; 16]).expect("the serial is positive"), ) .expect("issuer"); let issuer = @@ -450,9 +450,11 @@ impl Link for FakeLink { _ => { let request = CertificateRequest::parse(&request.csr).expect("the csr parses"); - let mut serial = [0u8; 16]; - serial[..8].copy_from_slice(&state.serials.to_be_bytes()); + let mut bytes = [0x21_u8; 16]; + bytes[8..].copy_from_slice(&state.serials.to_be_bytes()); state.serials += 1; + let serial = + CertificateSerial::new(bytes).expect("the serial is positive"); let leaf = issuer .issue( &request, @@ -532,7 +534,7 @@ async fn authority(not_before: SystemTime) -> Authority { &public_key_info(&issuer_key.key), not_before - 10 * HOUR, 400 * 24 * HOUR, - [2; 16], + CertificateSerial::new([2; 16]).expect("the serial is positive"), ) .expect("the issuer signs"); let issuer = DeviceIssuer::from_pkcs8(certificate.clone(), &issuer_pkcs8, root.certificate()) @@ -551,7 +553,7 @@ fn issue_for( key: &dyn DeviceKey, not_before: SystemTime, lifetime: Duration, - serial: [u8; 16], + serial: CertificateSerial, ) -> Held { let csr = CertificateRequest::build(&CertificateSigner::new(key), &[1; 32]).expect("the csr builds"); @@ -678,7 +680,13 @@ async fn a_renewal_sends_no_attestation() { let (key, der) = attesting_key(); let now = SystemTime::now(); let authority = authority(now).await; - let held = issue_for(&authority, &key, now - 6 * HOUR, 12 * HOUR, [3; 16]); + let held = issue_for( + &authority, + &key, + now - 6 * HOUR, + 12 * HOUR, + CertificateSerial::new([3; 16]).expect("the serial is positive"), + ); let old_serial = held.leaf.serial().to_vec(); let state = Arc::new(Mutex::new(FakeState::default())); let (events, _events) = broadcast::channel(64); diff --git a/crates/connetto-client/tests/it/enrolment.rs b/crates/connetto-client/tests/it/enrolment.rs index 3ef1db0f..aec310b4 100644 --- a/crates/connetto-client/tests/it/enrolment.rs +++ b/crates/connetto-client/tests/it/enrolment.rs @@ -21,7 +21,8 @@ use connetto_client::{ REPLICA_PREFIX, SyncStatus, device_key_record, replica_db_name, }; use connetto_core::device_cert::{ - AttestationLevel, DeploymentId, DeviceCertificate, DeviceIssuer, KeyHome, RootCa, + AttestationLevel, CertificateSerial, DeploymentId, DeviceCertificate, DeviceIssuer, KeyHome, + RootCa, }; use connetto_core::messages::EnrolRefusal; use connetto_core::traits::{HandshakeAuthority, RefreshTokenStore}; @@ -181,11 +182,21 @@ static ROTATION: std::sync::LazyLock = std::sync::LazyLock::new(|| { .expect("root"); let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("issuer key"); let cert = root - .sign_issuer(&key.public_key_der(), now - DAY, 395 * DAY, [1; 16]) + .sign_issuer( + &key.public_key_der(), + now - DAY, + 395 * DAY, + CertificateSerial::new([1; 16]).expect("the serial is positive"), + ) .expect("issuer"); let next_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("next issuer key"); let next_cert = root - .sign_issuer(&next_key.public_key_der(), now - DAY, 395 * DAY, [2; 16]) + .sign_issuer( + &next_key.public_key_der(), + now - DAY, + 395 * DAY, + CertificateSerial::new([2; 16]).expect("the serial is positive"), + ) .expect("next issuer"); let root_list = root .sign_list( diff --git a/crates/connetto-core/src/device_cert/authority.rs b/crates/connetto-core/src/device_cert/authority.rs index 6eeb2575..c03bc5c2 100644 --- a/crates/connetto-core/src/device_cert/authority.rs +++ b/crates/connetto-core/src/device_cert/authority.rs @@ -12,6 +12,7 @@ use x509_parser::prelude::FromDer; use super::attestation::{ATTESTATION_EXTENSION, AttestationLevel}; use super::identity::{DeploymentId, DeviceIdentity, KeyId}; use super::request::CertificateRequest; +use super::serial::CertificateSerial; /// The deployment's root, kept offline and used only by `connetto-ca`. pub struct RootCa { @@ -108,7 +109,7 @@ impl RootCa { issuer_key: &[u8], not_before: SystemTime, valid_for: Duration, - serial: [u8; 16], + serial: CertificateSerial, ) -> Result, RootError> { let public = SubjectPublicKeyInfo::from_der(issuer_key)?; let signer = Issuer::from_ca_cert_der(&self.certificate.as_slice().into(), &self.key)?; @@ -121,7 +122,7 @@ impl RootCa { params.key_usages = vec![KeyUsagePurpose::KeyCertSign, KeyUsagePurpose::CrlSign]; params.not_before = to_time(not_before).ok_or(RootError::Validity)?; params.not_after = to_time(not_before + valid_for).ok_or(RootError::Validity)?; - params.serial_number = Some(SerialNumber::from_slice(&serial)); + params.serial_number = Some(SerialNumber::from_slice(&serial.as_bytes())); Ok(params.signed_by(&public, &signer)?.der().to_vec()) } } @@ -288,7 +289,7 @@ impl DeviceIssuer { account: &str, not_before: SystemTime, lifetime: Duration, - serial: [u8; 16], + serial: CertificateSerial, attestation: AttestationLevel, ) -> Result, IssueError> { let identity = DeviceIdentity::new( @@ -314,7 +315,7 @@ impl DeviceIssuer { params.subject_alt_names = vec![SanType::URI(uri)]; params.not_before = to_time(not_before).ok_or(IssueError::Validity)?; params.not_after = to_time(not_after).ok_or(IssueError::Validity)?; - params.serial_number = Some(SerialNumber::from_slice(&serial)); + params.serial_number = Some(SerialNumber::from_slice(&serial.as_bytes())); params.custom_extensions = vec![CustomExtension::from_oid_content( ATTESTATION_EXTENSION, attestation.extension_value(), diff --git a/crates/connetto-core/src/device_cert/mod.rs b/crates/connetto-core/src/device_cert/mod.rs index 5853ace3..ae5f81ec 100644 --- a/crates/connetto-core/src/device_cert/mod.rs +++ b/crates/connetto-core/src/device_cert/mod.rs @@ -15,6 +15,7 @@ mod key; pub mod layout; mod request; mod revocation; +mod serial; pub use attestation::{ ANDROID_ATTESTATION_CHALLENGE, ATTESTATION_EXTENSION, ATTESTATION_OID_IS_STAND_IN, @@ -29,6 +30,7 @@ pub use revocation::{ ListError, RevocationList, Revoked, certificate_key_id, certificate_serial, verify_chain, verify_signer, }; +pub use serial::CertificateSerial; /// What an application tells the lost-device list about a device, a tuple or a /// struct it defines, sent at enrolment in `MessagePack` of at most 4096 bytes. diff --git a/crates/connetto-core/src/device_cert/serial.rs b/crates/connetto-core/src/device_cert/serial.rs new file mode 100644 index 00000000..9e6d94ed --- /dev/null +++ b/crates/connetto-core/src/device_cert/serial.rs @@ -0,0 +1,46 @@ +/// The X.509 serial of a certificate the deployment signs. +/// +/// Sixteen octets whose first is `0x01` through `0x7F`, so the DER +/// `INTEGER` of it is positive and minimal, the form RFC 5280 4.1.2.2 +/// and the DER encoding rules ask for. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct CertificateSerial([u8; 16]); + +impl CertificateSerial { + /// The serial over `bytes`, when its first octet is `0x01` through `0x7F`. + /// + /// `None` when the first octet is `0x00`, which a minimal `INTEGER` + /// drops, or `0x80` or higher, which would read as negative. + #[must_use] + pub const fn new(bytes: [u8; 16]) -> Option { + match bytes[0] { + 0x01..=0x7F => Some(Self(bytes)), + _ => None, + } + } + + /// A fresh serial, drawn from `fill` until its first octet is positive + /// and minimal, so the redraw keeps the draw unbiased. + /// + /// # Errors + /// + /// The error `fill` reports when it refuses a draw. + pub fn random(mut fill: F) -> Result + where + F: FnMut(&mut [u8]) -> Result<(), E>, + { + loop { + let mut bytes = [0_u8; 16]; + fill(&mut bytes)?; + if let Some(serial) = Self::new(bytes) { + return Ok(serial); + } + } + } + + /// The octets, as the record and the certificate carry them. + #[must_use] + pub const fn as_bytes(self) -> [u8; 16] { + self.0 + } +} diff --git a/crates/connetto-core/src/device_cert/tests.rs b/crates/connetto-core/src/device_cert/tests.rs index a811b1b5..d3c0643d 100644 --- a/crates/connetto-core/src/device_cert/tests.rs +++ b/crates/connetto-core/src/device_cert/tests.rs @@ -30,7 +30,7 @@ fn authorities(start: SystemTime) -> (RootCa, DeviceIssuer) { &issuer_key.subject_public_key_info(), start, YEAR + 30 * DAY, - [2; 16], + serial(2), ) .expect("sign the issuer"); let issuer = @@ -45,6 +45,11 @@ fn device_request(nonce: &[u8; 32]) -> (KeyPair, Vec) { (key, csr) } +/// A serial for tests, all sixteen octets `byte`. +fn serial(byte: u8) -> CertificateSerial { + CertificateSerial::new([byte; 16]).expect("the test serial is positive") +} + fn verify_chain( root: &RootCa, issuer: &DeviceIssuer, @@ -142,7 +147,7 @@ fn an_issued_certificate_carries_exactly_the_profile() { "c0ffee-42", start, DAY, - [9; 16], + serial(9), AttestationLevel::ChipProven, ) .expect("issue"); @@ -174,6 +179,37 @@ fn an_issued_certificate_carries_exactly_the_profile() { ); } +#[test] +fn a_serial_the_integer_cannot_carry_is_refused() { + assert!(CertificateSerial::new([0x00; 16]).is_none()); + assert!(CertificateSerial::new([0x80; 16]).is_none()); + assert!(CertificateSerial::new([0xFF; 16]).is_none()); + assert_eq!(serial(0x01).as_bytes(), [0x01; 16]); + assert_eq!(serial(0x7F).as_bytes(), [0x7F; 16]); +} + +#[test] +fn boundary_serials_read_back_as_issued() { + let start = at(1_800_000_000); + let (_, issuer) = authorities(start); + let (_, csr) = device_request(&[7; 32]); + let request = CertificateRequest::parse(&csr).expect("parse"); + for serial in [serial(0x01), serial(0x7F)] { + let leaf = issuer + .issue( + &request, + "acct", + start, + DAY, + serial, + AttestationLevel::Unproven, + ) + .expect("issue"); + let cert = DeviceCertificate::parse(&leaf).expect("the profile"); + assert_eq!(cert.serial(), serial.as_bytes().as_slice()); + } +} + #[test] fn fields_the_request_asks_for_are_ignored() { let start = at(1_800_000_000); @@ -196,7 +232,7 @@ fn fields_the_request_asks_for_are_ignored() { "acct", start, DAY, - [3; 16], + serial(3), AttestationLevel::Unproven, ) .expect("issue"); @@ -251,7 +287,7 @@ fn a_certificate_outliving_its_issuer_is_not_issued() { "acct", start + YEAR, 31 * DAY, - [3; 16], + serial(3), AttestationLevel::Unproven ) .err(), @@ -266,7 +302,12 @@ fn an_issuer_the_root_did_not_sign_is_refused() { let other = RootCa::create(deployment(), start, 10 * YEAR).expect("other root"); let issuer_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("issuer key"); let issuer_cert = other - .sign_issuer(&issuer_key.subject_public_key_info(), start, YEAR, [2; 16]) + .sign_issuer( + &issuer_key.subject_public_key_info(), + start, + YEAR, + serial(2), + ) .expect("sign"); assert_eq!( DeviceIssuer::new(issuer_cert, issuer_key, root.certificate()).err(), @@ -356,7 +397,12 @@ fn a_root_reloaded_from_its_parts_signs_issuers_the_issuer_accepts() { let root = RootCa::from_parts(created.certificate().to_vec(), key).expect("reload the root"); let issuer_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("issuer key"); let issuer_cert = root - .sign_issuer(&issuer_key.subject_public_key_info(), start, YEAR, [2; 16]) + .sign_issuer( + &issuer_key.subject_public_key_info(), + start, + YEAR, + serial(2), + ) .expect("sign the issuer"); assert!(DeviceIssuer::new(issuer_cert, issuer_key, created.certificate()).is_ok()); } @@ -378,7 +424,12 @@ fn an_issuer_loads_from_its_stored_pkcs8_key() { let root = RootCa::create(deployment(), start, 10 * YEAR).expect("root"); let issuer_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("issuer key"); let issuer_cert = root - .sign_issuer(&issuer_key.subject_public_key_info(), start, YEAR, [2; 16]) + .sign_issuer( + &issuer_key.subject_public_key_info(), + start, + YEAR, + serial(2), + ) .expect("sign"); assert!( DeviceIssuer::from_pkcs8( @@ -428,7 +479,7 @@ fn a_device_key_requests_a_certificate_naming_it() { "acct", start, DAY, - [5; 16], + serial(5), AttestationLevel::Unproven, ) .expect("issue"); @@ -467,7 +518,7 @@ fn issued(issuer: &DeviceIssuer, start: SystemTime, serial: u8) -> Vec { "alice", start, DAY, - [serial; 16], + CertificateSerial::new([serial; 16]).expect("the test serial is positive"), AttestationLevel::Unproven, ) .expect("issue") @@ -477,7 +528,7 @@ fn issued(issuer: &DeviceIssuer, start: SystemTime, serial: u8) -> Vec { fn a_signed_list_verifies_against_its_root_and_names_its_serials() { let start = at(1_800_000_000); let (root, issuer) = authorities(start); - let leaf = issued(&issuer, start, 0x81); + let leaf = issued(&issuer, start, 0x11); let revoked = [Revoked { serial: vec![0x81; 16], at: start + DAY / 2, diff --git a/crates/connetto-server/src/device_cert/attestation.rs b/crates/connetto-server/src/device_cert/attestation.rs index dc2fbac6..b9df2719 100644 --- a/crates/connetto-server/src/device_cert/attestation.rs +++ b/crates/connetto-server/src/device_cert/attestation.rs @@ -833,7 +833,8 @@ mod tests { use std::time::{Duration, SystemTime, UNIX_EPOCH}; use connetto_core::device_cert::{ - ANDROID_ATTESTATION_CHALLENGE, AttestationLevel, DeploymentId, DeviceIssuer, RootCa, + ANDROID_ATTESTATION_CHALLENGE, AttestationLevel, CertificateSerial, DeploymentId, + DeviceIssuer, RootCa, }; use connetto_core::messages::DeviceAttestation; use rcgen::{ @@ -905,7 +906,12 @@ mod tests { .expect("root"); let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("issuer key"); let cert = root - .sign_issuer(&key.subject_public_key_info(), now, TEN_YEARS, [1; 16]) + .sign_issuer( + &key.subject_public_key_info(), + now, + TEN_YEARS, + CertificateSerial::new([1; 16]).expect("the serial is positive"), + ) .expect("issuer"); DeviceIssuer::new(cert, key, root.certificate()).expect("issuer") } diff --git a/crates/connetto-server/src/device_cert/enrolment.rs b/crates/connetto-server/src/device_cert/enrolment.rs index 9654e505..a51d96cd 100644 --- a/crates/connetto-server/src/device_cert/enrolment.rs +++ b/crates/connetto-server/src/device_cert/enrolment.rs @@ -7,10 +7,10 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; use connetto_core::SessionId; use connetto_core::device_cert::{ - AttestationLevel, CertificateRequest, IssueError, KeyId, ListError, Revoked, + AttestationLevel, CertificateRequest, CertificateSerial, IssueError, KeyId, ListError, Revoked, }; use connetto_core::messages::{DeviceSummary, EnrolGrant, EnrolRefusal, EnrolRequest, SignedList}; -use ring::rand::{SecureRandom as _, SystemRandom}; +use ring::rand::{SecureRandom, SystemRandom}; use tokio::time::Instant; use super::{DeviceCertConfig, LifetimeError, StatusList, verify}; @@ -352,12 +352,28 @@ pub enum RevokeError { Unavailable(#[source] EnrolmentError), } +/// A source the enrolment draws its nonces and serials from. +pub trait RandomSource: Send + Sync { + /// Draw random bytes into `dest`. + /// + /// # Errors + /// + /// The source's error. + fn fill(&self, dest: &mut [u8]) -> Result<(), ring::error::Unspecified>; +} + +impl RandomSource for SystemRandom { + fn fill(&self, dest: &mut [u8]) -> Result<(), ring::error::Unspecified> { + SecureRandom::fill(self, dest) + } +} + /// The issuer settings, the table enrolments are recorded in, and the /// revocation lists last published. pub struct DeviceEnrolment { config: DeviceCertConfig, store: Arc>, - random: SystemRandom, + random: Box, lists: tokio::sync::Mutex>>, revoker: Option, /// The attestation status list an Android chain's serials are checked against. @@ -379,7 +395,7 @@ impl DeviceEnrolment { Self { config, store, - random: SystemRandom::new(), + random: Box::new(SystemRandom::new()), lists: tokio::sync::Mutex::new(None), revoker: None, clock: SystemTime::now, @@ -396,6 +412,15 @@ impl DeviceEnrolment { self } + /// A source the enrolment draws its serials from, so a test holds a + /// draw the system source would not make. + #[cfg(feature = "test-seams")] + #[must_use] + pub fn with_random(mut self, random: Box) -> Self { + self.random = random; + self + } + /// Revoke the auth-store session a revoked device last enrolled with /// through `revoker`, so a stolen device still signed in cannot enrol a /// fresh key and undo the report. @@ -478,9 +503,7 @@ impl DeviceEnrolment { ceiling_secs: ceiling.as_secs(), }, })?; - let mut serial = [0; 16]; - self.random - .fill(&mut serial) + let serial = CertificateSerial::random(|bytes| self.random.fill(bytes)) .map_err(|_| EnrolRefusal::IssuerUnavailable)?; let not_before = (self.clock)(); let issuer = self.config.issuer(); @@ -496,7 +519,7 @@ impl DeviceEnrolment { let enrolment = Enrolment { user: user.clone(), key, - serial, + serial: serial.as_bytes(), issuer: issuer.key_id(), issued_at: not_before, expires_at: not_before + lifetime, diff --git a/crates/connetto-server/src/device_cert/mod.rs b/crates/connetto-server/src/device_cert/mod.rs index 27f54b72..c4780819 100644 --- a/crates/connetto-server/src/device_cert/mod.rs +++ b/crates/connetto-server/src/device_cert/mod.rs @@ -32,7 +32,7 @@ pub use connetto_core::device_cert::{KeyId, Revoked}; pub(crate) use enrolment::PendingChallenge; pub use enrolment::{ Device, DeviceEnrolment, Enrolment, EnrolmentError, EnrolmentFuture, EnrolmentStore, - MemoryEnrolments, Recorded, Revocation, RevokeError, SessionRevoker, + MemoryEnrolments, RandomSource, Recorded, Revocation, RevokeError, SessionRevoker, }; #[doc(hidden)] pub use schema::__key_id; diff --git a/crates/connetto-server/src/device_cert/tests.rs b/crates/connetto-server/src/device_cert/tests.rs index ead006e9..318a8375 100644 --- a/crates/connetto-server/src/device_cert/tests.rs +++ b/crates/connetto-server/src/device_cert/tests.rs @@ -1,6 +1,6 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; -use connetto_core::device_cert::{DeploymentId, DeviceIssuer, RootCa}; +use connetto_core::device_cert::{CertificateSerial, DeploymentId, DeviceIssuer, RootCa}; use rcgen::{KeyPair, PKCS_ECDSA_P256_SHA256, PublicKeyData as _}; use super::*; @@ -21,7 +21,12 @@ fn issuer(valid_for: Duration) -> DeviceIssuer { .expect("root"); let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("issuer key"); let cert = root - .sign_issuer(&key.subject_public_key_info(), start(), valid_for, [1; 16]) + .sign_issuer( + &key.subject_public_key_info(), + start(), + valid_for, + CertificateSerial::new([1; 16]).expect("the serial is positive"), + ) .expect("issuer"); DeviceIssuer::new(cert, key, root.certificate()).expect("load") } @@ -90,7 +95,12 @@ fn issuer_and_secret(valid_for: Duration) -> (DeviceIssuer, Vec) { let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("issuer key"); let secret = key.serialize_der(); let cert = root - .sign_issuer(&key.subject_public_key_info(), start(), valid_for, [1; 16]) + .sign_issuer( + &key.subject_public_key_info(), + start(), + valid_for, + CertificateSerial::new([1; 16]).expect("the serial is positive"), + ) .expect("issuer"); let issuer = DeviceIssuer::new(cert, key, root.certificate()).expect("load"); (issuer, secret) @@ -116,3 +126,30 @@ fn a_debug_render_carries_no_private_key() { ); } } + +#[test] +fn a_thousand_random_serials_read_back_as_issued() { + use connetto_core::device_cert::{AttestationLevel, CertificateRequest, DeviceCertificate}; + use ring::rand::{SecureRandom, SystemRandom}; + let issuer = issuer(30 * DAY); + let key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256).expect("device key"); + let csr = CertificateRequest::build(&key, &[7; 32]).expect("the request builds"); + let request = CertificateRequest::parse(&csr).expect("the request parses"); + let random = SystemRandom::new(); + for _ in 0..1000 { + let serial = + CertificateSerial::random(|bytes| SecureRandom::fill(&random, bytes)).expect("a draw"); + let leaf = issuer + .issue( + &request, + "alice", + start(), + DAY, + serial, + AttestationLevel::Unproven, + ) + .expect("issued"); + let parsed = DeviceCertificate::parse(&leaf).expect("the profile"); + assert_eq!(parsed.serial(), serial.as_bytes().as_slice()); + } +} diff --git a/crates/connetto-server/tests/it/device_identity.rs b/crates/connetto-server/tests/it/device_identity.rs index 293b8cea..5b4c13c3 100644 --- a/crates/connetto-server/tests/it/device_identity.rs +++ b/crates/connetto-server/tests/it/device_identity.rs @@ -315,7 +315,7 @@ async fn a_chip_proven_enrolment_records_its_level_and_a_renewal_keeps_it() { /// operator copies it to the server: `root.der` beside an issuer directory. fn ca_output(dir: &std::path::Path) -> (std::path::PathBuf, std::path::PathBuf) { use connetto_core::device_cert::layout::{ISSUER_CERTIFICATE, ISSUER_KEY, ROOT_CERTIFICATE}; - use connetto_core::device_cert::{DeploymentId, RootCa}; + use connetto_core::device_cert::{CertificateSerial, DeploymentId, RootCa}; use rcgen::{KeyPair, PKCS_ECDSA_P256_SHA256}; use std::time::{Duration, SystemTime}; @@ -333,7 +333,7 @@ fn ca_output(dir: &std::path::Path) -> (std::path::PathBuf, std::path::PathBuf) &key.subject_public_key_info(), now - day, 395 * day, - [2; 16], + CertificateSerial::new([2; 16]).expect("the serial is positive"), ) .expect("issuer"); let issuer_dir = dir.join("issuer-2026"); diff --git a/crates/connetto-server/tests/it/enrolment.rs b/crates/connetto-server/tests/it/enrolment.rs index b481f25e..f612a43e 100644 --- a/crates/connetto-server/tests/it/enrolment.rs +++ b/crates/connetto-server/tests/it/enrolment.rs @@ -5,8 +5,8 @@ use std::sync::atomic::{AtomicU32, Ordering}; use std::time::{Duration, SystemTime}; use connetto_core::device_cert::{ - ANDROID_ATTESTATION_CHALLENGE, AttestationLevel, CertificateRequest, DeploymentId, - DeviceCertificate, DeviceIssuer, KeyId, RootCa, + ANDROID_ATTESTATION_CHALLENGE, AttestationLevel, CertificateRequest, CertificateSerial, + DeploymentId, DeviceCertificate, DeviceIssuer, KeyId, RootCa, }; use connetto_core::messages::{ ControlMessage, DeviceAttestation, EnrolChallenge, EnrolChallengeRequest, EnrolGrant, @@ -14,7 +14,7 @@ use connetto_core::messages::{ }; use connetto_core::traits::{IncomingFrame, Transport}; use connetto_server::device_cert::{ - AndroidStatus, DeviceCertConfig, DeviceEnrolment, MemoryEnrolments, StatusList, + AndroidStatus, DeviceCertConfig, DeviceEnrolment, MemoryEnrolments, RandomSource, StatusList, }; use connetto_server::{AbuseConfig, LoopbackTransport, RequestGuard, ThrottleConfig, loopback}; use connetto_test_harness::{Fixture, RosterAuth, WITHHELD_ID}; @@ -52,7 +52,7 @@ pub(super) fn rooted_issuer() -> (Vec, Vec, DeviceIssuer) { &key.subject_public_key_info(), now - DAY, 395 * DAY, - [1; 16], + CertificateSerial::new([1; 16]).expect("the serial is positive"), ) .expect("issuer"); let issuer = DeviceIssuer::new(cert.clone(), key, root.certificate()).expect("load"); @@ -327,6 +327,60 @@ async fn a_signed_in_device_enrols_and_is_recorded() { assert_eq!(records[0].descriptor, vec![0x91, 0x01]); } +/// The serial a held draw hands the enrolment, its leading zero a +/// certificate cannot carry. +const ZERO_LEADING: [u8; 16] = [ + 0x00, 109, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, +]; +/// The serial a redraw hands it, one the certificate can carry. +const REDRAWN: [u8; 16] = [ + 109, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, +]; + +/// A source that holds its first 16-octet draw, a serial with a leading +/// zero, and hands the redraw one the certificate can carry. +struct HeldDraw { + held: AtomicU32, +} + +impl RandomSource for HeldDraw { + fn fill(&self, dest: &mut [u8]) -> Result<(), ring::error::Unspecified> { + if dest.len() == 16 { + let held = self.held.fetch_add(1, Ordering::Relaxed); + dest.copy_from_slice(if held == 0 { &ZERO_LEADING } else { &REDRAWN }); + } else { + dest.fill(0x11); + } + Ok(()) + } +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn a_held_zero_leading_draw_is_redrawn_and_recorded_as_issued() { + let fixture = Fixture::acquire().await; + let store = Arc::new(MemoryEnrolments::default()); + let (issuer_cert, issuer) = issuer(); + let enrolment = DeviceEnrolment::new(DeviceCertConfig::new(issuer), Arc::clone(&store) as _) + .with_random(Box::new(HeldDraw { + held: AtomicU32::new(0), + })); + let manager = enrolling_manager(&fixture, Some(enrolment)).await; + let mut client = connect(&manager, Some("alice")).await; + let key = device_key(); + let handed = nonce(&mut client).await; + let reply = enrol(&mut client, &key, handed, None, vec![0x91, 0x01]).await; + let ControlMessage::EnrolGrant(EnrolGrant { chain, .. }) = reply else { + panic!("expected a grant, got {reply:?}"); + }; + assert_eq!(chain.len(), 2); + assert_eq!(chain[1].as_slice(), issuer_cert.as_slice()); + let leaf = DeviceCertificate::parse(&chain[0]).expect("the leaf meets the profile"); + assert_eq!(leaf.serial(), REDRAWN.as_slice()); + let records = store.records(); + assert_eq!(records.len(), 1); + assert_eq!(records[0].serial.as_slice(), leaf.serial()); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn an_anonymous_caller_cannot_enrol() { let fixture = Fixture::acquire().await; diff --git a/crates/connetto-server/tests/it/revocation.rs b/crates/connetto-server/tests/it/revocation.rs index 52805a69..05343531 100644 --- a/crates/connetto-server/tests/it/revocation.rs +++ b/crates/connetto-server/tests/it/revocation.rs @@ -530,7 +530,7 @@ type IssuerBytes = (Vec, Vec); /// A root and two issuers it signed, each as its certificate and PKCS #8 key, /// so a test loads one issuer into two configurations. fn root_and_two_issuers() -> (connetto_core::device_cert::RootCa, IssuerBytes, IssuerBytes) { - use connetto_core::device_cert::{DeploymentId, RootCa}; + use connetto_core::device_cert::{CertificateSerial, DeploymentId, RootCa}; let now = std::time::SystemTime::now(); let day = Duration::from_hours(24); let root = RootCa::create( @@ -546,7 +546,7 @@ fn root_and_two_issuers() -> (connetto_core::device_cert::RootCa, IssuerBytes, I &key.subject_public_key_info(), now - day, 395 * day, - [serial; 16], + CertificateSerial::new([serial; 16]).expect("the serial is positive"), ) .expect("issuer"); (cert, key.serialize_der()) diff --git a/plans/master-implementation-plan.md b/plans/master-implementation-plan.md index 9e22e84e..bef26b0d 100644 --- a/plans/master-implementation-plan.md +++ b/plans/master-implementation-plan.md @@ -5130,7 +5130,7 @@ Each was asked with primary references for every option, and the rejected option 14. **The issuer is replaced every year and the root lasts 10 years** (2026-10-03). `connetto-ca` signs a new issuer yearly, valid one year plus the certificate ceiling so its last certificates stay checkable, within NIST SP 800-57's one-to-three-year bound for a private signature key. The root lasts 10 years. A device trusts every root its build carries, so the next root is signed and carried beside the current one well before the current one expires. The server warns at startup when its issuer has less than 60 days left. A three-year issuer with a 20-year root was rejected, since a stolen issuer key would then sign for up to three years unless the root revokes it first. 15. **`connetto-ca` stores the root key encrypted under an operator passphrase** (2026-10-03). The root key is written as PKCS #8 encrypted with PBES2 (scrypt and AES-256-CBC, RFC 8018) and the passphrase is asked at every ceremony, as step-ca encrypts its keys with a password, so a copy found on a backup is useless without the passphrase and the operator can keep several copies apart. The issuer key on the server stays a plain file, provisioned like `CONNETTO_CONTENT_KEY`. A plain root key file kept offline was rejected, since anyone copying it could sign issuers and revocation lists until the root is replaced. 16. **A device without a usable key chip gets a software key in its secret store** (2026-10-03). A Windows machine without a TPM the app can use, an Intel Mac without Touch ID, or a Mac build without the keychain entitlement creates its P-256 key in software and keeps it unprompted in the plain store beside its secrets (R51 to R53 and R71), so it still enrols and peer-syncs. Its certificate records `unproven`, so a deployment refusing unproven devices shuts it out. Android needs no fallback, since every handheld backs its keystore with an isolated environment (CDD 9.11 H-0-2). Leaving such a device without an identity was rejected, since Chrome measured only about 60% of Windows users with a usable TPM and every Intel Mac without Touch ID would be left out of peer sync. -17. **Enrolments and their certificates are two tables the enrolment schema trait names** (2026-10-04). One row per device key holds the account, attestation, session, times and descriptor, and one row per issued certificate holds its X.509 serial, its key and its expiry, so a lost-device report lists every unexpired serial of the key by a join. The serial is 16 random bytes, the RFC 5280 identifier a revocation list names a certificate by. A key already enrolled under one account is refused to any other. Serial arrays on the enrolment row were rejected, since PostgreSQL's documentation advises a row per element over searching arrays, and so was one row per certificate repeating the enrolment's fields. +17. **Enrolments and their certificates are two tables the enrolment schema trait names** (2026-10-04). One row per device key holds the account, attestation, session, times and descriptor, and one row per issued certificate holds its X.509 serial, its key and its expiry, so a lost-device report lists every unexpired serial of the key by a join. The serial is 16 random bytes, the RFC 5280 identifier a revocation list names a certificate by. The first byte is drawn in 1 to 127, so the serial is positive and minimal, the form RFC 5280 and the DER encoding rules ask of it. A key already enrolled under one account is refused to any other. Serial arrays on the enrolment row were rejected, since PostgreSQL's documentation advises a row per element over searching arrays, and so was one row per certificate repeating the enrolment's fields. 18. **A native client enrols from a task beside its pump** (2026-10-04). On every attach and when half-life passes on a live connection, the shared client asks for a challenge, signs on the key's own thread and sends the request. The pump routes each `Enrol*` answer to the waiting request by its `request_id` and never to `ClientEvent`, every wait ends after 30 seconds on a monotonic clock and at once on disconnect, and `reissue_certificate` uses the same path. Sync starts without waiting for enrolment. Enrolling inline in `attach` before the subscriptions replay was rejected, since it delays every connect by two round trips and a signature and still leaves half-life renewal and `reissue_certificate` needing the same machinery. 19. **The client keeps its certificate and lists in connetto's own replica tables** (2026-10-04). `_connetto_device_certificate` holds the chain, its validity and the stored lifetime, created like `_connetto_pending`, so it exists for every application, belongs to its account, is encrypted under the replica key and goes with the replica on `forget_device`. Step 5's lists go in `_connetto_revocation_list` beside it. The device-private tier was rejected, since it exists only when the application declares one, and so was a reserved keyring record, since the keyring holds secrets as strings. 20. **An Android application hands the enrolment its JNI access through the builder** (2026-10-04). `NativeDurable::with_java_access(Arc)` exists on Android only, and `connetto-auth-session` offers `java_access()` over `manganis::android::with_activity`, so a Dioxus application writes one line. An Android build with `device-identity` that connects without it fails with a typed error naming the setter, and decision 16 keeps Android without a software fallback. A software key on Android without the setter was rejected, since it would enrol copyable keys unseen, and so was `connetto-client` reaching the Activity through manganis itself, since that ties the core client to Dioxus.