From 644484cb31d6fadce45daa99b5b719fd266e7ad9 Mon Sep 17 00:00:00 2001 From: Rodolphe Breard Date: Tue, 30 Apr 2019 16:49:23 +0200 Subject: [PATCH] Change the hook and domains definitions The previous system was too limited when it comes to flexibility using hooks. This limitation came from the false idea that, for a given certificate, all challenges must be validated with the same method. In order to prove that false, domains in a certificate can now make use of any challenge type available. In order to be more flexible, hooks are now given a type and are defined in the same registry (instead of 6). Each one will be called when considered relevant based on its type. --- CHANGELOG.md | 5 ++ README.md | 1 + acmed/acmed_example.toml | 27 +++++--- acmed/src/acme_proto.rs | 21 +++++-- acmed/src/acme_proto/certificate.rs | 2 +- acmed/src/certificate.rs | 81 +++++++++++++++--------- acmed/src/config.rs | 98 +++++++++++++---------------- acmed/src/hooks.rs | 14 ++--- acmed/src/main_event_loop.rs | 19 +++--- acmed/src/storage.rs | 9 +-- tacd/src/server.rs | 3 +- 11 files changed, 160 insertions(+), 120 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5c3c14..36cf6c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,16 +9,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - An account object has been added in the configuration. +- In the configuration, hooks now have a mandatory `type` variable. +- It is now possible to declare hooks to clean after the challenge validation hooks. - Failure recovery: HTTPS requests rejected by the server that are recoverable, like the badNonce error, are now retried several times before being considered a hard failure. - The TLS-ALPN-01 challenge is now supported. The proof is a string representation of the acmeIdentifier extension. The self-signed certificate itself has to be built by a hook. ### Changed - In the configuration, the `email` certificate field has been replaced by the `account` field which matches an account object. +- The format of the `domain` configuration variable has changed and now includes the challenge type. - The `token` challenge hook variable has been renamed `file_name`. +- The `challenge_hooks`, `post_operation_hooks`, `file_pre_create_hooks`, `file_post_create_hooks`, `file_pre_edit_hooks` and `file_post_edit_hooks` certificate variables has been replaced by `hooks`. - The logs has been purged from many useless debug and trace entries. ### Removed - The DER storage format has been removed. +- The `challenge` certificate variables has been removed. ## [0.2.1] - 2019-03-30 diff --git a/README.md b/README.md index b3c715e..aa64712 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,7 @@ The Automatic Certificate Management Environment (ACME), is an internet standard - Nice and simple configuration file - Retry of HTTPS request rejected with a badNonce or other recoverable errors - Optional private-key reuse (useful for [HPKP](https://en.wikipedia.org/wiki/HTTP_Public_Key_Pinning)) +- For a given certificate, each domain names may be validated using a different challenge. ## Planned features diff --git a/acmed/acmed_example.toml b/acmed/acmed_example.toml index 5829f8c..54bf238 100644 --- a/acmed/acmed_example.toml +++ b/acmed/acmed_example.toml @@ -13,13 +13,28 @@ url = "https://acme-staging-v02.api.letsencrypt.org/directory" tos_agreed = false [[hook]] -name = "http-echo" +name = "http-echo-create" +type = ["challenge-http-01"] cmd = "echo" args = ["{{proof}}"] stdout = "/srv/http/{{current_domain}}/.well-known/acme-challenge/{{file_name}}" +[[hook]] +name = "http-echo-clean" +type = ["challenge-http-01-clean"] +cmd = "rm" +args = [ + "-f", + "/srv/http/{{current_domain}}/.well-known/acme-challenge/{{file_name}}" +] + +[[group]] +name = "http-echo" +hooks = ["http-echo-create", "http-echo-clean"] + [[hook]] name = "email-report" +type = ["post-operation"] cmd = "sendmail" args = [ "-f", "noreply@example.org", @@ -41,12 +56,10 @@ email = "certs@example.org" account = "test_account" endpoint = "letsencrypt v2 staging" domains = [ - "example.org", - "sub-1.example.org", - "sub-2.example.org" + { dns = "example.org", challenge = "http-01"}, + { dns = "sub-1.example.org", challenge = "http-01" } + { dns = "sub-2.example.org", challenge = "http-01" } ] algorithm = "ecdsa_p384" kp_reuse = false -challenge = "http-01" -challenge_hooks = ["http-echo"] -post_operation_hooks = ["email-report"] +hooks = ["http-echo", "email-report"] diff --git a/acmed/src/acme_proto.rs b/acmed/src/acme_proto.rs index 3b1648d..c755e51 100644 --- a/acmed/src/acme_proto.rs +++ b/acmed/src/acme_proto.rs @@ -67,6 +67,13 @@ macro_rules! set_empty_data_builder { } pub fn request_certificate(cert: &Certificate) -> Result<(), Error> { + let domains = cert + .domains + .iter() + .map(|d| d.0.to_owned()) + .collect::>(); + let mut hook_datas = vec![]; + // 1. Get the directory let directory = http::get_directory(&cert.remote_url)?; @@ -77,7 +84,7 @@ pub fn request_certificate(cert: &Certificate) -> Result<(), Error> { let (account, nonce) = AccountManager::new(cert, &directory, &nonce)?; // 4. Create a new order - let new_order = NewOrder::new(&cert.domains); + let new_order = NewOrder::new(&domains); let new_order = serde_json::to_string(&new_order)?; let data_builder = set_data_builder!(account, new_order.as_bytes(), directory.new_order); let (order, order_url, mut nonce): (Order, String, String) = @@ -102,14 +109,16 @@ pub fn request_certificate(cert: &Certificate) -> Result<(), Error> { } // 6. For each authorization, fetch the associated challenges + let current_challenge = cert.get_domain_challenge(&auth.identifier.value)?; for challenge in auth.challenges.iter() { - if cert.challenge == *challenge { + if current_challenge == *challenge { let proof = challenge.get_proof(&account.priv_key)?; let file_name = challenge.get_file_name(); let domain = auth.identifier.value.to_owned(); // 7. Call the challenge hook in order to complete it - cert.call_challenge_hooks(&file_name, &proof, &domain)?; + let data = cert.call_challenge_hooks(&file_name, &proof, &domain)?; + hook_datas.push(data); // 8. Tell the server the challenge has been completed let chall_url = challenge.get_url(); @@ -153,6 +162,10 @@ pub fn request_certificate(cert: &Certificate) -> Result<(), Error> { let (crt, _) = http::get_certificate(&crt_url, &data_builder, &nonce)?; storage::write_certificate(cert, &crt.as_bytes())?; - info!("Certificate renewed for {}", cert.domains.join(", ")); + for (data, hook_type) in hook_datas.iter() { + cert.call_challenge_hooks_clean(&data, (*hook_type).to_owned())?; + } + + info!("Certificate renewed for {}", domains.join(", ")); Ok(()) } diff --git a/acmed/src/acme_proto/certificate.rs b/acmed/src/acme_proto/certificate.rs index 0a3cf12..debd6f4 100644 --- a/acmed/src/acme_proto/certificate.rs +++ b/acmed/src/acme_proto/certificate.rs @@ -46,7 +46,7 @@ pub fn generate_csr( builder.set_pubkey(pub_key)?; let ctx = builder.x509v3_context(None); let mut san = SubjectAlternativeName::new(); - for name in cert.domains.iter() { + for name in cert.domains.iter().map(|d| d.0.to_owned()) { san.dns(&name); } let san = san.build(&ctx)?; diff --git a/acmed/src/certificate.rs b/acmed/src/certificate.rs index 19a013f..1079219 100644 --- a/acmed/src/certificate.rs +++ b/acmed/src/certificate.rs @@ -1,5 +1,5 @@ use crate::acme_proto::Challenge; -use crate::config::Account; +use crate::config::{Account, HookType}; use crate::hooks::{self, ChallengeHookData, Hook, PostOperationHookData}; use crate::storage::{certificate_files_exists, get_certificate}; use acme_common::error::Error; @@ -42,14 +42,12 @@ impl fmt::Display for Algorithm { #[derive(Debug)] pub struct Certificate { pub account: Account, - pub domains: Vec, + pub domains: Vec<(String, Challenge)>, pub algo: Algorithm, pub kp_reuse: bool, pub remote_url: String, pub tos_agreed: bool, - pub challenge: Challenge, - pub challenge_hooks: Vec, - pub post_operation_hooks: Vec, + pub hooks: Vec, pub account_directory: String, pub crt_directory: String, pub crt_name: String, @@ -60,24 +58,20 @@ pub struct Certificate { pub pk_file_mode: u32, pub pk_file_owner: Option, pub pk_file_group: Option, - pub file_pre_create_hooks: Vec, - pub file_post_create_hooks: Vec, - pub file_pre_edit_hooks: Vec, - pub file_post_edit_hooks: Vec, } impl fmt::Display for Certificate { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let challenge_hooks = self - .challenge_hooks + let hooks = self + .hooks .iter() .map(std::string::ToString::to_string) .collect::>() .join(", "); - let post_operation_hooks = self - .post_operation_hooks + let domains = self + .domains .iter() - .map(std::string::ToString::to_string) + .map(|d| format!("{} ({})", d.0, d.1)) .collect::>() .join(", "); write!( @@ -87,21 +81,28 @@ Domains: {domains} Algorithm: {algo} Account: {account} Private key reuse: {kp_reuse} -Challenge: {challenge} -Challenge hooks: {challenge_hooks} -Post operation hooks: {post_operation_hooks}", - domains = self.domains.join(", "), +Hooks: {hooks}", + domains = domains, algo = self.algo, account = self.account.name, kp_reuse = self.kp_reuse, - challenge = self.challenge, - challenge_hooks = challenge_hooks, - post_operation_hooks = post_operation_hooks, + hooks = hooks, ) } } impl Certificate { + pub fn get_domain_challenge(&self, domain_name: &str) -> Result { + let domain_name = domain_name.to_string(); + for (domain, challenge) in self.domains.iter() { + if *domain == domain_name { + return Ok((*challenge).to_owned()); + } + } + let msg = format!("{}: domain name not found", domain_name); + Err(msg.into()) + } + pub fn should_renew(&self) -> Result { if !certificate_files_exists(&self) { debug!("certificate does not exist: requesting one"); @@ -135,27 +136,47 @@ impl Certificate { file_name: &str, proof: &str, domain: &str, - ) -> Result<(), Error> { + ) -> Result<(ChallengeHookData, HookType), Error> { + let challenge = self.get_domain_challenge(domain)?; let hook_data = ChallengeHookData { - domains: self.domains.to_owned(), algorithm: self.algo.to_string(), - challenge: self.challenge.to_string(), - current_domain: domain.to_string(), + challenge: challenge.to_string(), + domain: domain.to_string(), file_name: file_name.to_string(), proof: proof.to_string(), }; - hooks::call_multiple(&hook_data, &self.challenge_hooks)?; - Ok(()) + let hook_type = match challenge { + Challenge::Http01 => (HookType::ChallengeHttp01, HookType::ChallengeHttp01Clean), + Challenge::Dns01 => (HookType::ChallengeDns01, HookType::ChallengeDns01Clean), + Challenge::TlsAlpn01 => ( + HookType::ChallengeTlsAlpn01, + HookType::ChallengeTlsAlpn01Clean, + ), + }; + hooks::call(&hook_data, &self.hooks, hook_type.0)?; + Ok((hook_data, hook_type.1)) + } + + pub fn call_challenge_hooks_clean( + &self, + data: &ChallengeHookData, + hook_type: HookType, + ) -> Result<(), Error> { + hooks::call(data, &self.hooks, hook_type) } pub fn call_post_operation_hooks(&self, status: &str) -> Result<(), Error> { + let domains = self + .domains + .iter() + .map(|d| format!("{} ({})", d.0, d.1)) + .collect::>(); let hook_data = PostOperationHookData { - domains: self.domains.to_owned(), + domains, algorithm: self.algo.to_string(), - challenge: self.challenge.to_string(), status: status.to_string(), }; - hooks::call_multiple(&hook_data, &self.post_operation_hooks)?; + hooks::call(&hook_data, &self.hooks, HookType::PostOperation)?; Ok(()) } } diff --git a/acmed/src/config.rs b/acmed/src/config.rs index c6e9d22..449b1ae 100644 --- a/acmed/src/config.rs +++ b/acmed/src/config.rs @@ -1,9 +1,9 @@ -use crate::acme_proto::Challenge; use crate::certificate::Algorithm; use crate::hooks; use acme_common::error::Error; use log::info; use serde::Deserialize; +use std::fmt; use std::fs::{self, File}; use std::io::prelude::*; use std::path::Path; @@ -37,6 +37,7 @@ impl Config { if name == hook.name { let h = hooks::Hook { name: hook.name.to_owned(), + hook_type: hook.hook_type.to_owned(), cmd: hook.cmd.to_owned(), args: hook.args.to_owned(), stdin: hook.stdin.to_owned(), @@ -130,6 +131,8 @@ pub struct Endpoint { #[derive(Deserialize)] pub struct Hook { pub name: String, + #[serde(rename = "type")] + pub hook_type: Vec, pub cmd: String, pub args: Option>, pub stdin: Option, @@ -137,6 +140,28 @@ pub struct Hook { pub stderr: Option, } +#[derive(Clone, Debug, Eq, PartialEq, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum HookType { + FilePreCreate, + FilePostCreate, + FilePreEdit, + FilePostEdit, + #[serde(rename = "challenge-http-01")] + ChallengeHttp01, + #[serde(rename = "challenge-http-01-clean")] + ChallengeHttp01Clean, + #[serde(rename = "challenge-dns-01")] + ChallengeDns01, + #[serde(rename = "challenge-dns-01-clean")] + ChallengeDns01Clean, + #[serde(rename = "challenge-tls-alpn-01")] + ChallengeTlsAlpn01, + #[serde(rename = "challenge-tls-alpn-01-clean")] + ChallengeTlsAlpn01Clean, + PostOperation, +} + #[derive(Deserialize)] pub struct Group { pub name: String, @@ -153,20 +178,14 @@ pub struct Account { pub struct Certificate { pub account: String, pub endpoint: String, - pub domains: Vec, - pub challenge: String, + pub domains: Vec, pub algorithm: Option, pub kp_reuse: Option, pub directory: Option, pub name: Option, pub name_format: Option, pub formats: Option>, - pub challenge_hooks: Vec, - pub post_operation_hooks: Option>, - pub file_pre_create_hooks: Option>, - pub file_post_create_hooks: Option>, - pub file_pre_edit_hooks: Option>, - pub file_post_edit_hooks: Option>, + pub hooks: Vec, } impl Certificate { @@ -187,10 +206,6 @@ impl Certificate { Algorithm::from_str(algo) } - pub fn get_challenge(&self) -> Result { - Challenge::from_str(&self.challenge) - } - pub fn get_kp_reuse(&self) -> bool { match self.kp_reuse { Some(b) => b, @@ -201,7 +216,7 @@ impl Certificate { pub fn get_crt_name(&self) -> String { match &self.name { Some(n) => n.to_string(), - None => self.domains.first().unwrap().to_string(), + None => self.domains.first().unwrap().dns.to_owned(), } } @@ -245,53 +260,26 @@ impl Certificate { Ok(ep.tos_agreed) } - pub fn get_challenge_hooks(&self, cnf: &Config) -> Result, Error> { - get_hooks(&self.challenge_hooks, cnf) - } - - pub fn get_post_operation_hooks(&self, cnf: &Config) -> Result, Error> { - match &self.post_operation_hooks { - Some(hooks) => get_hooks(hooks, cnf), - None => Ok(vec![]), - } - } - - pub fn get_file_pre_create_hooks(&self, cnf: &Config) -> Result, Error> { - match &self.file_pre_create_hooks { - Some(hooks) => get_hooks(hooks, cnf), - None => Ok(vec![]), - } - } - - pub fn get_file_post_create_hooks(&self, cnf: &Config) -> Result, Error> { - match &self.file_post_create_hooks { - Some(hooks) => get_hooks(hooks, cnf), - None => Ok(vec![]), - } - } - - pub fn get_file_pre_edit_hooks(&self, cnf: &Config) -> Result, Error> { - match &self.file_pre_edit_hooks { - Some(hooks) => get_hooks(hooks, cnf), - None => Ok(vec![]), + pub fn get_hooks(&self, cnf: &Config) -> Result, Error> { + let mut res = vec![]; + for name in self.hooks.iter() { + let mut h = cnf.get_hook(&name)?; + res.append(&mut h); } + Ok(res) } +} - pub fn get_file_post_edit_hooks(&self, cnf: &Config) -> Result, Error> { - match &self.file_post_edit_hooks { - Some(hooks) => get_hooks(hooks, cnf), - None => Ok(vec![]), - } - } +#[derive(Clone, Debug, Deserialize)] +pub struct Domain { + pub challenge: String, + pub dns: String, } -fn get_hooks(lst: &[String], cnf: &Config) -> Result, Error> { - let mut res = vec![]; - for name in lst.iter() { - let mut h = cnf.get_hook(&name)?; - res.append(&mut h); +impl fmt::Display for Domain { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.dns) } - Ok(res) } fn create_dir(path: &str) -> Result<(), Error> { diff --git a/acmed/src/hooks.rs b/acmed/src/hooks.rs index 120f0d2..728af6f 100644 --- a/acmed/src/hooks.rs +++ b/acmed/src/hooks.rs @@ -1,3 +1,4 @@ +use crate::config::HookType; use acme_common::error::Error; use handlebars::Handlebars; use log::debug; @@ -12,16 +13,14 @@ use std::process::{Command, Stdio}; pub struct PostOperationHookData { pub domains: Vec, pub algorithm: String, - pub challenge: String, pub status: String, } #[derive(Serialize)] pub struct ChallengeHookData { - pub domains: Vec, + pub domain: String, pub algorithm: String, pub challenge: String, - pub current_domain: String, pub file_name: String, pub proof: String, } @@ -37,6 +36,7 @@ pub struct FileStorageHookData { #[derive(Clone, Debug)] pub struct Hook { pub name: String, + pub hook_type: Vec, pub cmd: String, pub args: Option>, pub stdin: Option, @@ -63,7 +63,7 @@ macro_rules! get_hook_output { }}; } -pub fn call(data: &T, hook: &Hook) -> Result<(), Error> { +fn call_single(data: &T, hook: &Hook) -> Result<(), Error> { debug!("Calling hook: {}", hook.name); let reg = Handlebars::new(); let mut v = vec![]; @@ -103,9 +103,9 @@ pub fn call(data: &T, hook: &Hook) -> Result<(), Error> { Ok(()) } -pub fn call_multiple(data: &T, hooks: &[Hook]) -> Result<(), Error> { - for hook in hooks.iter() { - call(data, &hook)?; +pub fn call(data: &T, hooks: &[Hook], hook_type: HookType) -> Result<(), Error> { + for hook in hooks.iter().filter(|h| h.hook_type.contains(&hook_type)) { + call_single(data, &hook)?; } Ok(()) } diff --git a/acmed/src/main_event_loop.rs b/acmed/src/main_event_loop.rs index 2cad4ff..78a3230 100644 --- a/acmed/src/main_event_loop.rs +++ b/acmed/src/main_event_loop.rs @@ -1,4 +1,5 @@ use crate::acme_proto::request_certificate; +use crate::acme_proto::Challenge; use crate::certificate::Certificate; use crate::config; use acme_common::error::Error; @@ -18,14 +19,16 @@ impl MainEventLoop { for crt in cnf.certificate.iter() { let cert = Certificate { account: crt.get_account(&cnf)?, - domains: crt.domains.to_owned(), + domains: crt + .domains + .iter() + .map(|d| (d.dns.to_owned(), Challenge::from_str(&d.challenge).unwrap())) + .collect(), algo: crt.get_algorithm()?, kp_reuse: crt.get_kp_reuse(), remote_url: crt.get_remote_url(&cnf)?, tos_agreed: crt.get_tos_agreement(&cnf)?, - challenge: crt.get_challenge()?, - challenge_hooks: crt.get_challenge_hooks(&cnf)?, - post_operation_hooks: crt.get_post_operation_hooks(&cnf)?, + hooks: crt.get_hooks(&cnf)?, account_directory: cnf.get_account_dir(), crt_directory: crt.get_crt_dir(&cnf), crt_name: crt.get_crt_name(), @@ -36,10 +39,6 @@ impl MainEventLoop { pk_file_mode: cnf.get_pk_file_mode(), pk_file_owner: cnf.get_pk_file_user(), pk_file_group: cnf.get_pk_file_group(), - file_pre_create_hooks: crt.get_file_pre_create_hooks(&cnf)?, - file_post_create_hooks: crt.get_file_post_create_hooks(&cnf)?, - file_pre_edit_hooks: crt.get_file_pre_edit_hooks(&cnf)?, - file_post_edit_hooks: crt.get_file_post_edit_hooks(&cnf)?, }; certs.push(cert); } @@ -60,7 +59,7 @@ impl MainEventLoop { let msg = format!( "Unable to renew the {} certificate for {}: {}", crt.algo, - crt.domains.first().unwrap(), + crt.domains.first().unwrap().0, e ); warn!("{}", msg); @@ -73,7 +72,7 @@ impl MainEventLoop { let msg = format!( "{} certificate for {}: post-operation hook error: {}", crt.algo, - crt.domains.first().unwrap(), + crt.domains.first().unwrap().0, e ); warn!("{}", msg); diff --git a/acmed/src/storage.rs b/acmed/src/storage.rs index a4dc170..6c8e054 100644 --- a/acmed/src/storage.rs +++ b/acmed/src/storage.rs @@ -1,4 +1,5 @@ use crate::certificate::Certificate; +use crate::config::HookType; use crate::hooks::{self, FileStorageHookData}; use acme_common::b64_encode; use acme_common::error::Error; @@ -141,9 +142,9 @@ fn write_file(cert: &Certificate, file_type: FileType, data: &[u8]) -> Result<() let is_new = !path.is_file(); if is_new { - hooks::call_multiple(&hook_data, &cert.file_pre_create_hooks)?; + hooks::call(&hook_data, &cert.hooks, HookType::FilePreCreate)?; } else { - hooks::call_multiple(&hook_data, &cert.file_pre_edit_hooks)?; + hooks::call(&hook_data, &cert.hooks, HookType::FilePreEdit)?; } trace!("Writing file {:?}", path); @@ -165,9 +166,9 @@ fn write_file(cert: &Certificate, file_type: FileType, data: &[u8]) -> Result<() } if is_new { - hooks::call_multiple(&hook_data, &cert.file_post_create_hooks)?; + hooks::call(&hook_data, &cert.hooks, HookType::FilePostCreate)?; } else { - hooks::call_multiple(&hook_data, &cert.file_post_edit_hooks)?; + hooks::call(&hook_data, &cert.hooks, HookType::FilePostEdit)?; } Ok(()) } diff --git a/tacd/src/server.rs b/tacd/src/server.rs index 8a9c7c5..b039365 100644 --- a/tacd/src/server.rs +++ b/tacd/src/server.rs @@ -20,8 +20,7 @@ pub fn start( let mut acceptor = SslAcceptor::mozilla_intermediate(SslMethod::tls())?; acceptor.set_alpn_select_callback(|_, client| { debug!("ALPN negociation"); - ssl::select_next_proto(crate::ALPN_ACME_PROTO_NAME, client) - .ok_or(ALPN_ERROR) + ssl::select_next_proto(crate::ALPN_ACME_PROTO_NAME, client).ok_or(ALPN_ERROR) }); acceptor.set_private_key(private_key)?; acceptor.set_certificate(certificate)?;