Issue certificate serials as positive minimal integers (R74)

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<DeploymentI
 /// [`CaError::Passphrase`] when the passphrase does not open the root key,
 /// [`CaError::Exists`] when `out_dir` already holds an issuer, or the error
 /// of a step that failed.
+///
+/// # Panics
+///
+/// When the system source refuses a serial draw, which the OS source does
+/// not.
 pub fn sign_issuer(
     ca_dir: &Path,
     passphrase: &str,
@@ -110,8 +116,11 @@ pub fn sign_issuer(
     refuse_existing(&[&certificate_path, &key_path])?;
     let root = open_root(ca_dir, passphrase)?;
     let issuer_key = KeyPair::generate_for(&PKCS_ECDSA_P256_SHA256)?;
-    let mut serial = [0_u8; 16];
-    OsRng.fill_bytes(&mut serial);
+    let serial = CertificateSerial::random(|bytes: &mut [u8]| -> 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,

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

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<Rotation> = 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(

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<Vec<u8>, 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<Vec<u8>, 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(),

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.

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<Self> {
+        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<F, E>(mut fill: F) -> Result<Self, E>
+    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
+    }
+}