acme: hardcode and remove ExternalAccountBinding.Algorithm

HMAC-SHA256 is a perfectly fine MAC algorithm, and there is no need to ask the user to choose one. This does break compatibility with the previous API, but it had been live only for a weekend, so hopefully still in a window in which we can make changes with a limited blast radius. Updates golang/go#41430 Change-Id: I03741a545b25b9fcc147760cd20e9d7029844a6c Reviewed-on: https://go-review.googlesource.com/c/crypto/+/279453 Trust: Filippo Valsorda <filippo@golang.org> Run-TryBot: Filippo Valsorda <filippo@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: James Kasten <jdkasten@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
2020-12-21 17:57:36 +01:00 · 2020-12-21 17:57:36 +01:00 · eec23a3978
--- a/acme/jws.go
+++ b/acme/jws.go
@ -11,27 +11,15 @@ import (
 	"crypto/rand"
 	"crypto/rsa"
 	"crypto/sha256"
-	"crypto/sha512"
 	_ "crypto/sha512" // need for EC keys
 	"encoding/asn1"
 	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"fmt"
-	"hash"
 	"math/big"
 )

-// MACAlgorithm represents a JWS MAC signature algorithm.
-// See https://tools.ietf.org/html/rfc7518#section-3.1 for more details.
-type MACAlgorithm string
-
-const (
-	MACAlgorithmHS256 = MACAlgorithm("HS256")
-	MACAlgorithmHS384 = MACAlgorithm("HS384")
-	MACAlgorithmHS512 = MACAlgorithm("HS512")
-)
-
 // keyID is the account identity provided by a CA during registration.
 type keyID string

@ -101,24 +89,35 @@ func jwsEncodeJSON(claimset interface{}, key crypto.Signer, kid keyID, nonce, ur
 	return json.Marshal(&enc)
 }

-// jwsWithMAC creates and signs a JWS using the given key and algorithm.
-// "rawProtected" and "rawPayload" should not be base64-URL-encoded.
-func jwsWithMAC(key []byte, alg MACAlgorithm, rawProtected, rawPayload []byte) (*jsonWebSignature, error) {
+// jwsWithMAC creates and signs a JWS using the given key and the HS256
+// algorithm. kid and url are included in the protected header. rawPayload
+// should not be base64-URL-encoded.
+func jwsWithMAC(key []byte, kid, url string, rawPayload []byte) (*jsonWebSignature, error) {
 	if len(key) == 0 {
 		return nil, errors.New("acme: cannot sign JWS with an empty MAC key")
 	}
+	header := struct {
+		Algorithm string `json:"alg"`
+		KID       string `json:"kid"`
+		URL       string `json:"url,omitempty"`
+	}{
+		// Only HMAC-SHA256 is supported.
+		Algorithm: "HS256",
+		KID:       kid,
+		URL:       url,
+	}
+	rawProtected, err := json.Marshal(header)
+	if err != nil {
+		return nil, err
+	}
 	protected := base64.RawURLEncoding.EncodeToString(rawProtected)
 	payload := base64.RawURLEncoding.EncodeToString(rawPayload)

-	// Only HMACs are currently supported.
-	hmac, err := newHMAC(key, alg)
-	if err != nil {
+	h := hmac.New(sha256.New, key)
+	if _, err := h.Write([]byte(protected + "." + payload)); err != nil {
 		return nil, err
 	}
-	if _, err := hmac.Write([]byte(protected + "." + payload)); err != nil {
-		return nil, err
-	}
-	mac := hmac.Sum(nil)
+	mac := h.Sum(nil)

 	return &jsonWebSignature{
 		Protected: protected,
@ -218,20 +217,6 @@ func jwsHasher(pub crypto.PublicKey) (string, crypto.Hash) {
 	return "", 0
 }

-// newHMAC returns an appropriate HMAC for the given MACAlgorithm.
-func newHMAC(key []byte, alg MACAlgorithm) (hash.Hash, error) {
-	switch alg {
-	case MACAlgorithmHS256:
-		return hmac.New(sha256.New, key), nil
-	case MACAlgorithmHS384:
-		return hmac.New(sha512.New384, key), nil
-	case MACAlgorithmHS512:
-		return hmac.New(sha512.New, key), nil
-	default:
-		return nil, fmt.Errorf("acme: unsupported MAC algorithm: %v", alg)
-	}
-}
-
 // JWKThumbprint creates a JWK thumbprint out of pub
 // as specified in https://tools.ietf.org/html/rfc7638.
 func JWKThumbprint(pub crypto.PublicKey) (string, error) {
--- a/acme/jws_test.go
+++ b/acme/jws_test.go
@ -397,8 +397,6 @@ func TestJWSWithMAC(t *testing.T) {
 	// Example from RFC 7520 Section 4.4.3.
 	// https://tools.ietf.org/html/rfc7520#section-4.4.3
 	b64Key := "hJtXIZ2uSN5kbQfbtTNWbpdmhkV8FJG-Onbc6mxCcYg"
-	alg := MACAlgorithmHS256
-	rawProtected := []byte(`{"alg":"HS256","kid":"018c0ae5-4d9b-471b-bfd6-eef314bc7037"}`)
 	rawPayload := []byte("It\xe2\x80\x99s a dangerous business, Frodo, going out your " +
 		"door. You step onto the road, and if you don't keep your feet, " +
 		"there\xe2\x80\x99s no knowing where you might be swept off " +
@ -416,7 +414,7 @@ func TestJWSWithMAC(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unable to decode key: %q", b64Key)
 	}
-	got, err := jwsWithMAC(key, alg, rawProtected, rawPayload)
+	got, err := jwsWithMAC(key, "018c0ae5-4d9b-471b-bfd6-eef314bc7037", "", rawPayload)
 	if err != nil {
 		t.Fatalf("jwsWithMAC() = %q", err)
 	}
@ -432,22 +430,9 @@ func TestJWSWithMAC(t *testing.T) {
 }

 func TestJWSWithMACError(t *testing.T) {
-	tt := []struct {
-		desc string
-		alg  MACAlgorithm
-		key  []byte
-	}{
-		{"Unknown Algorithm", MACAlgorithm("UNKNOWN-ALG"), []byte("hmac-key")},
-		{"Empty Key", MACAlgorithmHS256, nil},
-	}
-	for _, tc := range tt {
-		tc := tc
-		t.Run(string(tc.desc), func(t *testing.T) {
-			p := "{}"
-			if _, err := jwsWithMAC(tc.key, tc.alg, []byte(p), []byte(p)); err == nil {
-				t.Errorf("jwsWithMAC(%v, %v, %s, %s) = success; want err", tc.key, tc.alg, p, p)
-			}
-		})
+	p := "{}"
+	if _, err := jwsWithMAC(nil, "", "", []byte(p)); err == nil {
+		t.Errorf("jwsWithMAC(nil, ...) = success; want err")
 	}
 }

@ -525,33 +510,3 @@ func TestJWKThumbprintErrUnsupportedKey(t *testing.T) {
 		t.Errorf("err = %q; want %q", err, ErrUnsupportedKey)
 	}
 }
-
-func TestNewHMAC(t *testing.T) {
-	tt := []struct {
-		alg      MACAlgorithm
-		wantSize int
-	}{
-		{MACAlgorithmHS256, 32},
-		{MACAlgorithmHS384, 48},
-		{MACAlgorithmHS512, 64},
-	}
-	for _, tc := range tt {
-		tc := tc
-		t.Run(string(tc.alg), func(t *testing.T) {
-			h, err := newHMAC([]byte("key"), tc.alg)
-			if err != nil {
-				t.Fatalf("newHMAC(%v) = %q", tc.alg, err)
-			}
-			gotSize := len(h.Sum(nil))
-			if gotSize != tc.wantSize {
-				t.Errorf("HMAC produced signature with unexpected length; got %d want %d", gotSize, tc.wantSize)
-			}
-		})
-	}
-}
-
-func TestNewHMACError(t *testing.T) {
-	if h, err := newHMAC([]byte("key"), MACAlgorithm("UNKNOWN-ALG")); err == nil {
-		t.Errorf("newHMAC(UNKNOWN-ALG) = %T, nil; want error", h)
-	}
-}
--- a/acme/rfc8555.go
+++ b/acme/rfc8555.go
@ -5,7 +5,6 @@
 package acme

 import (
-	"bytes"
 	"context"
 	"crypto"
 	"encoding/base64"
@ -93,9 +92,7 @@ func (c *Client) encodeExternalAccountBinding(eab *ExternalAccountBinding) (*jso
 	if err != nil {
 		return nil, err
 	}
-	var rProtected bytes.Buffer
-	fmt.Fprintf(&rProtected, `{"alg":%q,"kid":%q,"url":%q}`, eab.Algorithm, eab.KID, c.dir.RegURL)
-	return jwsWithMAC(eab.Key, eab.Algorithm, rProtected.Bytes(), []byte(jwk))
+	return jwsWithMAC(eab.Key, eab.KID, c.dir.RegURL, []byte(jwk))
 }

 // updateRegRFC is equivalent to c.UpdateReg but for CAs implementing RFC 8555.
--- a/acme/rfc8555_test.go
+++ b/acme/rfc8555_test.go
@ -351,9 +351,8 @@ func TestRFC_Register(t *testing.T) {

 func TestRFC_RegisterExternalAccountBinding(t *testing.T) {
 	eab := &ExternalAccountBinding{
-		KID:       "kid-1",
-		Key:       []byte("secret"),
-		Algorithm: MACAlgorithmHS256,
+		KID: "kid-1",
+		Key: []byte("secret"),
 	}

 	type protected struct {
@ -403,10 +402,10 @@ func TestRFC_RegisterExternalAccountBinding(t *testing.T) {
 		if prot.KID != eab.KID {
 			t.Errorf("j.ExternalAccountBinding.KID = %s; want %s", prot.KID, eab.KID)
 		}
-		// Ensure same Algorithm.
-		if prot.Algorithm != string(eab.Algorithm) {
+		// Ensure expected Algorithm.
+		if prot.Algorithm != "HS256" {
 			t.Errorf("j.ExternalAccountBinding.Alg = %s; want %s",
-				prot.Algorithm, eab.Algorithm)
+				prot.Algorithm, "HS256")
 		}

 		// Ensure same URL as outer JWS.
--- a/acme/types.go
+++ b/acme/types.go
@ -217,13 +217,10 @@ type ExternalAccountBinding struct {
 	// Key is the bytes of the symmetric key that the CA provides to identify
 	// the account. Key must correspond to the KID.
 	Key []byte
-
-	// Algorithm used to sign the JWS.
-	Algorithm MACAlgorithm
 }

 func (e *ExternalAccountBinding) String() string {
-	return fmt.Sprintf("&{KID: %q, Key: redacted, Algorithm: %v}", e.KID, e.Algorithm)
+	return fmt.Sprintf("&{KID: %q, Key: redacted}", e.KID)
 }

 // Directory is ACME server discovery data.
--- a/acme/types_test.go
+++ b/acme/types_test.go
@ -13,12 +13,11 @@ import (

 func TestExternalAccountBindingString(t *testing.T) {
 	eab := ExternalAccountBinding{
-		KID:       "kid",
-		Key:       []byte("key"),
-		Algorithm: MACAlgorithmHS256,
+		KID: "kid",
+		Key: []byte("key"),
 	}
 	got := eab.String()
-	want := `&{KID: "kid", Key: redacted, Algorithm: HS256}`
+	want := `&{KID: "kid", Key: redacted}`
 	if got != want {
 		t.Errorf("eab.String() = %q, want: %q", got, want)
 	}