letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

wfe, csr: Add IP address identifier support & integration test (#8187) 

Permit all valid identifier types in `wfe.NewOrder` and `csr.VerifyCSR`.

Permit certs with just IP address identifiers to skip
`sa.addIssuedNames`.

Check that URI SANs are empty in `csr.VerifyCSR`, which was previously
missed.

Use a real (Let's Encrypt) IP address range in integration testing, to
let challtestsrv satisfy IP address challenges.

Fixes #8192
Depends on #8154
This commit is contained in:
James Renken 2025-05-27 13:17:47 -07:00 committed by GitHub
parent 8a7c3193a9
commit 103ffb03d0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 170 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
ca/ca.go
View File

@ -554,7 +554,7 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
Serial: serialBigInt.Bytes(), Serial: serialBigInt.Bytes(),
DNSNames: dnsNames, DNSNames: dnsNames,
IPAddresses: ipAddresses, IPAddresses: ipAddresses,
CommonName: csrlib.NamesFromCSR(csr).CN, CommonName: csrlib.CNFromCSR(csr),
IncludeCTPoison: true, IncludeCTPoison: true,
NotBefore: notBefore, NotBefore: notBefore,
NotAfter: notAfter, NotAfter: notAfter,

68
csr/csr.go
View File

@ -34,13 +34,13 @@ var (
unsupportedSigAlg = berrors.BadCSRError("signature algorithm not supported") unsupportedSigAlg = berrors.BadCSRError("signature algorithm not supported")
invalidSig = berrors.BadCSRError("invalid signature on CSR") invalidSig = berrors.BadCSRError("invalid signature on CSR")
invalidEmailPresent = berrors.BadCSRError("CSR contains one or more email address fields") invalidEmailPresent = berrors.BadCSRError("CSR contains one or more email address fields")
invalidIPPresent = berrors.BadCSRError("CSR contains one or more IP address fields") invalidURIPresent = berrors.BadCSRError("CSR contains one or more URI fields")
invalidNoIdent = berrors.BadCSRError("at least one identifier is required") invalidNoIdent = berrors.BadCSRError("at least one identifier is required")
) )
// VerifyCSR checks the validity of a x509.CertificateRequest. It uses // VerifyCSR checks the validity of a x509.CertificateRequest. It uses
// NamesFromCSR to normalize the DNS names before checking whether we'll issue // identifier.FromCSR to normalize the DNS names before checking whether we'll
// for them. // issue for them.
func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int, keyPolicy *goodkey.KeyPolicy, pa core.PolicyAuthority) error { func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int, keyPolicy *goodkey.KeyPolicy, pa core.PolicyAuthority) error {
key, ok := csr.PublicKey.(crypto.PublicKey) key, ok := csr.PublicKey.(crypto.PublicKey)
if !ok { if !ok {
@ -64,71 +64,47 @@ func VerifyCSR(ctx context.Context, csr *x509.CertificateRequest, maxNames int,
if len(csr.EmailAddresses) > 0 { if len(csr.EmailAddresses) > 0 {
return invalidEmailPresent return invalidEmailPresent
} }
if len(csr.IPAddresses) > 0 { if len(csr.URIs) > 0 {
return invalidIPPresent return invalidURIPresent
} }
// NamesFromCSR also performs normalization, returning values that may not // FromCSR also performs normalization, returning values that may not match
// match the literal CSR contents. // the literal CSR contents.
names := NamesFromCSR(csr) idents := identifier.FromCSR(csr)
if len(idents) == 0 {
if len(names.SANs) == 0 && names.CN == "" {
return invalidNoIdent return invalidNoIdent
} }
if len(names.CN) > maxCNLength { if len(idents) > maxNames {
return berrors.BadCSRError("CN was longer than %d bytes", maxCNLength) return berrors.BadCSRError("CSR contains more than %d identifiers", maxNames)
}
if len(names.SANs) > maxNames {
return berrors.BadCSRError("CSR contains more than %d DNS names", maxNames)
} }
err = pa.WillingToIssue(identifier.NewDNSSlice(names.SANs)) err = pa.WillingToIssue(idents)
if err != nil { if err != nil {
return err return err
} }
return nil return nil
} }
type names struct { // CNFromCSR returns the lower-cased Subject Common Name from the CSR, if a
SANs []string // short enough CN was provided. If it was too long, there will be no CN. If
CN string // none was provided, the CN will be the first SAN that is short enough, which
} // is done only for backwards compatibility with prior Let's Encrypt behaviour.
func CNFromCSR(csr *x509.CertificateRequest) string {
// NamesFromCSR deduplicates and lower-cases the Subject Common Name and Subject
// Alternative Names from the CSR. If a CN was provided, it will be used if it
// is short enough, otherwise there will be no CN. If no CN was provided, the CN
// will be the first SAN that is short enough, which is done only for backwards
// compatibility with prior Let's Encrypt behaviour. The resulting SANs will
// always include the original CN, if any.
//
// TODO(#7311): For callers that don't care about CNs, use identifier.FromCSR.
// For the rest, either revise the names struct to hold identifiers instead of
// strings, or add an ipSANs field (and rename SANs to dnsSANs).
func NamesFromCSR(csr *x509.CertificateRequest) names {
// Produce a new "sans" slice with the same memory address as csr.DNSNames
// but force a new allocation if an append happens so that we don't
// accidentally mutate the underlying csr.DNSNames array.
sans := csr.DNSNames[0:len(csr.DNSNames):len(csr.DNSNames)]
if csr.Subject.CommonName != "" {
sans = append(sans, csr.Subject.CommonName)
}
if len(csr.Subject.CommonName) > maxCNLength { if len(csr.Subject.CommonName) > maxCNLength {
return names{SANs: core.UniqueLowerNames(sans)} return ""
} }
if csr.Subject.CommonName != "" { if csr.Subject.CommonName != "" {
return names{SANs: core.UniqueLowerNames(sans), CN: strings.ToLower(csr.Subject.CommonName)} return strings.ToLower(csr.Subject.CommonName)
} }
// If there's no CN already, but we want to set one, promote the first SAN // If there's no CN already, but we want to set one, promote the first SAN
// which is shorter than the maximum acceptable CN length (if any). // which is shorter than the maximum acceptable CN length (if any).
for _, name := range sans { for _, name := range csr.DNSNames {
if len(name) <= maxCNLength { if len(name) <= maxCNLength {
return names{SANs: core.UniqueLowerNames(sans), CN: strings.ToLower(name)} return strings.ToLower(name)
} }
} }
return names{SANs: core.UniqueLowerNames(sans)} return ""
} }

38
csr/csr_test.go
View File

@ -9,6 +9,7 @@ import (
"encoding/asn1" "encoding/asn1"
"errors" "errors"
"net" "net"
"net/url"
"strings" "strings"
"testing" "testing"
@ -68,6 +69,10 @@ func TestVerifyCSR(t *testing.T) {
signedReqWithIPAddress := new(x509.CertificateRequest) signedReqWithIPAddress := new(x509.CertificateRequest)
*signedReqWithIPAddress = *signedReq *signedReqWithIPAddress = *signedReq
signedReqWithIPAddress.IPAddresses = []net.IP{net.IPv4(1, 2, 3, 4)} signedReqWithIPAddress.IPAddresses = []net.IP{net.IPv4(1, 2, 3, 4)}
signedReqWithURI := new(x509.CertificateRequest)
*signedReqWithURI = *signedReq
testURI, _ := url.ParseRequestURI("https://example.com/")
signedReqWithURI.URIs = []*url.URL{testURI}
signedReqWithAllLongSANs := new(x509.CertificateRequest) signedReqWithAllLongSANs := new(x509.CertificateRequest)
*signedReqWithAllLongSANs = *signedReq *signedReqWithAllLongSANs = *signedReq
signedReqWithAllLongSANs.DNSNames = []string{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com"} signedReqWithAllLongSANs.DNSNames = []string{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com"}
@ -115,7 +120,7 @@ func TestVerifyCSR(t *testing.T) {
signedReqWithHosts, signedReqWithHosts,
1, 1,
&mockPA{}, &mockPA{},
berrors.BadCSRError("CSR contains more than 1 DNS names"), berrors.BadCSRError("CSR contains more than 1 identifiers"),
}, },
{ {
signedReqWithBadNames, signedReqWithBadNames,
@ -133,7 +138,13 @@ func TestVerifyCSR(t *testing.T) {
signedReqWithIPAddress, signedReqWithIPAddress,
100, 100,
&mockPA{}, &mockPA{},
invalidIPPresent, nil,
},
{
signedReqWithURI,
100,
&mockPA{},
invalidURIPresent,
}, },
{ {
signedReqWithAllLongSANs, signedReqWithAllLongSANs,
@ -149,44 +160,38 @@ func TestVerifyCSR(t *testing.T) {
} }
} }
func TestNamesFromCSR(t *testing.T) { func TestCNFromCSR(t *testing.T) {
tooLongString := strings.Repeat("a", maxCNLength+1) tooLongString := strings.Repeat("a", maxCNLength+1)
cases := []struct { cases := []struct {
name string name string
csr *x509.CertificateRequest csr *x509.CertificateRequest
expectedCN string expectedCN string
expectedNames []string
}{ }{
{ {
"no explicit CN", "no explicit CN",
&x509.CertificateRequest{DNSNames: []string{"a.com"}}, &x509.CertificateRequest{DNSNames: []string{"a.com"}},
"a.com", "a.com",
[]string{"a.com"},
}, },
{ {
"explicit uppercase CN", "explicit uppercase CN",
&x509.CertificateRequest{Subject: pkix.Name{CommonName: "A.com"}, DNSNames: []string{"a.com"}}, &x509.CertificateRequest{Subject: pkix.Name{CommonName: "A.com"}, DNSNames: []string{"a.com"}},
"a.com", "a.com",
[]string{"a.com"},
}, },
{ {
"no explicit CN, uppercase SAN", "no explicit CN, uppercase SAN",
&x509.CertificateRequest{DNSNames: []string{"A.com"}}, &x509.CertificateRequest{DNSNames: []string{"A.com"}},
"a.com", "a.com",
[]string{"a.com"},
}, },
{ {
"duplicate SANs", "duplicate SANs",
&x509.CertificateRequest{DNSNames: []string{"b.com", "b.com", "a.com", "a.com"}}, &x509.CertificateRequest{DNSNames: []string{"b.com", "b.com", "a.com", "a.com"}},
"b.com", "b.com",
[]string{"a.com", "b.com"},
}, },
{ {
"explicit CN not found in SANs", "explicit CN not found in SANs",
&x509.CertificateRequest{Subject: pkix.Name{CommonName: "a.com"}, DNSNames: []string{"b.com"}}, &x509.CertificateRequest{Subject: pkix.Name{CommonName: "a.com"}, DNSNames: []string{"b.com"}},
"a.com", "a.com",
[]string{"a.com", "b.com"},
}, },
{ {
"no explicit CN, all SANs too long to be the CN", "no explicit CN, all SANs too long to be the CN",
@ -195,7 +200,6 @@ func TestNamesFromCSR(t *testing.T) {
tooLongString + ".b.com", tooLongString + ".b.com",
}}, }},
"", "",
[]string{tooLongString + ".a.com", tooLongString + ".b.com"},
}, },
{ {
"no explicit CN, leading SANs too long to be the CN", "no explicit CN, leading SANs too long to be the CN",
@ -206,7 +210,6 @@ func TestNamesFromCSR(t *testing.T) {
"b.com", "b.com",
}}, }},
"a.com", "a.com",
[]string{"a.com", tooLongString + ".a.com", tooLongString + ".b.com", "b.com"},
}, },
{ {
"explicit CN, leading SANs too long to be the CN", "explicit CN, leading SANs too long to be the CN",
@ -219,7 +222,6 @@ func TestNamesFromCSR(t *testing.T) {
"b.com", "b.com",
}}, }},
"a.com", "a.com",
[]string{"a.com", tooLongString + ".a.com", tooLongString + ".b.com", "b.com"},
}, },
{ {
"explicit CN that's too long to be the CN", "explicit CN that's too long to be the CN",
@ -227,7 +229,6 @@ func TestNamesFromCSR(t *testing.T) {
Subject: pkix.Name{CommonName: tooLongString + ".a.com"}, Subject: pkix.Name{CommonName: tooLongString + ".a.com"},
}, },
"", "",
[]string{tooLongString + ".a.com"},
}, },
{ {
"explicit CN that's too long to be the CN, with a SAN", "explicit CN that's too long to be the CN, with a SAN",
@ -237,14 +238,11 @@ func TestNamesFromCSR(t *testing.T) {
"b.com", "b.com",
}}, }},
"", "",
[]string{tooLongString + ".a.com", "b.com"},
}, },
} }
for _, tc := range cases { for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
names := NamesFromCSR(tc.csr) test.AssertEquals(t, CNFromCSR(tc.csr), tc.expectedCN)
test.AssertEquals(t, names.CN, tc.expectedCN)
test.AssertDeepEquals(t, names.SANs, tc.expectedNames)
}) })
} }
} }

4
db/multi.go
View File

@ -85,6 +85,10 @@ func (mi *MultiInserter) query() (string, []interface{}) {
// Insert inserts all the collected rows into the database represented by // Insert inserts all the collected rows into the database represented by
// `queryer`. // `queryer`.
func (mi *MultiInserter) Insert(ctx context.Context, db Execer) error { func (mi *MultiInserter) Insert(ctx context.Context, db Execer) error {
if len(mi.values) == 0 {
return nil
}
query, queryArgs := mi.query() query, queryArgs := mi.query()
res, err := db.ExecContext(ctx, query, queryArgs...) res, err := db.ExecContext(ctx, query, queryArgs...)
if err != nil { if err != nil {

2
docker-compose.next.yml
View File

@ -1,7 +1,7 @@
services: services:
boulder: boulder:
environment: environment:
FAKE_DNS: 10.77.77.77 FAKE_DNS: 64.112.117.122
BOULDER_CONFIG_DIR: test/config-next BOULDER_CONFIG_DIR: test/config-next
GOFLAGS: -mod=vendor GOFLAGS: -mod=vendor
GOCACHE: /boulder/.gocache/go-build-next GOCACHE: /boulder/.gocache/go-build-next

11
docker-compose.yml
View File

@ -13,7 +13,7 @@ services:
# To solve HTTP-01 and TLS-ALPN-01 challenges, change the IP in FAKE_DNS # To solve HTTP-01 and TLS-ALPN-01 challenges, change the IP in FAKE_DNS
# to the IP address where your ACME client's solver is listening. # to the IP address where your ACME client's solver is listening.
# FAKE_DNS: 172.17.0.1 # FAKE_DNS: 172.17.0.1
FAKE_DNS: 10.77.77.77 FAKE_DNS: 64.112.117.122
BOULDER_CONFIG_DIR: test/config BOULDER_CONFIG_DIR: test/config
GOCACHE: /boulder/.gocache/go-build GOCACHE: /boulder/.gocache/go-build
GOFLAGS: -mod=vendor GOFLAGS: -mod=vendor
@ -30,6 +30,8 @@ services:
ipv4_address: 10.33.33.33 ipv4_address: 10.33.33.33
consulnet: consulnet:
ipv4_address: 10.55.55.55 ipv4_address: 10.55.55.55
publicnet:
ipv4_address: 64.112.117.122
# Use consul as a backup to Docker's embedded DNS server. If there's a name # Use consul as a backup to Docker's embedded DNS server. If there's a name
# Docker's DNS server doesn't know about, it will forward the query to this # Docker's DNS server doesn't know about, it will forward the query to this
# IP (running consul). # IP (running consul).
@ -209,3 +211,10 @@ networks:
driver: default driver: default
config: config:
- subnet: 10.55.55.0/24 - subnet: 10.55.55.0/24
publicnet:
driver: bridge
ipam:
driver: default
config:
- subnet: 64.112.117.0/24

6
sa/model.go
View File

@ -1056,8 +1056,10 @@ func deleteOrderFQDNSet(
} }
func addIssuedNames(ctx context.Context, queryer db.Execer, cert *x509.Certificate, isRenewal bool) error { func addIssuedNames(ctx context.Context, queryer db.Execer, cert *x509.Certificate, isRenewal bool) error {
if len(cert.DNSNames) == 0 { // TODO(#7311): Determine & explicitly document whether to place IP address
return berrors.InternalServerError("certificate has no DNSNames") // identifiers in issuedNames. We currently skip them.
if len(cert.DNSNames) == 0 && len(cert.IPAddresses) == 0 {
return berrors.InternalServerError("certificate has no DNSNames or IPAddresses")
} }
multiInserter, err := db.NewMultiInserter("issuedNames", []string{"reversedName", "serial", "notBefore", "renewal"}) multiInserter, err := db.NewMultiInserter("issuedNames", []string{"reversedName", "serial", "notBefore", "renewal"})

15
test/integration/common_test.go
View File

@ -69,15 +69,11 @@ func makeClientAndOrder(c *client, csrKey *ecdsa.PrivateKey, idents []acme.Ident
} }
} }
var ids []acme.Identifier
for _, ident := range idents {
ids = append(ids, acme.Identifier{Type: string(ident.Type), Value: ident.Value})
}
var order acme.Order var order acme.Order
if certToReplace != nil { if certToReplace != nil {
order, err = c.Client.ReplacementOrderExtension(c.Account, certToReplace, ids, acme.OrderExtension{Profile: profile}) order, err = c.Client.ReplacementOrderExtension(c.Account, certToReplace, idents, acme.OrderExtension{Profile: profile})
} else { } else {
order, err = c.Client.NewOrderExtension(c.Account, ids, acme.OrderExtension{Profile: profile}) order, err = c.Client.NewOrderExtension(c.Account, idents, acme.OrderExtension{Profile: profile})
} }
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
@ -100,10 +96,7 @@ func makeClientAndOrder(c *client, csrKey *ecdsa.PrivateKey, idents []acme.Ident
} }
chal, err = c.Client.UpdateChallenge(c.Account, chal) chal, err = c.Client.UpdateChallenge(c.Account, chal)
if err != nil { if err != nil {
_, err = testSrvClient.RemoveHTTP01Response(chal.Token) testSrvClient.RemoveHTTP01Response(chal.Token)
if err != nil {
return nil, nil, err
}
return nil, nil, err return nil, nil, err
} }
_, err = testSrvClient.RemoveHTTP01Response(chal.Token) _, err = testSrvClient.RemoveHTTP01Response(chal.Token)
@ -194,7 +187,7 @@ func makeCSR(k *ecdsa.PrivateKey, idents []acme.Identifier, cn bool) (*x509.Cert
DNSNames: names, DNSNames: names,
IPAddresses: ips, IPAddresses: ips,
} }
if cn { if cn && len(names) > 0 {
tmpl.Subject = pkix.Name{CommonName: names[0]} tmpl.Subject = pkix.Name{CommonName: names[0]}
} }

64
test/integration/issuance_test.go
View File

@ -8,6 +8,8 @@ import (
"crypto/rand" "crypto/rand"
"crypto/x509" "crypto/x509"
"fmt" "fmt"
"os"
"strings"
"testing" "testing"
"github.com/eggsampler/acme/v3" "github.com/eggsampler/acme/v3"
@ -167,3 +169,65 @@ func TestIssuanceProfiles(t *testing.T) {
test.AssertEquals(t, len(legacy.SubjectKeyId), 20) test.AssertEquals(t, len(legacy.SubjectKeyId), 20)
test.AssertEquals(t, len(modern.SubjectKeyId), 0) test.AssertEquals(t, len(modern.SubjectKeyId), 0)
} }
// TestIPShortLived verifies that we will allow IP address identifiers only in
// orders that use the shortlived profile.
func TestIPShortLived(t *testing.T) {
t.Parallel()
// Create an account.
client, err := makeClient("mailto:example@letsencrypt.org")
if err != nil {
t.Fatalf("creating acme client: %s", err)
}
// Create a private key.
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
if err != nil {
t.Fatalf("creating random cert key: %s", err)
}
// Create an IP address identifier to request.
ip := "64.112.117.122"
idents := []acme.Identifier{
{Type: "ip", Value: ip},
}
// Ensure we fail under each other profile that we know the test server advertises.
_, err = authAndIssue(client, key, idents, false, "legacy")
if err == nil {
t.Error("issued for IP address identifier under legacy profile")
}
if !strings.Contains(err.Error(), "Profile \"legacy\" does not permit ip type identifiers") {
t.Fatalf("issuing under legacy profile failed for the wrong reason: %s", err)
}
_, err = authAndIssue(client, key, idents, false, "modern")
if err == nil {
t.Error("issued for IP address identifier under modern profile")
}
if !strings.Contains(err.Error(), "Profile \"modern\" does not permit ip type identifiers") {
t.Fatalf("issuing under legacy profile failed for the wrong reason: %s", err)
}
// Get one cert for the shortlived profile.
res, err := authAndIssue(client, key, idents, false, "shortlived")
if os.Getenv("BOULDER_CONFIG_DIR") == "test/config-next" {
if err != nil {
t.Errorf("issuing under shortlived profile: %s", err)
}
if res.Order.Profile != "shortlived" {
t.Errorf("got '%s' profile, wanted 'shortlived'", res.Order.Profile)
}
cert := res.certs[0]
// Check that the shortlived profile worked as expected.
if cert.IPAddresses[0].String() != ip {
t.Errorf("got cert with first IP SAN '%s', wanted '%s'", cert.IPAddresses[0], ip)
}
} else {
if !strings.Contains(err.Error(), "Profile \"shortlived\" does not permit ip type identifiers") {
t.Errorf("issuing under shortlived profile failed for the wrong reason: %s", err)
}
}
}

4
test/startservers.py
View File

@ -295,8 +295,8 @@ def startChallSrv():
'--doh-cert', 'test/certs/ipki/10.77.77.77/cert.pem', '--doh-cert', 'test/certs/ipki/10.77.77.77/cert.pem',
'--doh-cert-key', 'test/certs/ipki/10.77.77.77/key.pem', '--doh-cert-key', 'test/certs/ipki/10.77.77.77/key.pem',
'--management', ':8055', '--management', ':8055',
'--http01', '10.77.77.77:80', '--http01', '64.112.117.122:80',
'-https01', '10.77.77.77:443', '-https01', '64.112.117.122:443',
'--tlsalpn01', '10.88.88.88:443'], '--tlsalpn01', '10.88.88.88:443'],
None) None)
# Wait for the chall-test-srv management port. # Wait for the chall-test-srv management port.

10
test/v2_integration.py
View File

@ -157,7 +157,7 @@ def test_http_challenge_broken_redirect():
redirect) redirect)
# Expect the specialized error message # Expect the specialized error message
expectedError = "10.77.77.77: Fetching {0}: Invalid host in redirect target \"{1}.well-known\". Check webserver config for missing '/' in redirect target.".format(redirect, d) expectedError = "64.112.117.122: Fetching {0}: Invalid host in redirect target \"{1}.well-known\". Check webserver config for missing '/' in redirect target.".format(redirect, d)
# NOTE(@cpu): Can't use chisel2.expect_problem here because it doesn't let # NOTE(@cpu): Can't use chisel2.expect_problem here because it doesn't let
# us interrogate the detail message easily. # us interrogate the detail message easily.
@ -363,7 +363,7 @@ def test_http_challenge_https_redirect():
# Also add an A record for the domain pointing to the interface that the # Also add an A record for the domain pointing to the interface that the
# HTTPS HTTP-01 challtestsrv is bound. # HTTPS HTTP-01 challtestsrv is bound.
challSrv.add_a_record(d, ["10.77.77.77"]) challSrv.add_a_record(d, ["64.112.117.122"])
try: try:
chisel2.auth_and_issue([d], client=client, chall_type="http-01") chisel2.auth_and_issue([d], client=client, chall_type="http-01")
@ -445,7 +445,7 @@ def test_http_challenge_timeout():
to a slow HTTP server appropriately. to a slow HTTP server appropriately.
""" """
# Start a simple python HTTP server on port 80 in its own thread. # Start a simple python HTTP server on port 80 in its own thread.
# NOTE(@cpu): The chall-test-srv binds 10.77.77.77:80 for HTTP-01 # NOTE(@cpu): The chall-test-srv binds 64.112.117.122:80 for HTTP-01
# challenges so we must use the 10.88.88.88 address for the throw away # challenges so we must use the 10.88.88.88 address for the throw away
# server for this test and add a mock DNS entry that directs the VA to it. # server for this test and add a mock DNS entry that directs the VA to it.
httpd = SlowHTTPServer(("10.88.88.88", 80), SlowHTTPRequestHandler) httpd = SlowHTTPServer(("10.88.88.88", 80), SlowHTTPRequestHandler)
@ -785,10 +785,10 @@ def multiva_setup(client, guestlist):
# Add an A record for the redirect target that sends it to the real chall # Add an A record for the redirect target that sends it to the real chall
# test srv for a valid HTTP-01 response. # test srv for a valid HTTP-01 response.
redirHostname = "chall-test-srv.example.com" redirHostname = "chall-test-srv.example.com"
challSrv.add_a_record(redirHostname, ["10.77.77.77"]) challSrv.add_a_record(redirHostname, ["64.112.117.122"])
# Start a simple python HTTP server on port 80 in its own thread. # Start a simple python HTTP server on port 80 in its own thread.
# NOTE(@cpu): The chall-test-srv binds 10.77.77.77:80 for HTTP-01 # NOTE(@cpu): The chall-test-srv binds 64.112.117.122:80 for HTTP-01
# challenges so we must use the 10.88.88.88 address for the throw away # challenges so we must use the 10.88.88.88 address for the throw away
# server for this test and add a mock DNS entry that directs the VA to it. # server for this test and add a mock DNS entry that directs the VA to it.
redirect = "http://{0}/.well-known/acme-challenge/{1}".format( redirect = "http://{0}/.well-known/acme-challenge/{1}".format(

23
wfe2/wfe.go
View File

@ -2022,7 +2022,8 @@ func (wfe *WebFrontEndImpl) orderToOrderJSON(request *http.Request, order *corep
// function is returned that can be used to refund the quota if the order is not // function is returned that can be used to refund the quota if the order is not
// created, the func will be nil if any error was encountered during the check. // created, the func will be nil if any error was encountered during the check.
// //
// TODO(#7311): Handle IP address identifiers. // TODO(#7311): Handle IP address identifiers properly; don't just trust that
// the value will always make sense in context.
func (wfe *WebFrontEndImpl) checkNewOrderLimits(ctx context.Context, regId int64, idents identifier.ACMEIdentifiers, isRenewal bool) (func(), error) { func (wfe *WebFrontEndImpl) checkNewOrderLimits(ctx context.Context, regId int64, idents identifier.ACMEIdentifiers, isRenewal bool) (func(), error) {
names, err := idents.ToDNSSlice() names, err := idents.ToDNSSlice()
if err != nil { if err != nil {
@ -2055,7 +2056,7 @@ func (wfe *WebFrontEndImpl) checkNewOrderLimits(ctx context.Context, regId int64
// as identified by the provided ARI CertID. This function ensures that: // as identified by the provided ARI CertID. This function ensures that:
// - the certificate being replaced exists, // - the certificate being replaced exists,
// - the requesting account owns that certificate, and // - the requesting account owns that certificate, and
// - a name in this new order matches a name in the certificate being // - an identifier in this new order matches an identifier in the certificate being
// replaced. // replaced.
func (wfe *WebFrontEndImpl) orderMatchesReplacement(ctx context.Context, acct *core.Registration, idents identifier.ACMEIdentifiers, serial string) error { func (wfe *WebFrontEndImpl) orderMatchesReplacement(ctx context.Context, acct *core.Registration, idents identifier.ACMEIdentifiers, serial string) error {
// It's okay to use GetCertificate (vs trying to get a precertificate), // It's okay to use GetCertificate (vs trying to get a precertificate),
@ -2077,18 +2078,17 @@ func (wfe *WebFrontEndImpl) orderMatchesReplacement(ctx context.Context, acct *c
return fmt.Errorf("error parsing certificate replaced by this order: %w", err) return fmt.Errorf("error parsing certificate replaced by this order: %w", err)
} }
var nameMatch bool var identMatch bool
for _, ident := range idents { for _, ident := range idents {
// TODO(#7311): Handle IP address identifiers.
if parsedCert.VerifyHostname(ident.Value) == nil { if parsedCert.VerifyHostname(ident.Value) == nil {
// At least one name in the new order matches a name in the // At least one identifier in the new order matches an identifier in
// predecessor certificate. // the predecessor certificate.
nameMatch = true identMatch = true
break break
} }
} }
if !nameMatch { if !identMatch {
return berrors.MalformedError("identifiers in this order do not match any names in the certificate being replaced") return berrors.MalformedError("identifiers in this order do not match any identifiers in the certificate being replaced")
} }
return nil return nil
} }
@ -2282,12 +2282,11 @@ func (wfe *WebFrontEndImpl) NewOrder(
return return
} }
// TODO(#7311): Handle non-DNS identifiers.
idents := newOrderRequest.Identifiers idents := newOrderRequest.Identifiers
for _, ident := range idents { for _, ident := range idents {
if ident.Type != identifier.TypeDNS { if !ident.Type.IsValid() {
wfe.sendError(response, logEvent, wfe.sendError(response, logEvent,
probs.UnsupportedIdentifier("NewOrder request included invalid non-DNS type identifier: type %q, value %q", probs.UnsupportedIdentifier("NewOrder request included unsupported identifier: type %q, value %q",
ident.Type, ident.Value), ident.Type, ident.Value),
nil) nil)
return return

34
wfe2/wfe_test.go
View File

@ -2586,7 +2586,7 @@ func TestNewOrder(t *testing.T) {
targetPath := "new-order" targetPath := "new-order"
signedURL := fmt.Sprintf("http://%s/%s", targetHost, targetPath) signedURL := fmt.Sprintf("http://%s/%s", targetHost, targetPath)
nonDNSIdentifierBody := ` invalidIdentifierBody := `
{ {
"Identifiers": [ "Identifiers": [
{"type": "dns", "value": "not-example.com"}, {"type": "dns", "value": "not-example.com"},
@ -2600,7 +2600,8 @@ func TestNewOrder(t *testing.T) {
{ {
"Identifiers": [ "Identifiers": [
{"type": "dns", "value": "not-example.com"}, {"type": "dns", "value": "not-example.com"},
{"type": "dns", "value": "www.not-example.com"} {"type": "dns", "value": "www.not-example.com"},
{"type": "ip", "value": "9.9.9.9"}
] ]
}` }`
@ -2608,7 +2609,8 @@ func TestNewOrder(t *testing.T) {
{ {
"Identifiers": [ "Identifiers": [
{"type": "dns", "value": "Not-Example.com"}, {"type": "dns", "value": "Not-Example.com"},
{"type": "dns", "value": "WWW.Not-example.com"} {"type": "dns", "value": "WWW.Not-example.com"},
{"type": "ip", "value": "9.9.9.9"}
] ]
}` }`
@ -2665,24 +2667,34 @@ func TestNewOrder(t *testing.T) {
ExpectedBody: `{"type":"` + probs.ErrorNS + `malformed","detail":"Unable to validate JWS :: Request payload did not parse as JSON","status":400}`, ExpectedBody: `{"type":"` + probs.ErrorNS + `malformed","detail":"Unable to validate JWS :: Request payload did not parse as JSON","status":400}`,
}, },
{ {
Name: "POST, empty domain name identifier", Name: "POST, empty DNS identifier",
Request: signAndPost(signer, targetPath, signedURL, `{"identifiers":[{"type":"dns","value":""}]}`), Request: signAndPost(signer, targetPath, signedURL, `{"identifiers":[{"type":"dns","value":""}]}`),
ExpectedBody: `{"type":"` + probs.ErrorNS + `malformed","detail":"NewOrder request included empty identifier","status":400}`, ExpectedBody: `{"type":"` + probs.ErrorNS + `malformed","detail":"NewOrder request included empty identifier","status":400}`,
}, },
{ {
Name: "POST, invalid domain name identifier", Name: "POST, empty IP identifier",
Request: signAndPost(signer, targetPath, signedURL, `{"identifiers":[{"type":"ip","value":""}]}`),
ExpectedBody: `{"type":"` + probs.ErrorNS + `malformed","detail":"NewOrder request included empty identifier","status":400}`,
},
{
Name: "POST, invalid DNS identifier",
Request: signAndPost(signer, targetPath, signedURL, `{"identifiers":[{"type":"dns","value":"example.invalid"}]}`), Request: signAndPost(signer, targetPath, signedURL, `{"identifiers":[{"type":"dns","value":"example.invalid"}]}`),
ExpectedBody: `{"type":"` + probs.ErrorNS + `rejectedIdentifier","detail":"Invalid identifiers requested :: Cannot issue for \"example.invalid\": Domain name does not end with a valid public suffix (TLD)","status":400}`, ExpectedBody: `{"type":"` + probs.ErrorNS + `rejectedIdentifier","detail":"Invalid identifiers requested :: Cannot issue for \"example.invalid\": Domain name does not end with a valid public suffix (TLD)","status":400}`,
}, },
{
Name: "POST, invalid IP identifier",
Request: signAndPost(signer, targetPath, signedURL, `{"identifiers":[{"type":"ip","value":"127.0.0.0.0.0.0.1"}]}`),
ExpectedBody: `{"type":"` + probs.ErrorNS + `rejectedIdentifier","detail":"Invalid identifiers requested :: Cannot issue for \"127.0.0.0.0.0.0.1\": IP address is invalid","status":400}`,
},
{ {
Name: "POST, no identifiers in payload", Name: "POST, no identifiers in payload",
Request: signAndPost(signer, targetPath, signedURL, "{}"), Request: signAndPost(signer, targetPath, signedURL, "{}"),
ExpectedBody: `{"type":"` + probs.ErrorNS + `malformed","detail":"NewOrder request did not specify any identifiers","status":400}`, ExpectedBody: `{"type":"` + probs.ErrorNS + `malformed","detail":"NewOrder request did not specify any identifiers","status":400}`,
}, },
{ {
Name: "POST, non-DNS identifier in payload", Name: "POST, invalid identifier type in payload",
Request: signAndPost(signer, targetPath, signedURL, nonDNSIdentifierBody), Request: signAndPost(signer, targetPath, signedURL, invalidIdentifierBody),
ExpectedBody: `{"type":"` + probs.ErrorNS + `unsupportedIdentifier","detail":"NewOrder request included invalid non-DNS type identifier: type \"fakeID\", value \"www.i-am-21.com\"","status":400}`, ExpectedBody: `{"type":"` + probs.ErrorNS + `unsupportedIdentifier","detail":"NewOrder request included unsupported identifier: type \"fakeID\", value \"www.i-am-21.com\"","status":400}`,
}, },
{ {
Name: "POST, notAfter and notBefore in payload", Name: "POST, notAfter and notBefore in payload",
@ -2731,7 +2743,8 @@ func TestNewOrder(t *testing.T) {
"expires": "2021-02-01T01:01:01Z", "expires": "2021-02-01T01:01:01Z",
"identifiers": [ "identifiers": [
{ "type": "dns", "value": "not-example.com"}, { "type": "dns", "value": "not-example.com"},
{ "type": "dns", "value": "www.not-example.com"} { "type": "dns", "value": "www.not-example.com"},
{ "type": "ip", "value": "9.9.9.9"}
], ],
"authorizations": [ "authorizations": [
"http://localhost/acme/authz/1/1" "http://localhost/acme/authz/1/1"
@ -2748,7 +2761,8 @@ func TestNewOrder(t *testing.T) {
"expires": "2021-02-01T01:01:01Z", "expires": "2021-02-01T01:01:01Z",
"identifiers": [ "identifiers": [
{ "type": "dns", "value": "not-example.com"}, { "type": "dns", "value": "not-example.com"},
{ "type": "dns", "value": "www.not-example.com"} { "type": "dns", "value": "www.not-example.com"},
{ "type": "ip", "value": "9.9.9.9"}
], ],
"authorizations": [ "authorizations": [
"http://localhost/acme/authz/1/1" "http://localhost/acme/authz/1/1"