From 8249f73fa2fef71542a5019e089bbc938cd6c0f8 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Sun, 28 Sep 2025 21:15:41 +0200 Subject: [PATCH] fix: deduplicate order identifiers (#2656) --- acme/api/identifier.go | 22 ++++++++++++++++++++++ acme/api/order.go | 31 ++++++++++++------------------- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/acme/api/identifier.go b/acme/api/identifier.go index 27337ccb..42a8fd39 100644 --- a/acme/api/identifier.go +++ b/acme/api/identifier.go @@ -2,11 +2,33 @@ package api import ( "cmp" + "maps" + "net" "slices" "github.com/go-acme/lego/v4/acme" ) +func createIdentifiers(domains []string) []acme.Identifier { + uniqIdentifiers := make(map[string]acme.Identifier) + + for _, domain := range domains { + if _, ok := uniqIdentifiers[domain]; ok { + continue + } + + ident := acme.Identifier{Value: domain, Type: "dns"} + + if net.ParseIP(domain) != nil { + ident.Type = "ip" + } + + uniqIdentifiers[domain] = ident + } + + return slices.AppendSeq(make([]acme.Identifier, 0, len(uniqIdentifiers)), maps.Values(uniqIdentifiers)) +} + // compareIdentifiers compares 2 slices of [acme.Identifier]. func compareIdentifiers(a, b []acme.Identifier) int { // Clones slices to avoid modifying original slices. diff --git a/acme/api/order.go b/acme/api/order.go index 96cd4d28..13caa0d0 100644 --- a/acme/api/order.go +++ b/acme/api/order.go @@ -4,7 +4,6 @@ import ( "encoding/base64" "errors" "fmt" - "net" "slices" "time" @@ -36,18 +35,7 @@ func (o *OrderService) New(domains []string) (acme.ExtendedOrder, error) { // NewWithOptions Creates a new order. func (o *OrderService) NewWithOptions(domains []string, opts *OrderOptions) (acme.ExtendedOrder, error) { - var identifiers []acme.Identifier - for _, domain := range domains { - ident := acme.Identifier{Value: domain, Type: "dns"} - - if net.ParseIP(domain) != nil { - ident.Type = "ip" - } - - identifiers = append(identifiers, ident) - } - - orderReq := acme.Order{Identifiers: identifiers} + orderReq := acme.Order{Identifiers: createIdentifiers(domains)} if opts != nil { if !opts.NotAfter.IsZero() { @@ -86,18 +74,23 @@ func (o *OrderService) NewWithOptions(domains []string, opts *OrderOptions) (acm } } - // The elements of the "authorizations" and "identifiers" arrays are immutable once set. - // The server MUST NOT change the contents of either array after they are created. - // If a client observes a change in the contents of either array, - // then it SHOULD consider the order invalid. - // https://www.rfc-editor.org/rfc/rfc8555#section-7.1.3 + // The server MUST return an error if it cannot fulfill the request as specified, + // and it MUST NOT issue a certificate with contents other than those requested. + // If the server requires the request to be modified in a certain way, + // it should indicate the required changes using an appropriate error type and description. + // https://www.rfc-editor.org/rfc/rfc8555#section-7.4 + // + // Some ACME servers don't return an error, + // and/or change the order identifiers in the response, + // so we need to ensure that the identifiers are the same as requested. + // Deduplication by the server is allowed. if compareIdentifiers(orderReq.Identifiers, order.Identifiers) != 0 { // Sorts identifiers to avoid error message ambiguities about the order of the identifiers. slices.SortStableFunc(orderReq.Identifiers, compareIdentifier) slices.SortStableFunc(order.Identifiers, compareIdentifier) return acme.ExtendedOrder{}, - fmt.Errorf("order identifiers have been by the ACME server (RFC8555 §7.1.3): %+v != %+v", + fmt.Errorf("order identifiers have been modified by the ACME server (RFC8555 §7.4): %+v != %+v", orderReq.Identifiers, order.Identifiers) }