From 32db794688a5a24a23a43f2a984cecd5b3d8da58 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Sun, 10 May 2020 13:40:17 -0700 Subject: [PATCH] acme: fix deadlock when Client.Key is nil When methods that use POSTs are called on a acme.Client which has a nil Key field it will cause a deadlock due to an infinite loop in the code that looks up the account KID. This change adds a check for the key being nil, and errors out if that is the case. Also adds a test for this behavior. Fixes golang/go#38790 Change-Id: I65ff6bbbade7ed2d85306895904a976089730bbf Reviewed-on: https://go-review.googlesource.com/c/crypto/+/233164 Trust: Roland Shoemaker Reviewed-by: Brad Fitzpatrick --- acme/http.go | 4 ++++ acme/http_test.go | 15 +++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/acme/http.go b/acme/http.go index c51943e..2b4c1a1 100644 --- a/acme/http.go +++ b/acme/http.go @@ -10,6 +10,7 @@ import ( "crypto" "crypto/rand" "encoding/json" + "errors" "fmt" "io/ioutil" "math/big" @@ -215,6 +216,9 @@ func (c *Client) post(ctx context.Context, key crypto.Signer, url string, body i func (c *Client) postNoRetry(ctx context.Context, key crypto.Signer, url string, body interface{}) (*http.Response, *http.Request, error) { kid := noKeyID if key == nil { + if c.Key == nil { + return nil, nil, errors.New("acme: Client.Key must be populated to make POST requests") + } key = c.Key kid = c.accountKID(ctx) } diff --git a/acme/http_test.go b/acme/http_test.go index 79095cc..cf1df36 100644 --- a/acme/http_test.go +++ b/acme/http_test.go @@ -238,3 +238,18 @@ func TestUserAgent(t *testing.T) { } } } + +func TestAccountKidLoop(t *testing.T) { + // if Client.postNoRetry is called with a nil key argument + // then Client.Key must be set, otherwise we fall into an + // infinite loop (which also causes a deadlock). + client := &Client{dir: &Directory{OrderURL: ":)"}} + _, _, err := client.postNoRetry(context.Background(), nil, "", nil) + if err == nil { + t.Fatal("Client.postNoRetry didn't fail with a nil key") + } + expected := "acme: Client.Key must be populated to make POST requests" + if err.Error() != expected { + t.Fatalf("Unexpected error returned: wanted %q, got %q", expected, err.Error()) + } +}