Skip to content

Commit

Permalink
WFE: Normalize SANs in NewOrder request (#7554)
Browse files Browse the repository at this point in the history
In #7530, `wfe.NewOrder` [began constructing a rate limit
transaction](https://github.com/letsencrypt/boulder/pull/7530/files#diff-3f950e720c205ce9fa8dea12c6fd7fd44272c2671f19d0e06962abfbea00d491R2340-R2344)
with a precondition that all names must be lower-cased, however the
actual implementation of the precondition was accidentally overlooked.
This fix corrects that and adds a unit test to prevent a future
regression.

Other changes:
- Only normalized names count towards max names limit
- Only normalized names will be logged in the web.RequestEvent

---------

Co-authored-by: Samantha Frank <[email protected]>
  • Loading branch information
pgporada and beautifulentropy committed Jun 20, 2024
1 parent 1ece848 commit 6d1f439
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 4 deletions.
1 change: 1 addition & 0 deletions wfe2/wfe.go
Original file line number Diff line number Diff line change
Expand Up @@ -2337,6 +2337,7 @@ func (wfe *WebFrontEndImpl) NewOrder(
names[i] = ident.Value
}

names = core.UniqueLowerNames(names)
err = policy.WellFormedDomainNames(names)
if err != nil {
wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Invalid identifiers requested"), nil)
Expand Down
33 changes: 29 additions & 4 deletions wfe2/wfe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2467,7 +2467,7 @@ func TestNewOrder(t *testing.T) {
nonDNSIdentifierBody := `
{
"Identifiers": [
{"type": "dns", "value": "not-example.com"},
{"type": "dns", "value": "not-example.com"},
{"type": "dns", "value": "www.not-example.com"},
{"type": "fakeID", "value": "www.i-am-21.com"}
]
Expand All @@ -2477,11 +2477,19 @@ func TestNewOrder(t *testing.T) {
validOrderBody := `
{
"Identifiers": [
{"type": "dns", "value": "not-example.com"},
{"type": "dns", "value": "not-example.com"},
{"type": "dns", "value": "www.not-example.com"}
]
}`

validOrderBodyWithMixedCaseIdentifiers := `
{
"Identifiers": [
{"type": "dns", "value": "Not-Example.com"},
{"type": "dns", "value": "WWW.Not-example.com"}
]
}`

// Body with a SAN that is longer than 64 bytes. This one is 65 bytes.
tooLongCNBody := `
{
Expand Down Expand Up @@ -2583,8 +2591,8 @@ func TestNewOrder(t *testing.T) {
"status": "pending",
"expires": "2021-02-01T01:01:01Z",
"identifiers": [
{ "type": "dns", "value": "thisreallylongexampledomainisabytelongerthanthemaxcnbytelimit.com"},
{ "type": "dns", "value": "not-example.com"}
{ "type": "dns", "value": "not-example.com"},
{ "type": "dns", "value": "thisreallylongexampledomainisabytelongerthanthemaxcnbytelimit.com"}
],
"authorizations": [
"http://localhost/acme/authz-v3/1"
Expand All @@ -2609,6 +2617,23 @@ func TestNewOrder(t *testing.T) {
"finalize": "http://localhost/acme/finalize/1/1"
}`,
},
{
Name: "POST, good payload, but when the input had mixed case",
Request: signAndPost(signer, targetPath, signedURL, validOrderBodyWithMixedCaseIdentifiers),
ExpectedBody: `
{
"status": "pending",
"expires": "2021-02-01T01:01:01Z",
"identifiers": [
{ "type": "dns", "value": "not-example.com"},
{ "type": "dns", "value": "www.not-example.com"}
],
"authorizations": [
"http://localhost/acme/authz-v3/1"
],
"finalize": "http://localhost/acme/finalize/1/1"
}`,
},
}

for _, tc := range testCases {
Expand Down

0 comments on commit 6d1f439

Please sign in to comment.