Skip to content

Commit

Permalink
Stop to require the orders field on account creation
Browse files Browse the repository at this point in the history
RFC 8555 states that:
 - when an account is successfully created, the server "returns this
   account object" (section 7.3);
 - the `orders` field in account objects is mandatory (section 7.1.2).

Despite that, Boulder does not returns the `orders` field when an
account is created. This non-standard behavior prevented ACMEd from
creating account and testing them for existence.

In order to allow ACMEd to retrieve certificates from CAs using Boulder,
the `orders` field is no longer mandatory and the account existence is
tested when the order is requested.
letsencrypt/boulder#3335
  • Loading branch information
breard-r committed Sep 26, 2020
1 parent 0c8b0d3 commit 0db5e68
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 47 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Some subject attributes can now be specified.
- Support for NIST P-521 certificates and account keys.

### Fixed
- Support for Let's Encrypt non-standard account creation object.


## [0.11.0] - 2020-09-19

Expand Down
23 changes: 13 additions & 10 deletions acmed/src/account.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use crate::acme_proto::account::{
check_account_exists, register_account, update_account_contacts, update_account_key,
};
use crate::acme_proto::account::{register_account, update_account_contacts, update_account_key};
use crate::endpoint::Endpoint;
use crate::logs::HasLogger;
use crate::storage::FileManager;
Expand Down Expand Up @@ -67,7 +65,7 @@ impl AccountKey {
pub struct AccountEndpoint {
pub creation_date: SystemTime,
pub account_url: String,
pub order_url: String,
pub orders_url: String,
pub key_hash: Vec<u8>,
pub contacts_hash: Vec<u8>,
pub external_account_hash: Vec<u8>,
Expand All @@ -78,7 +76,7 @@ impl AccountEndpoint {
AccountEndpoint {
creation_date: SystemTime::UNIX_EPOCH,
account_url: String::new(),
order_url: String::new(),
orders_url: String::new(),
key_hash: Vec::new(),
contacts_hash: Vec::new(),
external_account_hash: Vec::new(),
Expand Down Expand Up @@ -233,15 +231,20 @@ impl Account {
if key_changed {
update_account_key(endpoint, root_certs, self)?;
}
if !contacts_changed && !key_changed {
check_account_exists(endpoint, root_certs, self)?;
}
} else {
register_account(endpoint, root_certs, self)?;
}
Ok(())
}

pub fn register(
&mut self,
endpoint: &mut Endpoint,
root_certs: &[String],
) -> Result<(), Error> {
register_account(endpoint, root_certs, self)
}

pub fn save(&self) -> Result<(), Error> {
storage::save(&self.file_manager, self)
}
Expand All @@ -252,9 +255,9 @@ impl Account {
Ok(())
}

pub fn set_order_url(&mut self, endpoint_name: &str, order_url: &str) -> Result<(), Error> {
pub fn set_orders_url(&mut self, endpoint_name: &str, orders_url: &str) -> Result<(), Error> {
let mut ep = self.get_endpoint_mut(endpoint_name)?;
ep.order_url = order_url.to_string();
ep.orders_url = orders_url.to_string();
Ok(())
}

Expand Down
6 changes: 3 additions & 3 deletions acmed/src/account/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl AccountKeyStorage {
struct AccountEndpointStorage {
creation_date: SystemTime,
account_url: String,
order_url: String,
orders_url: String,
key_hash: Vec<u8>,
contacts_hash: Vec<u8>,
external_account_hash: Vec<u8>,
Expand All @@ -72,7 +72,7 @@ impl AccountEndpointStorage {
AccountEndpointStorage {
creation_date: account_endpoint.creation_date,
account_url: account_endpoint.account_url.clone(),
order_url: account_endpoint.order_url.clone(),
orders_url: account_endpoint.orders_url.clone(),
key_hash: account_endpoint.key_hash.clone(),
contacts_hash: account_endpoint.contacts_hash.clone(),
external_account_hash: account_endpoint.external_account_hash.clone(),
Expand All @@ -83,7 +83,7 @@ impl AccountEndpointStorage {
AccountEndpoint {
creation_date: self.creation_date,
account_url: self.account_url.clone(),
order_url: self.order_url.clone(),
orders_url: self.orders_url.clone(),
key_hash: self.key_hash.clone(),
contacts_hash: self.contacts_hash.clone(),
external_account_hash: self.external_account_hash.clone(),
Expand Down
32 changes: 23 additions & 9 deletions acmed/src/acme_proto.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::account::Account;
use crate::acme_proto::structs::{
ApiError, Authorization, AuthorizationStatus, NewOrder, Order, OrderStatus,
AcmeError, ApiError, Authorization, AuthorizationStatus, NewOrder, Order, OrderStatus,
};
use crate::certificate::Certificate;
use crate::endpoint::Endpoint;
Expand Down Expand Up @@ -98,14 +98,28 @@ pub fn request_certificate(
account.synchronize(endpoint, root_certs)?;

// Create a new order
let new_order = NewOrder::new(&cert.identifiers);
let new_order = serde_json::to_string(&new_order)?;
let data_builder = set_data_builder!(account, endpoint_name, new_order.as_bytes());
let (order, order_url) =
http::new_order(endpoint, root_certs, &data_builder).map_err(HttpError::in_err)?;
if let Some(e) = order.get_error() {
cert.warn(&e.prefix("Error").message);
}
let mut new_reg = false;
let (order, order_url) = loop {
let new_order = NewOrder::new(&cert.identifiers);
let new_order = serde_json::to_string(&new_order)?;
let data_builder = set_data_builder!(account, endpoint_name, new_order.as_bytes());
match http::new_order(endpoint, root_certs, &data_builder) {
Ok((order, order_url)) => {
if let Some(e) = order.get_error() {
cert.warn(&e.prefix("Error").message);
}
break (order, order_url);
}
Err(e) => {
if !new_reg && e.is_acme_err(AcmeError::AccountDoesNotExist) {
account.register(endpoint, root_certs)?;
new_reg = true;
} else {
return Err(HttpError::in_err(e));
}
}
};
};

// Begin iter over authorizations
for auth_url in order.authorizations.iter() {
Expand Down
38 changes: 14 additions & 24 deletions acmed/src/acme_proto/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::endpoint::Endpoint;
use crate::http::HttpError;
use crate::jws::{encode_jwk, encode_jwk_no_nonce, encode_kid};
use crate::logs::HasLogger;
use crate::{set_data_builder, set_empty_data_builder};
use crate::set_data_builder;
use acme_common::error::Error;

macro_rules! create_account_if_does_not_exist {
Expand Down Expand Up @@ -49,12 +49,19 @@ pub fn register_account(
let (acc_rep, account_url) =
http::new_account(endpoint, root_certs, &data_builder).map_err(HttpError::in_err)?;
account.set_account_url(&endpoint.name, &account_url)?;
let msg = format!(
"endpoint \"{}\": account \"{}\": the server has not provided an order URL upon account creation",
&endpoint.name, &account.name
);
let order_url = acc_rep.orders.ok_or_else(|| Error::from(&msg))?;
account.set_order_url(&endpoint.name, &order_url)?;
let orders_url = match acc_rep.orders {
Some(url) => url,
None => {
let msg = format!(
"endpoint \"{}\": account \"{}\": the server has not provided an order URL upon account creation",
&endpoint.name,
&account.name
);
account.warn(&msg);
String::new()
}
};
account.set_orders_url(&endpoint.name, &orders_url)?;
account.update_key_hash(&endpoint.name)?;
account.update_contacts_hash(&endpoint.name)?;
account.update_external_account_hash(&endpoint.name)?;
Expand Down Expand Up @@ -143,20 +150,3 @@ pub fn update_account_key(
));
Ok(())
}

pub fn check_account_exists(
endpoint: &mut Endpoint,
root_certs: &[String],
account: &mut BaseAccount,
) -> Result<(), Error> {
let endpoint_name = endpoint.name.clone();
let url = account.get_endpoint(&endpoint_name)?.order_url.clone();
let data_builder = set_empty_data_builder!(account, endpoint_name);
create_account_if_does_not_exist!(
http::post_jose_no_response(endpoint, root_certs, &data_builder, &url),
endpoint,
root_certs,
account
)?;
Ok(())
}
9 changes: 8 additions & 1 deletion acmed/src/http.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::acme_proto::structs::HttpApiError;
use crate::acme_proto::structs::{AcmeError, HttpApiError};
use crate::endpoint::Endpoint;
use acme_common::crypto::X509Certificate;
use acme_common::error::Error;
Expand Down Expand Up @@ -58,6 +58,13 @@ impl HttpError {
HttpError::GenericError(e) => e,
}
}

pub fn is_acme_err(&self, acme_error: AcmeError) -> bool {
match self {
HttpError::ApiError(aerr) => aerr.get_acme_type() == acme_error,
HttpError::GenericError(_) => false,
}
}
}

impl From<Error> for HttpError {
Expand Down

0 comments on commit 0db5e68

Please sign in to comment.