From 79ababeef594ad243c4444ab292908aa9c9ba62e Mon Sep 17 00:00:00 2001 From: Phil Porada Date: Wed, 29 May 2024 17:38:12 -0400 Subject: [PATCH] Add support for testing ARI in Pebble (#24) * Support testing ARI against Pebble * Test replacing a replacement order that had previously failed * Deactivate prior authorizations to induce a validation failure * Skip ARI tests if server doesnt support ARI --- acme.go | 21 +++-- ari.go | 1 + ari_test.go | 102 +++++++++++++++++++++- order.go | 2 + utility_test.go | 227 ++++++++++++++++++++++++++++++++++++++++-------- 5 files changed, 307 insertions(+), 46 deletions(-) diff --git a/acme.go b/acme.go index 18e559e..0c02cbd 100644 --- a/acme.go +++ b/acme.go @@ -74,8 +74,9 @@ func (c Client) getPollingDurations() (time.Duration, time.Duration) { return pollInterval, pollTimeout } -// Helper function to have a central point for performing http requests. -// Stores any returned nonces in the stack. +// Helper function to have a central point for performing http requests. Stores +// any returned nonces in the stack. The caller is responsible for closing the +// body so they can read the response. func (c Client) do(req *http.Request, addNonce bool) (*http.Response, error) { // identifier for this client, as well as the default go user agent if c.userAgentSuffix != "" { @@ -100,7 +101,8 @@ func (c Client) do(req *http.Request, addNonce bool) (*http.Response, error) { return resp, nil } -// Helper function to perform an HTTP get request and read the body. +// Helper function to perform an HTTP get request and read the body. The caller +// is responsible for closing the body so they can read the response. func (c Client) getRaw(url string, expectedStatus ...int) (*http.Response, []byte, error) { req, err := http.NewRequest(http.MethodGet, url, nil) if err != nil { @@ -125,7 +127,8 @@ func (c Client) getRaw(url string, expectedStatus ...int) (*http.Response, []byt return resp, body, nil } -// Helper function for performing a http get on an acme resource. +// Helper function for performing a http get on an acme resource. The caller is +// responsible for closing the body so they can read the response. func (c Client) get(url string, out interface{}, expectedStatus ...int) (*http.Response, error) { resp, body, err := c.getRaw(url, expectedStatus...) if err != nil { @@ -165,8 +168,9 @@ func (c Client) nonce() (string, error) { return nonce, nil } -// Helper function to perform an HTTP post request and read the body. -// Will attempt to retry if error is badNonce +// Helper function to perform an HTTP post request and read the body. Will +// attempt to retry if error is badNonce. The caller is responsible for closing +// the body so they can read the response. func (c Client) postRaw(retryCount int, requestURL, kid string, privateKey crypto.Signer, payload interface{}, expectedStatus []int) (*http.Response, []byte, error) { nonce, err := c.nonce() if err != nil { @@ -215,7 +219,8 @@ func (c Client) postRaw(retryCount int, requestURL, kid string, privateKey crypt return resp, body, nil } -// Helper function for performing a http post to an acme resource. +// Helper function for performing a http post to an acme resource. The caller is +// responsible for closing the body so they can read the response. func (c Client) post(requestURL, keyID string, privateKey crypto.Signer, payload interface{}, out interface{}, expectedStatus ...int) (*http.Response, error) { resp, body, err := c.postRaw(0, requestURL, keyID, privateKey, payload, expectedStatus) if err != nil { @@ -240,7 +245,7 @@ func (c Client) post(requestURL, keyID string, privateKey crypto.Signer, payload var regLink = regexp.MustCompile(`<(.+?)>;\s*rel="(.+?)"`) -// Fetches a http Link header from a http response +// Fetches a http Link header from an http response and closes the body. func fetchLink(resp *http.Response, wantedLink string) string { if resp == nil { return "" diff --git a/ari.go b/ari.go index 56bffcd..aad35e5 100644 --- a/ari.go +++ b/ari.go @@ -39,6 +39,7 @@ func (c Client) GetRenewalInfo(cert *x509.Certificate) (RenewalInfo, error) { if err != nil { return ri, err } + defer resp.Body.Close() ri.RetryAfter, err = parseRetryAfter(resp.Header.Get("Retry-After")) return ri, err diff --git a/ari_test.go b/ari_test.go index 2e24bdc..e25e7d9 100644 --- a/ari_test.go +++ b/ari_test.go @@ -8,8 +8,8 @@ import ( ) func TestClient_GetRenewalInfo(t *testing.T) { - if testClientMeta.Software == clientPebble { - t.Skip("pebble doesnt support ari") + if testClient.dir.RenewalInfo == "" { + t.Skip("acme server does not support ari renewals") return } @@ -18,13 +18,16 @@ func TestClient_GetRenewalInfo(t *testing.T) { t.Fatalf("no certificate: %+v", order) } certs, err := testClient.FetchCertificates(account, order.Certificate) + t.Logf("Issued serial %s\n", certs[0].SerialNumber.String()) if err != nil { - t.Fatalf("expeceted no error, got: %v", err) + t.Fatalf("expected no error, got: %v", err) } if len(certs) < 2 { t.Fatalf("no certs") } + renewalInfo, err := testClient.GetRenewalInfo(certs[0]) + t.Logf("Suggested renewal window for new issuance: %v\n", renewalInfo.SuggestedWindow) if err != nil { t.Fatalf("expected no error, got: %v", err) } @@ -40,6 +43,99 @@ func TestClient_GetRenewalInfo(t *testing.T) { if renewalInfo.SuggestedWindow.End.Before(renewalInfo.SuggestedWindow.Start) { t.Fatalf("suggested window end is before start?") } + + err = testClient.RevokeCertificate(account, certs[0], account.PrivateKey, ReasonUnspecified) + if err != nil { + t.Fatalf("failed to revoke certificate: %v", err) + } + + // The renewal window should adjust to allow immediate renewal + renewalInfo, err = testClient.GetRenewalInfo(certs[0]) + t.Logf("Suggested renewal window for revoked certificate: %v\n", renewalInfo.SuggestedWindow) + if err != nil { + t.Fatalf("expected no error, got: %v", err) + } + if !renewalInfo.SuggestedWindow.Start.Before(time.Now()) { + t.Fatalf("suggested window start is in the past?") + } + if !renewalInfo.SuggestedWindow.End.Before(time.Now()) { + t.Fatalf("suggested window start is in the past?") + } + if renewalInfo.SuggestedWindow.End.Before(renewalInfo.SuggestedWindow.Start) { + t.Fatalf("suggested window end is before start?") + } +} + +func TestClient_IssueReplacementCert(t *testing.T) { + if testClient.dir.RenewalInfo == "" { + t.Skip("acme server does not support ari renewals") + return + } + + t.Log("Issuing initial order") + account, order, _ := makeOrderFinalised(t, nil) + if order.Certificate == "" { + t.Fatalf("no certificate: %+v", order) + } + + // Replacing the original order should work + t.Log("Issuing first replacement order") + replacementOrder, err := makeReplacementOrderFinalized(t, order, account, nil, false) + if err != nil { + t.Fatal(err) + } + + // Replacing the replacement should work + t.Log("Issuing second replacement order") + _, err = makeReplacementOrderFinalized(t, replacementOrder, account, nil, false) + if err != nil { + t.Fatal(err) + } + + // Attempting to replace a previously replacement order should fail. + t.Log("Should not be able to create a duplicate replacement") + _, err = makeReplacementOrderFinalized(t, replacementOrder, account, nil, false) + if err == nil { + t.Fatal(err) + } +} + +func TestClient_FailedReplacementOrderAllowsAnotherReplacement(t *testing.T) { + if testClient.dir.RenewalInfo == "" { + t.Skip("acme server does not support ari renewals") + return + } + + t.Log("Issuing initial order") + account, order, _ := makeOrderFinalised(t, nil) + if order.Certificate == "" { + t.Fatalf("no certificate: %+v", order) + } + + // Explicitly deactivate the previous authorization so the VA has to + // re-validate the order and encounter a failure. Upon receiving a + // validation failure, Pebble marks an order as invalid which is what we + // need. + auth, err := testClient.DeactivateAuthorization(account, order.Authorizations[0]) + if err != nil { + t.Fatalf("expected no error, got: %v", err) + } + if auth.Status != "deactivated" { + t.Fatalf("expected deactivated status, got: %s", auth.Status) + } + + t.Log("Issuing replacement order which will intentionally fail") + _, err = makeReplacementOrderFinalized(t, order, account, nil, true) + if err == nil { + t.Fatal(err) + } + + // Attempting to replace a previously failed replacement order should pass + t.Log("Issuing replacement order for a parent order who previously had a failed replacement order") + _, err = makeReplacementOrderFinalized(t, order, account, nil, false) + if err != nil { + t.Fatal(err) + } } func Test_generateCertID(t *testing.T) { diff --git a/order.go b/order.go index d298bd8..d5a476f 100644 --- a/order.go +++ b/order.go @@ -65,6 +65,8 @@ func (c Client) ReplacementOrder(account Account, oldCert *x509.Certificate, ide if err != nil { return newOrderResp, err } + defer resp.Body.Close() + newOrderResp.URL = resp.Header.Get("Location") return newOrderResp, nil } diff --git a/utility_test.go b/utility_test.go index e5043e8..71cd740 100644 --- a/utility_test.go +++ b/utility_test.go @@ -151,21 +151,121 @@ func makeOrderFinalised(t *testing.T, supportedChalTypes []string, identifiers . acct, order := makeOrder(t, identifiers...) - for _, authURL := range order.Authorizations { + err := validateChallenges(t, order, acct, supportedChalTypes, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + updatedOrder, err := testClient.FetchOrder(acct, order.URL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if updatedOrder.Status != "ready" { + t.Fatal("order not ready") + } + + var domains []string + for _, id := range order.Identifiers { + domains = append(domains, id.Value) + } + csr, privKey := makeCSR(t, domains) + + finalizedOrder, err := testClient.FinalizeOrder(acct, order, csr) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if finalizedOrder.Status != "valid" { + t.Fatal("order not valid") + } + + return acct, finalizedOrder, privKey +} + +// makeReplacementOrderFinalized is a helper that fetches a certificate from the +// given order, creates a replacement order for the given order, and finalizes +// it. It differs from makeOrder and makeOrderFinalized in that it does not +// create a new Account object each time it's called. +func makeReplacementOrderFinalized(t *testing.T, order Order, account Account, supportedChalTypes []string, challengesShouldFail bool) (Order, error) { + certs, err := testClient.FetchCertificates(account, order.Certificate) + if err != nil { + return Order{}, fmt.Errorf("expected no error, got: %v", err) + } + if len(certs) < 2 { + return Order{}, fmt.Errorf("no certs") + } + + replacementOrder, err := testClient.ReplacementOrder(account, certs[0], order.Identifiers) + if err != nil { + return Order{}, fmt.Errorf("expected no error, got: %v", err) + } + + err = validateChallenges(t, replacementOrder, account, supportedChalTypes, challengesShouldFail) + if err != nil { + return Order{}, err + } + + updatedOrder, err := testClient.FetchOrder(account, replacementOrder.URL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // We can ignore updatedOrder after this. + if updatedOrder.Status != "ready" { + t.Fatal("order not ready") + } + + // Check that the replacement order Replaces field is populated with the + // expected value. + ariCertID, err := GenerateARICertID(certs[0]) + if err != nil { + return Order{}, fmt.Errorf("expected no error, got: %v", err) + } + if replacementOrder.Replaces != ariCertID { + return Order{}, fmt.Errorf("%s != %s", replacementOrder.Replaces, ariCertID) + } + + // Make sure that the replacement order shares at least one identifier as + // the order it is replacing. We'll do this by sending the exact identifiers + // as the original order. + // See: https://datatracker.ietf.org/doc/html/draft-ietf-acme-ari-03#section-5 + var domains []string + for _, id := range replacementOrder.Identifiers { + domains = append(domains, id.Value) + } + + csr, _ := makeCSR(t, domains) + + // Issue a certificate for the replacement order. + replacementOrder, err = testClient.FinalizeOrder(account, replacementOrder, csr) + if err != nil { + return Order{}, fmt.Errorf("unexpected error: %v", err) + } + if replacementOrder.Status != "valid" { + return Order{}, fmt.Errorf("order not valid") + } + + return replacementOrder, nil +} + +// validateChallenges validates all the challenges for each authorization on the +// given order or returns an error. +func validateChallenges(t *testing.T, order Order, account Account, supportedChalTypes []string, challengesShouldFail bool) error { + if supportedChalTypes == nil { + supportedChalTypes = []string{ChallengeTypeDNS01, ChallengeTypeHTTP01} + } - auth, err := testClient.FetchAuthorization(acct, authURL) + for _, authURL := range order.Authorizations { + auth, err := testClient.FetchAuthorization(account, authURL) if err != nil { - t.Fatalf("unexpected error fetching authorization: %v", err) + return fmt.Errorf("unexpected error fetching authorization: %v", err) } - // panic(fmt.Sprintf("AUTH: %+v\n\nORDER: %+v", auth, order)) - if auth.Status == "valid" { continue } if auth.Status != "pending" { - t.Fatalf("expected auth status pending, got: %v", auth.Status) + return fmt.Errorf("expected auth status pending, got: %v", auth.Status) } chalType := supportedChalTypes[mrand.Intn(len(supportedChalTypes))] @@ -178,45 +278,33 @@ func makeOrderFinalised(t *testing.T, supportedChalTypes []string, identifiers . continue } if chal.Status != "pending" { - t.Fatalf("unexpected status %q on challenge: %+v", chal.Status, chal) + return fmt.Errorf("unexpected status %q on challenge: %+v", chal.Status, chal) } - preChallenge(acct, auth, chal) - defer postChallenge(acct, auth, chal) + if challengesShouldFail { + // We want to trigger the VA's DNS resolver to return SERVFAIL or + // some other type of DNS badness so we can see how the client + // reacts to VA errors. + for _, id := range order.Identifiers { + failPreChallenge(chal, id.Value) + defer failPostChallenge(chal, id.Value) + } + } else { + preChallenge(account, auth, chal) + defer postChallenge(account, auth, chal) + } - updatedChal, err := testClient.UpdateChallenge(acct, chal) + updatedChal, err := testClient.UpdateChallenge(account, chal) if err != nil { - t.Fatalf("error updating challenge %s : %v", chal.URL, err) + return fmt.Errorf("error updating challenge %s : %v", chal.URL, err) } if updatedChal.Status != "valid" { - t.Fatalf("unexpected updated challenge status %q on challenge: %+v", updatedChal.Status, updatedChal) + return fmt.Errorf("unexpected updated challenge status %q on challenge: %+v", updatedChal.Status, updatedChal) } } - updatedOrder, err := testClient.FetchOrder(acct, order.URL) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if updatedOrder.Status != "ready" { - t.Fatal("order not ready") - } - - var domains []string - for _, id := range order.Identifiers { - domains = append(domains, id.Value) - } - csr, privKey := makeCSR(t, domains) - - finalizedOrder, err := testClient.FinalizeOrder(acct, order, csr) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if finalizedOrder.Status != "valid" { - t.Fatal("order not valid") - } - - return acct, finalizedOrder, privKey + return nil } func makeCSR(t *testing.T, domains []string) (*x509.CertificateRequest, crypto.Signer) { @@ -256,6 +344,75 @@ func doPost(name string, req interface{}) { } } +// failPreChallenge causes challtestsrv to respond with bogus data/SERVFAIL for +// the given domain which will trigger the VA to return a validation failure. +func failPreChallenge(chal Challenge, domain string) { + switch chal.Type { + case ChallengeTypeDNS01: + records := []string{domain, "_acme-challenge." + domain} + for _, r := range records { + setReq := struct { + Host string `json:"host"` + }{ + Host: r, + } + doPost("set-servfail", setReq) + } + case ChallengeTypeHTTP01: + addReq := struct { + Token string `json:"token"` + Content string `json:"content"` + }{ + Token: "bad", + Content: "chall", + } + doPost("add-http01", addReq) + + addReq2 := struct { + Host string `json:"host"` + }{ + Host: domain, + } + doPost("set-servfail", addReq2) + + default: + panic("post: unsupported challenge type: " + chal.Type) + } +} + +// failPostChallenge cleans up after failPreChallenge and resets challtestsrv. +func failPostChallenge(chal Challenge, domain string) { + switch chal.Type { + case ChallengeTypeDNS01: + records := []string{domain, "_acme-challenge." + domain} + for _, r := range records { + setReq := struct { + Host string `json:"host"` + }{ + Host: r, + } + doPost("clear-servfail", setReq) + } + case ChallengeTypeHTTP01: + addReq := struct { + Token string `json:"token"` + }{ + Token: "bad", + } + doPost("del-http01", addReq) + + addReq2 := struct { + Host string `json:"host"` + }{ + Host: domain, + } + doPost("clear-servfail", addReq2) + + default: + panic("post: unsupported challenge type: " + chal.Type) + } +} + func preChallenge(acct Account, auth Authorization, chal Challenge) { switch chal.Type { case ChallengeTypeDNS01: