From 2eff6cd7993ebdfbd23803dbcf6a9399c7f5172f Mon Sep 17 00:00:00 2001 From: Rodolphe Breard Date: Wed, 8 May 2019 19:21:32 +0200 Subject: [PATCH] Display a warning when fetching order or an authorization Orders and authorization can both contain an error which can, for example, help an user to fix a broken hook. It is therefore very useful to display it. Plus, when pooling one of those objects, having an error does not mean we should stop pooling since the error may be temporary. --- CHANGELOG.md | 6 +++++ acmed/src/acme_proto.rs | 15 ++++++++--- acmed/src/acme_proto/http.rs | 7 +++-- acmed/src/acme_proto/structs.rs | 2 +- acmed/src/acme_proto/structs/authorization.rs | 27 +++++++++++++++++-- acmed/src/acme_proto/structs/error.rs | 18 +++++++++++-- acmed/src/acme_proto/structs/order.rs | 9 ++++++- 7 files changed, 73 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6df19b6..de2564b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,12 @@ 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). +## [Unreleased] + +### Added +- ACMEd now displays a warning when the server indicates an error in an order or an authorization. + + ## [0.4.0] - 2019-05-08 ### Added diff --git a/acmed/src/acme_proto.rs b/acmed/src/acme_proto.rs index 5ff2ac3..be0d866 100644 --- a/acmed/src/acme_proto.rs +++ b/acmed/src/acme_proto.rs @@ -1,12 +1,12 @@ use crate::acme_proto::account::AccountManager; use crate::acme_proto::jws::encode_kid; use crate::acme_proto::structs::{ - Authorization, AuthorizationStatus, NewOrder, Order, OrderStatus, + ApiError, Authorization, AuthorizationStatus, NewOrder, Order, OrderStatus, }; use crate::certificate::Certificate; use crate::storage; use acme_common::error::Error; -use log::info; +use log::{info, warn}; use std::fmt; mod account; @@ -89,6 +89,9 @@ pub fn request_certificate(cert: &Certificate, root_certs: &[String]) -> Result< let data_builder = set_data_builder!(account, new_order.as_bytes(), directory.new_order); let (order, order_url, mut nonce): (Order, String, String) = http::get_obj_loc(root_certs, &directory.new_order, &data_builder, &nonce)?; + if let Some(e) = order.get_error() { + warn!("Error: {}", e); + } // 5. Get all the required authorizations for auth_url in order.authorizations.iter() { @@ -97,6 +100,9 @@ pub fn request_certificate(cert: &Certificate, root_certs: &[String]) -> Result< http::get_obj(root_certs, &auth_url, &data_builder, &nonce)?; nonce = new_nonce; + if let Some(e) = auth.get_error() { + warn!("Error: {}", e); + } if auth.status == AuthorizationStatus::Valid { continue; } @@ -148,8 +154,11 @@ pub fn request_certificate(cert: &Certificate, root_certs: &[String]) -> Result< let (priv_key, pub_key) = certificate::get_key_pair(cert)?; let csr = certificate::generate_csr(cert, &priv_key, &pub_key)?; let data_builder = set_data_builder!(account, csr.as_bytes(), order.finalize); - let (_, nonce): (Order, String) = + let (order, nonce): (Order, String) = http::get_obj(root_certs, &order.finalize, &data_builder, &nonce)?; + if let Some(e) = order.get_error() { + warn!("Error: {}", e); + } // 12. Pool the order in order to see whether or not it is valid let data_builder = set_empty_data_builder!(account, order_url); diff --git a/acmed/src/acme_proto/http.rs b/acmed/src/acme_proto/http.rs index a03bb40..7ac36a1 100644 --- a/acmed/src/acme_proto/http.rs +++ b/acmed/src/acme_proto/http.rs @@ -1,4 +1,4 @@ -use crate::acme_proto::structs::{AcmeError, Directory, HttpApiError}; +use crate::acme_proto::structs::{AcmeError, ApiError, Directory, HttpApiError}; use acme_common::error::Error; use http_req::request::{Method, Request}; use http_req::response::Response; @@ -213,7 +213,7 @@ pub fn pool_obj( nonce: &str, ) -> Result<(T, String), Error> where - T: std::str::FromStr, + T: std::str::FromStr + ApiError, G: Fn(&str) -> Result, S: Fn(&T) -> bool, { @@ -224,6 +224,9 @@ where if break_fn(&obj) { return Ok((obj, new_nonce)); } + if let Some(e) = obj.get_error() { + warn!("Error: {}", e); + } nonce = new_nonce; } let msg = format!("Pooling failed for {}", url); diff --git a/acmed/src/acme_proto/structs.rs b/acmed/src/acme_proto/structs.rs index 425c850..49bfe73 100644 --- a/acmed/src/acme_proto/structs.rs +++ b/acmed/src/acme_proto/structs.rs @@ -22,5 +22,5 @@ pub use account::{Account, AccountDeactivation, AccountResponse, AccountUpdate}; pub use authorization::{Authorization, AuthorizationStatus, Challenge}; pub use deserialize_from_str; pub use directory::Directory; -pub use error::{AcmeError, HttpApiError}; +pub use error::{AcmeError, ApiError, HttpApiError}; pub use order::{Identifier, IdentifierType, NewOrder, Order, OrderStatus}; diff --git a/acmed/src/acme_proto/structs/authorization.rs b/acmed/src/acme_proto/structs/authorization.rs index cc6173b..8415bc2 100644 --- a/acmed/src/acme_proto/structs/authorization.rs +++ b/acmed/src/acme_proto/structs/authorization.rs @@ -1,5 +1,5 @@ use crate::acme_proto::jws::algorithms::SignatureAlgorithm; -use crate::acme_proto::structs::Identifier; +use crate::acme_proto::structs::{ApiError, HttpApiError, Identifier}; use acme_common::b64_encode; use acme_common::error::Error; use openssl::pkey::{PKey, Private}; @@ -32,6 +32,18 @@ impl FromStr for Authorization { } } +impl ApiError for Authorization { + fn get_error(&self) -> Option { + for challenge in self.challenges.iter() { + let err = challenge.get_error(); + if err.is_some() { + return err; + } + } + None + } +} + #[derive(Debug, PartialEq, Eq, Deserialize)] #[serde(rename_all = "lowercase")] pub enum AuthorizationStatus { @@ -123,12 +135,23 @@ impl Challenge { } } +impl ApiError for Challenge { + fn get_error(&self) -> Option { + match self { + Challenge::Http01(tc) | Challenge::Dns01(tc) | Challenge::TlsAlpn01(tc) => { + tc.error.to_owned().map(Error::from) + } + Challenge::Unknown => None, + } + } +} + #[derive(PartialEq, Deserialize)] pub struct TokenChallenge { pub url: String, pub status: Option, pub validated: Option, - pub error: Option, // TODO: set the correct object + pub error: Option, pub token: String, } diff --git a/acmed/src/acme_proto/structs/error.rs b/acmed/src/acme_proto/structs/error.rs index fafd24c..8d4a288 100644 --- a/acmed/src/acme_proto/structs/error.rs +++ b/acmed/src/acme_proto/structs/error.rs @@ -3,6 +3,10 @@ use serde::Deserialize; use std::fmt; use std::str::FromStr; +pub trait ApiError { + fn get_error(&self) -> Option; +} + #[derive(PartialEq)] pub enum AcmeError { AccountDoesNotExist, @@ -123,12 +127,12 @@ impl From for Error { } } -#[derive(Deserialize)] +#[derive(Clone, PartialEq, Deserialize)] pub struct HttpApiError { #[serde(rename = "type")] error_type: Option, // title: Option, - // status: Option, + status: Option, detail: Option, // instance: Option, // TODO: implement subproblems @@ -142,6 +146,10 @@ impl fmt::Display for HttpApiError { .detail .to_owned() .unwrap_or_else(|| self.get_acme_type().to_string()); + let msg = match self.status { + Some(s) => format!("Status {}: {}", s, msg), + None => msg, + }; write!(f, "{}", msg) } } @@ -157,3 +165,9 @@ impl HttpApiError { self.get_type().into() } } + +impl From for Error { + fn from(error: HttpApiError) -> Self { + error.to_string().into() + } +} diff --git a/acmed/src/acme_proto/structs/order.rs b/acmed/src/acme_proto/structs/order.rs index 50ef7ec..76146f1 100644 --- a/acmed/src/acme_proto/structs/order.rs +++ b/acmed/src/acme_proto/structs/order.rs @@ -1,3 +1,4 @@ +use crate::acme_proto::structs::{ApiError, HttpApiError}; use acme_common::error::Error; use serde::{Deserialize, Serialize}; use std::fmt; @@ -28,12 +29,18 @@ pub struct Order { pub identifiers: Vec, pub not_before: Option, pub not_after: Option, - pub error: Option, // TODO: set the correct structure + pub error: Option, pub authorizations: Vec, pub finalize: String, pub certificate: Option, } +impl ApiError for Order { + fn get_error(&self) -> Option { + self.error.to_owned().map(Error::from) + } +} + deserialize_from_str!(Order); #[derive(Debug, PartialEq, Eq, Deserialize)]