From b07d8c96772f426812d3fc5530710ec1f3b205e7 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Wed, 26 Oct 2016 16:47:36 -0700 Subject: [PATCH] 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 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- ocsp/ocsp.go | 38 +++++++++++++++++--- ocsp/ocsp_test.go | 90 ++++++++++++++++++++++++++++++++--------------- 2 files changed, 95 insertions(+), 33 deletions(-) diff --git a/ocsp/ocsp.go b/ocsp/ocsp.go index 2c7e57a..6b1ffc3 100644 --- a/ocsp/ocsp.go +++ b/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, diff --git a/ocsp/ocsp_test.go b/ocsp/ocsp_test.go index f66489a..05f59b1 100644 --- a/ocsp/ocsp_test.go +++ b/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) + 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") } - resp, err := ParseResponse(responseBytes, nil) - if err != nil { - t.Fatal(err) + 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) + } - if !reflect.DeepEqual(resp.ThisUpdate, template.ThisUpdate) { - t.Errorf("resp.ThisUpdate: got %d, want %d", resp.ThisUpdate, template.ThisUpdate) - } + resp, err := ParseResponse(responseBytes, nil) + if err != nil { + t.Fatalf("ParseResponse failed: %s", err) + } - if !reflect.DeepEqual(resp.NextUpdate, template.NextUpdate) { - t.Errorf("resp.NextUpdate: got %d, want %d", resp.NextUpdate, template.NextUpdate) - } + if !reflect.DeepEqual(resp.ThisUpdate, template.ThisUpdate) { + t.Errorf("resp.ThisUpdate: got %d, want %d", resp.ThisUpdate, template.ThisUpdate) + } - if !reflect.DeepEqual(resp.RevokedAt, template.RevokedAt) { - t.Errorf("resp.RevokedAt: got %d, want %d", resp.RevokedAt, template.RevokedAt) - } + if !reflect.DeepEqual(resp.NextUpdate, template.NextUpdate) { + t.Errorf("resp.NextUpdate: got %d, want %d", resp.NextUpdate, template.NextUpdate) + } - if !reflect.DeepEqual(resp.Extensions, template.ExtraExtensions) { - t.Errorf("resp.Extensions: got %v, want %v", resp.Extensions, template.ExtraExtensions) - } + if !reflect.DeepEqual(resp.RevokedAt, template.RevokedAt) { + t.Errorf("resp.RevokedAt: got %d, want %d", resp.RevokedAt, template.RevokedAt) + } - if !resp.ProducedAt.Equal(producedAt) { - t.Errorf("resp.ProducedAt: got %d, want %d", resp.ProducedAt, producedAt) - } + if !reflect.DeepEqual(resp.Extensions, template.ExtraExtensions) { + t.Errorf("resp.Extensions: got %v, want %v", resp.Extensions, template.ExtraExtensions) + } - if resp.Status != template.Status { - t.Errorf("resp.Status: got %d, want %d", resp.Status, template.Status) - } + if !resp.ProducedAt.Equal(producedAt) { + t.Errorf("resp.ProducedAt: got %d, want %d", resp.ProducedAt, producedAt) + } - if resp.SerialNumber.Cmp(template.SerialNumber) != 0 { - t.Errorf("resp.SerialNumber: got %x, want %x", resp.SerialNumber, template.SerialNumber) - } + if resp.Status != template.Status { + t.Errorf("resp.Status: got %d, want %d", resp.Status, template.Status) + } - if resp.RevocationReason != template.RevocationReason { - t.Errorf("resp.RevocationReason: got %d, want %d", resp.RevocationReason, template.RevocationReason) + 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) + } + }) } }