Merge pull request #57 from dvdsk/fix-incorrect-fullpem

Fix incorrect fullpem

CHANGELOG.md

@@ -5,6 +5,18 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.3.0] - 2023-06-11
+
+### Added
+ - Print a short summary including paths after writing files to disk
+
+### Changes
+ - PEM output option now follows [rfc4346](https://www.rfc-editor.org/rfc/rfc4346#section-7.4.2) certificate lists order
+ - Challenge verification may take longer now, user is informed in if it takes long.
+
+### Fixes
+ - Full paths to output files are no longer miss their parent directory if it contained a dot
+
 ## [0.2.6] - 2023-06-03
 
 ### Added
@@ -14,7 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [0.2.5] - 2023-06-02
 
 ### Fixes
- - Continue y/n no longer continues when chosing n
+ - Continue y/n no longer continues when choosing n
 
 ## [0.2.4] - 2023-06-02
 

main/Cargo.toml

@@ -1,8 +1,9 @@
 [package]
 name = "renewc"
-version = "0.2.6"
+version = "0.3.0"
 authors = ["David Kleingeld <opensource@davidsk.dev>"]
 edition = "2021"
+rust-version = "1.70"
 description = "Certificate renewal, with advanced diagnostics without installing anything"
 license = "GNUv3"
 readme = "README.md"
@@ -32,7 +33,6 @@ hyper = { version = "0.14", features = ["client"] }
 itertools = "0.10"
 
 haproxy-config = "0.4"
-is-terminal = "0.4"
 rand = "0.8"
 # supports-color feature ends up enabling atty which has a RUSTSEC 
 # advisory against it. For now colors will ben enabled always.
@@ -42,7 +42,7 @@ yasna = "0.5"
 async-trait = "0.1"
 data-encoding = "2.4"
 pem = "2"
-strum = { version = "0.24.1", features = ["derive"] }
+strum = { version = "0.24", features = ["derive"] }
 
 [dev-dependencies]
 libc = "0.2"

main/src/advise.rs

@@ -116,8 +116,7 @@ pub fn given_existing(config: &Config, cert: &Option<Info>, stdout: &mut impl Wr
 
 #[must_use]
 fn exit_requested(w: &mut impl Write, config: &Config, question: &str) -> bool {
-    use is_terminal::IsTerminal;
-
+    use std::io::IsTerminal;
     info!(w, "{}", question);
 
     if config.non_interactive || !std::io::stdin().is_terminal() {

main/src/cert.rs

@@ -1,4 +1,4 @@
-use color_eyre::eyre::{self, bail};
+use color_eyre::eyre::{self, bail, Context};
 use format::{Label, PemItem};
 
 pub mod format;
@@ -19,17 +19,18 @@ pub struct MaybeSigned<P: PemItem> {
 impl<P: PemItem> std::fmt::Debug for MaybeSigned<P> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("MaybeSigned")
-            .field("certificate", &"censored for security")
+            .field("certificate", &"hidden to prevent security leaks")
             .field(
                 "private_key",
-                &self.private_key.as_ref().map(|_| "censored for security"),
+                &self.private_key.as_ref().map(|_| "hidden to prevent security leaks"),
             )
             .field(
                 "chain",
                 &self
                     .chain
                     .iter()
-                    .map(|_| "censored for security")
+                    .map(|p| p.as_bytes())
+                    .map(String::from_utf8)
                     .collect::<Vec<_>>(),
             )
             .finish()
@@ -49,14 +50,15 @@ pub struct Signed<P: PemItem> {
 impl<P: PemItem> std::fmt::Debug for Signed<P> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("Signed")
-            .field("certificate", &"censored for security")
-            .field("private_key", &"censored for security")
+            .field("certificate", &"hidden to prevent security leaks")
+            .field("private_key", &"hidden to prevent security leaks")
             .field(
                 "chain",
                 &self
                     .chain
                     .iter()
-                    .map(|_| "censored for security")
+                    .map(|p| p.as_bytes())
+                    .map(String::from_utf8)
                     .collect::<Vec<_>>(),
             )
             .finish()
@@ -82,53 +84,70 @@ impl<P: PemItem> TryFrom<MaybeSigned<P>> for Signed<P> {
 }
 
 impl<P: PemItem> Signed<P> {
-    /// last certificate in full chain must be the domains certificate
+    /// first certificate in full chain must be the domains certificate
     pub fn from_key_and_fullchain(
         private_key: String,
         mut full_chain: String,
     ) -> eyre::Result<Self> {
-        let start_cert = full_chain
-            .rfind("-----BEGIN CERTIFICATE-----")
-            .ok_or_else(|| eyre::eyre!("No certificates in full chain!"))?;
-        let certificate = PemItem::from_pem(full_chain.split_off(start_cert), Label::Certificate)?;
-
-        let mut chain = Vec::new();
+        let mut certs = Vec::new();
         while let Some(begin_cert) = full_chain.rfind("-----BEGIN CERTIFICATE-----") {
-            chain.push(PemItem::from_pem(
-                full_chain.split_off(begin_cert),
-                Label::Certificate,
-            )?);
+            certs.push(
+                PemItem::from_pem(full_chain.split_off(begin_cert), Label::Certificate)
+                    .wrap_err("failed to extract chain certificates")?,
+            );
         }
 
+        let signed = certs
+            .pop() // removes first element (list is reversed)
+            .ok_or_else(|| eyre::eyre!("no certificates in full chain"))?;
+        let chain = {
+            certs.reverse();
+            certs
+        };
+
         if chain.is_empty() {
             bail!("No chain certificates in full chain")
         }
 
-        let private_key = PemItem::from_pem(private_key, Label::PrivateKey)?;
+        let private_key = PemItem::from_pem(private_key, Label::PrivateKey)
+            .wrap_err("failed to extract private key")?;
 
-        Ok(Self { certificate, private_key, chain })
+        Ok(Self {
+            certificate: signed,
+            private_key,
+            chain,
+        })
     }
 }
 
 impl<P> MaybeSigned<P>
 where
     P: PemItem,
 {
+    /// expects the signed certificate to be the first certificate
+    /// item (see certificate list in
+    /// [rfc4346 section 7.4.2](https://www.rfc-editor.org/rfc/rfc4346#section-7.4.2)
     pub(super) fn from_pem(bytes: Vec<u8>) -> eyre::Result<Self> {
         let mut pem = String::from_utf8(bytes)?;
         let start_key = pem.rfind("-----BEGIN PRIVATE KEY-----");
         let private_key = start_key
             .map(|i| pem.split_off(i))
             .map(|p| PemItem::from_pem(p, Label::PrivateKey))
-            .transpose()?;
+            .transpose()
+            .wrap_err("failed to extract private key")?;
 
-        let start_cert = pem.rfind("-----BEGIN CERTIFICATE-----");
-        let certificate = start_cert.map(|i| pem.split_off(i)).ok_or(eyre::eyre!(
-            "Can not find a certificate label in the pem content"
-        ))?;
-        let certificate = PemItem::from_pem(certificate, Label::Certificate)?;
+        const DELIMITER: &'static str = "-----END CERTIFICATE-----";
+        let post_signed_cert = pem.find(DELIMITER).map(|i| i + DELIMITER.len());
+        let chain = post_signed_cert
+            .map(|i| pem.split_off(i))
+            .ok_or(eyre::eyre!(
+                "Can not find a certificate label in the pem content"
+            ))?;
+        let chain = P::chain_from_pem(chain.into_bytes()).wrap_err("failed to extract chain")?;
 
-        let chain = P::chain_from_pem(pem.into_bytes())?;
+        // only signed cert left in pem after we split off key and chain
+        let certificate = PemItem::from_pem(pem, Label::Certificate)
+            .wrap_err("failed to extract signed certificate")?;
 
         Ok(MaybeSigned {
             certificate,
@@ -137,3 +156,43 @@ where
         })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn acme_output_to_signed() {
+        let signed_cert = "-----BEGIN CERTIFICATE-----\r
+BQcDAjAMBgNVHRMBAf8EAjAAMB0GA1UdDgQWBBRfouS0E6yLB+fT3eNI3x8V9B81\r
+-----END CERTIFICATE-----\r\n";
+        let chain0 = "-----BEGIN CERTIFICATE-----\r
+BQcDAjAMBgNVHRMBAf8EAjAAMB0GA1UdDgQWBBRfouS0E6yLB+fT3eNI3x8V9B82\r
+-----END CERTIFICATE-----\r\n";
+        let chain1 = "-----BEGIN CERTIFICATE-----\r
+BQcDAjAMBgNVHRMBAf8EAjAAMB0GA1UdDgQWBBRfouS0E6yLB+fT3eNI3x8V9B83\r
+-----END CERTIFICATE-----\r\n";
+        let chain2 = "-----BEGIN CERTIFICATE-----\r
+BQcDAjAMBgNVHRMBAf8EAjAAMB0GA1UdDgQWBBRfouS0E6yLB+fT3eNI3x8V9B84\r
+-----END CERTIFICATE-----\r\n";
+
+        // in a single file the certificate is always before the
+        // full chain
+        let certs = format!("{signed_cert}\r\n{chain0}\r\n{chain1}\r\n{chain2}");
+
+        let key = "
+-----BEGIN PRIVATE KEY----- 
+BQcDAjAMBgNVHRMBAf8EAjAAMB0GA1UdDgQWBBRfouS0E6yLB+fT3eNI3x8V9B81
+-----END PRIVATE KEY-----
+            "
+        .to_string();
+
+        Signed::<pem::Pem>::from_key_and_fullchain(key, certs.clone()).unwrap();
+        let res = MaybeSigned::<pem::Pem>::from_pem(certs.into_bytes()).unwrap();
+        assert_eq!(res.certificate.to_string(), signed_cert);
+        let mut chain = res.chain.into_iter().map(|p| p.to_string());
+        assert_eq!(chain.next().unwrap(), chain0);
+        assert_eq!(chain.next().unwrap(), chain1);
+        assert_eq!(chain.next().unwrap(), chain2);
+    }
+}

main/src/cert/format.rs

@@ -3,8 +3,8 @@ use color_eyre::eyre::{self, bail, Context};
 use pem::Pem;
 
 impl PemItem for Pem {
-    fn into_bytes(self) -> Vec<u8> {
-        pem::encode(&self).into_bytes()
+    fn as_bytes(&self) -> Vec<u8> {
+        pem::encode(self).into_bytes()
     }
 
     fn from_pem(pem_encoded: impl AsRef<[u8]>, label: Label) -> eyre::Result<Self> {
@@ -36,7 +36,7 @@ impl PemItem for Pem {
 
 pub trait PemItem: Sized {
     #[must_use]
-    fn into_bytes(self) -> Vec<u8>;
+    fn as_bytes(&self) -> Vec<u8>;
 
     fn from_pem(encoded: impl AsRef<[u8]>, label: Label) -> eyre::Result<Self>
     where
@@ -118,6 +118,6 @@ mod tests {
         let der = Pem::from_pem(ROOT_CA, Label::Certificate).unwrap().der();
         assert_ne!(der.clone().into_bytes(), ROOT_CA);
         let pem: Pem = der.to_pem(Label::Certificate);
-        assert_eq!(pem.into_bytes(), ROOT_CA);
+        assert_eq!(pem.as_bytes(), ROOT_CA);
     }
 }

main/src/cert/info.rs

@@ -1,3 +1,5 @@
+use std::io::Write;
+
 use super::format::PemItem;
 use super::{load, Signed};
 use crate::config;
@@ -19,14 +21,18 @@ pub struct Info {
 }
 
 impl Info {
-    pub fn from_disk(config: &config::Config) -> eyre::Result<Option<Self>> {
-        let Some(signed) = load::from_disk::<pem::Pem>(config)? else {
+    pub fn from_disk(
+        config: &config::Config,
+        stdout: &mut impl Write,
+    ) -> eyre::Result<Option<Self>> {
+        let Some(signed) = load::from_disk::<pem::Pem>(config, stdout)? else {
             return Ok(None);
         };
         let info = analyze(signed)?;
         Ok(Some(info))
     }
 
+    /// how far before the expiration date to renew a certificate
     #[instrument(ret, skip(self))]
     pub fn renew_period(&self) -> Duration {
         let mut rng = rand::rngs::StdRng::seed_from_u64(self.seed);
@@ -53,32 +59,27 @@ impl Info {
 
 /// returns number of days until the first certificate in the chain
 /// expires and whether any certificate is from STAGING
+#[instrument(ret)]
 pub fn analyze(signed: Signed<impl PemItem>) -> eyre::Result<Info> {
-    let mut staging = false;
-    let mut expires_in = Duration::MAX;
-    let mut expires_at = u64::MAX;
-
-    let cert = signed.certificate.into_bytes();
+    let cert = signed.certificate.as_bytes();
     let cert = Pem::iter_from_buffer(&cert).next().unwrap()?;
     let cert = Pem::parse_x509(&cert)?;
 
-    staging |= cert
+    let staging = cert
         .issuer()
         .iter_organization()
         .map(|o| o.as_str().unwrap())
         .any(|s| s.contains("STAGING"));
-    expires_in = expires_in.min(
-        cert.validity()
-            .time_to_expiration()
-            .unwrap_or(Duration::ZERO),
-    );
-    expires_at = expires_at.min(
-        cert.validity()
-            .not_after
-            .timestamp()
-            .try_into()
-            .expect("got negative timestamp from x509 certificate, this is a bug"),
-    );
+    let expires_in = cert
+        .validity()
+        .time_to_expiration()
+        .unwrap_or(Duration::ZERO);
+    let expires_at = cert
+        .validity()
+        .not_after
+        .timestamp()
+        .try_into()
+        .expect("got negative timestamp from x509 certificate, this is a bug");
 
     Ok(Info {
         staging,