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 <roland@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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 @@
"crypto"
"crypto/rand"
"encoding/json"
+ "errors"
"fmt"
"io/ioutil"
"math/big"
@@ -215,6 +216,9 @@
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 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())
+ }
+}
