x/crypto/ocsp: Don't hard-code OCSP response hash function

Allows user to set the hash function to use in the OCSP response
when using ocsp.CreateResponse instead of hard-coding the use of
SHA-1. The field IssuerHash is added to the ocsp.Response struct
to set which hash to use. If none is provided CreateResponse falls
back to SHA1. ParseResponse also attempts to populate this field
and will fail if a unsupported hash algorithm is provided.

Change-Id: I3905b1706f347387724e57c33cb82a3b46ffcdf9
Reviewed-on: https://go-review.googlesource.com/32214
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ocsp/ocsp.go

@@ -13,11 +13,14 @@ import (
 	"crypto/elliptic"
 	"crypto/rand"
 	"crypto/rsa"
-	"crypto/sha1"
+	_ "crypto/sha1"
+	_ "crypto/sha256"
+	_ "crypto/sha512"
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/asn1"
 	"errors"
+	"fmt"
 	"math/big"
 	"strconv"
 	"time"
@@ -355,6 +358,11 @@ type Response struct {
 	Signature          []byte
 	SignatureAlgorithm x509.SignatureAlgorithm
 
+	// IssuerHash is the hash used to compute the IssuerNameHash and IssuerKeyHash.
+	// Valid values are crypto.SHA1, crypto.SHA256, crypto.SHA384, and crypto.SHA512.
+	// If zero, the default is crypto.SHA1.
+	IssuerHash crypto.Hash
+
 	// Extensions contains raw X.509 extensions from the singleExtensions field
 	// of the OCSP response. When parsing certificates, this can be used to
 	// extract non-critical extensions that are not parsed by this package. When
@@ -524,6 +532,16 @@ func ParseResponseForCert(bytes []byte, cert, issuer *x509.Certificate) (*Respon
 
 	ret.SerialNumber = r.CertID.SerialNumber
 
+	for h, oid := range hashOIDs {
+		if r.CertID.HashAlgorithm.Algorithm.Equal(oid) {
+			ret.IssuerHash = h
+			break
+		}
+	}
+	if ret.IssuerHash == 0 {
+		return nil, ParseError("unsupported issuer hash algorithm")
+	}
+
 	switch {
 	case bool(r.Good):
 		ret.Status = Good
@@ -606,11 +624,12 @@ func CreateRequest(cert, issuer *x509.Certificate, opts *RequestOptions) ([]byte
 // itself is provided alongside the OCSP response signature.
 //
 // The issuer cert is used to puplate the IssuerNameHash and IssuerKeyHash fields.
-// (SHA-1 is used for the hash function; this is not configurable.)
 //
 // The template is used to populate the SerialNumber, RevocationStatus, RevokedAt,
 // RevocationReason, ThisUpdate, and NextUpdate fields.
 //
+// If template.IssuerHash is not set, SHA1 will be used.
+//
 // The ProducedAt date is automatically set to the current date, to the nearest minute.
 func CreateResponse(issuer, responderCert *x509.Certificate, template Response, priv crypto.Signer) ([]byte, error) {
 	var publicKeyInfo struct {
@@ -621,7 +640,18 @@ func CreateResponse(issuer, responderCert *x509.Certificate, template Response,
 		return nil, err
 	}
 
-	h := sha1.New()
+	if template.IssuerHash == 0 {
+		template.IssuerHash = crypto.SHA1
+	}
+	hashOID := getOIDFromHashAlgorithm(template.IssuerHash)
+	if hashOID == nil {
+		return nil, errors.New("unsupported issuer hash algorithm")
+	}
+
+	if !template.IssuerHash.Available() {
+		return nil, fmt.Errorf("issuer hash algorithm %v not linked into binarya", template.IssuerHash)
+	}
+	h := template.IssuerHash.New()
 	h.Write(publicKeyInfo.PublicKey.RightAlign())
 	issuerKeyHash := h.Sum(nil)
 
@@ -632,7 +662,7 @@ func CreateResponse(issuer, responderCert *x509.Certificate, template Response,
 	innerResponse := singleResponse{
 		CertID: certID{
 			HashAlgorithm: pkix.AlgorithmIdentifier{
-				Algorithm:  hashOIDs[crypto.SHA1],
+				Algorithm:  hashOID,
 				Parameters: asn1.RawValue{Tag: 5 /* ASN.1 NULL */},
 			},
 			NameHash:      issuerNameHash,

ocsp/ocsp_test.go

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
+// +build go1.7
+
 package ocsp
 
 import (
@@ -222,46 +224,76 @@ func TestOCSPResponse(t *testing.T) {
 		ExtraExtensions:  extensions,
 	}
 
-	responseBytes, err := CreateResponse(issuer, responder, template, responderPrivateKey)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	resp, err := ParseResponse(responseBytes, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if !reflect.DeepEqual(resp.ThisUpdate, template.ThisUpdate) {
-		t.Errorf("resp.ThisUpdate: got %d, want %d", resp.ThisUpdate, template.ThisUpdate)
-	}
-
-	if !reflect.DeepEqual(resp.NextUpdate, template.NextUpdate) {
-		t.Errorf("resp.NextUpdate: got %d, want %d", resp.NextUpdate, template.NextUpdate)
-	}
-
-	if !reflect.DeepEqual(resp.RevokedAt, template.RevokedAt) {
-		t.Errorf("resp.RevokedAt: got %d, want %d", resp.RevokedAt, template.RevokedAt)
-	}
-
-	if !reflect.DeepEqual(resp.Extensions, template.ExtraExtensions) {
-		t.Errorf("resp.Extensions: got %v, want %v", resp.Extensions, template.ExtraExtensions)
-	}
-
-	if !resp.ProducedAt.Equal(producedAt) {
-		t.Errorf("resp.ProducedAt: got %d, want %d", resp.ProducedAt, producedAt)
-	}
-
-	if resp.Status != template.Status {
-		t.Errorf("resp.Status: got %d, want %d", resp.Status, template.Status)
-	}
-
-	if resp.SerialNumber.Cmp(template.SerialNumber) != 0 {
-		t.Errorf("resp.SerialNumber: got %x, want %x", resp.SerialNumber, template.SerialNumber)
-	}
-
-	if resp.RevocationReason != template.RevocationReason {
-		t.Errorf("resp.RevocationReason: got %d, want %d", resp.RevocationReason, template.RevocationReason)
+	template.IssuerHash = crypto.MD5
+	_, err = CreateResponse(issuer, responder, template, responderPrivateKey)
+	if err == nil {
+		t.Fatal("CreateResponse didn't fail with non-valid template.IssuerHash value crypto.MD5")
+	}
+
+	testCases := []struct {
+		name       string
+		issuerHash crypto.Hash
+	}{
+		{"Zero value", 0},
+		{"crypto.SHA1", crypto.SHA1},
+		{"crypto.SHA256", crypto.SHA256},
+		{"crypto.SHA384", crypto.SHA384},
+		{"crypto.SHA512", crypto.SHA512},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			template.IssuerHash = tc.issuerHash
+			responseBytes, err := CreateResponse(issuer, responder, template, responderPrivateKey)
+			if err != nil {
+				t.Fatalf("CreateResponse failed: %s", err)
+			}
+
+			resp, err := ParseResponse(responseBytes, nil)
+			if err != nil {
+				t.Fatalf("ParseResponse failed: %s", err)
+			}
+
+			if !reflect.DeepEqual(resp.ThisUpdate, template.ThisUpdate) {
+				t.Errorf("resp.ThisUpdate: got %d, want %d", resp.ThisUpdate, template.ThisUpdate)
+			}
+
+			if !reflect.DeepEqual(resp.NextUpdate, template.NextUpdate) {
+				t.Errorf("resp.NextUpdate: got %d, want %d", resp.NextUpdate, template.NextUpdate)
+			}
+
+			if !reflect.DeepEqual(resp.RevokedAt, template.RevokedAt) {
+				t.Errorf("resp.RevokedAt: got %d, want %d", resp.RevokedAt, template.RevokedAt)
+			}
+
+			if !reflect.DeepEqual(resp.Extensions, template.ExtraExtensions) {
+				t.Errorf("resp.Extensions: got %v, want %v", resp.Extensions, template.ExtraExtensions)
+			}
+
+			if !resp.ProducedAt.Equal(producedAt) {
+				t.Errorf("resp.ProducedAt: got %d, want %d", resp.ProducedAt, producedAt)
+			}
+
+			if resp.Status != template.Status {
+				t.Errorf("resp.Status: got %d, want %d", resp.Status, template.Status)
+			}
+
+			if resp.SerialNumber.Cmp(template.SerialNumber) != 0 {
+				t.Errorf("resp.SerialNumber: got %x, want %x", resp.SerialNumber, template.SerialNumber)
+			}
+
+			if resp.RevocationReason != template.RevocationReason {
+				t.Errorf("resp.RevocationReason: got %d, want %d", resp.RevocationReason, template.RevocationReason)
+			}
+
+			expectedHash := tc.issuerHash
+			if tc.issuerHash == 0 {
+				expectedHash = crypto.SHA1
+			}
+
+			if resp.IssuerHash != expectedHash {
+				t.Errorf("resp.IssuerHash: got %d, want %d", resp.IssuerHash, expectedHash)
+			}
+		})
 	}
 }