]> Cypherpunks repositories - gostls13.git/commitdiff
crypto/x509: rollback new CertificateRequest fields
authorFilippo Valsorda <filippo@golang.org>
Tue, 5 Jan 2021 19:52:00 +0000 (20:52 +0100)
committerFilippo Valsorda <filippo@golang.org>
Wed, 6 Jan 2021 21:24:47 +0000 (21:24 +0000)
In general, we don't want to encourage reading them from CSRs, and
applications that really want to can parse the Extensions field.

Note that this also fixes a bug where the error of
parseKeyUsageExtension was not handled in parseCertificateRequest.

Fixes #43477
Updates #37172

Change-Id: Ia5707b0e23cecc0aed57e419a1ca25e26eea6bbe
Reviewed-on: https://go-review.googlesource.com/c/go/+/281235
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
api/go1.16.txt
doc/go1.16.html
src/crypto/x509/x509.go
src/crypto/x509/x509_test.go

index 16d9cb891b90a48771cb84dc70f864cda5bf5c20..baac5379f81e51090b4efd9a72c270ab8b844787 100644 (file)
@@ -1,15 +1,6 @@
 pkg archive/zip, method (*ReadCloser) Open(string) (fs.File, error)
 pkg archive/zip, method (*Reader) Open(string) (fs.File, error)
 pkg crypto/x509, method (SystemRootsError) Unwrap() error
-pkg crypto/x509, type CertificateRequest struct, BasicConstraintsValid bool
-pkg crypto/x509, type CertificateRequest struct, ExtKeyUsage []ExtKeyUsage
-pkg crypto/x509, type CertificateRequest struct, IsCA bool
-pkg crypto/x509, type CertificateRequest struct, KeyUsage KeyUsage
-pkg crypto/x509, type CertificateRequest struct, MaxPathLen int
-pkg crypto/x509, type CertificateRequest struct, MaxPathLenZero bool
-pkg crypto/x509, type CertificateRequest struct, PolicyIdentifiers []asn1.ObjectIdentifier
-pkg crypto/x509, type CertificateRequest struct, SubjectKeyId []uint8
-pkg crypto/x509, type CertificateRequest struct, UnknownExtKeyUsage []asn1.ObjectIdentifier
 pkg debug/elf, const DT_ADDRRNGHI = 1879047935
 pkg debug/elf, const DT_ADDRRNGHI DynTag
 pkg debug/elf, const DT_ADDRRNGLO = 1879047680
index 0c2921fe6b0b1880e479e30e8be4138413573a43..f0dbee7b8992d71da4d263aaef3c0a7e210a4ea1 100644 (file)
@@ -590,14 +590,6 @@ func TestFoo(t *testing.T) {
       a malformed certificate.
     </p>
 
-    <p><!-- CL 233163 -->
-      A number of additional fields have been added to the
-      <a href="/pkg/crypto/x509/#CertificateRequest"><code>CertificateRequest</code></a> type.
-      These fields are now parsed in <a href="/pkg/crypto/x509/#ParseCertificateRequest">
-      <code>ParseCertificateRequest</code></a> and marshalled in
-      <a href="/pkg/crypto/x509/#CreateCertificateRequest"><code>CreateCertificateRequest</code></a>.
-    </p>
-
     <p><!-- CL 257939 -->
       DSA signature verification is no longer supported. Note that DSA signature
       generation was never supported.
index 60dfac741b8774017dd967abb868c9ba99a8a44a..42d8158d63ce75052e0b74eea7f62d33143434f6 100644 (file)
@@ -2006,40 +2006,6 @@ func buildCSRExtensions(template *CertificateRequest) ([]pkix.Extension, error)
                ret = append(ret, ext)
        }
 
-       if (len(template.ExtKeyUsage) > 0 || len(template.UnknownExtKeyUsage) > 0) &&
-               !oidInExtensions(oidExtensionExtendedKeyUsage, template.ExtraExtensions) {
-               ext, err := marshalExtKeyUsage(template.ExtKeyUsage, template.UnknownExtKeyUsage)
-               if err != nil {
-                       return nil, err
-               }
-               ret = append(ret, ext)
-       }
-
-       if template.BasicConstraintsValid && !oidInExtensions(oidExtensionBasicConstraints, template.ExtraExtensions) {
-               ext, err := marshalBasicConstraints(template.IsCA, template.MaxPathLen, template.MaxPathLenZero)
-               if err != nil {
-                       return nil, err
-               }
-               ret = append(ret, ext)
-       }
-
-       if len(template.SubjectKeyId) > 0 && !oidInExtensions(oidExtensionSubjectKeyId, template.ExtraExtensions) {
-               skidBytes, err := asn1.Marshal(template.SubjectKeyId)
-               if err != nil {
-                       return nil, err
-               }
-               ret = append(ret, pkix.Extension{Id: oidExtensionSubjectKeyId, Value: skidBytes})
-       }
-
-       if len(template.PolicyIdentifiers) > 0 &&
-               !oidInExtensions(oidExtensionCertificatePolicies, template.ExtraExtensions) {
-               ext, err := marshalCertificatePolicies(template.PolicyIdentifiers)
-               if err != nil {
-                       return nil, err
-               }
-               ret = append(ret, ext)
-       }
-
        return append(ret, template.ExtraExtensions...), nil
 }
 
@@ -2438,37 +2404,6 @@ type CertificateRequest struct {
        EmailAddresses []string
        IPAddresses    []net.IP
        URIs           []*url.URL
-
-       ExtKeyUsage        []ExtKeyUsage           // Sequence of extended key usages.
-       UnknownExtKeyUsage []asn1.ObjectIdentifier // Encountered extended key usages unknown to this package.
-
-       // BasicConstraintsValid indicates whether IsCA, MaxPathLen,
-       // and MaxPathLenZero are valid.
-       BasicConstraintsValid bool
-       IsCA                  bool
-
-       // MaxPathLen and MaxPathLenZero indicate the presence and
-       // value of the BasicConstraints' "pathLenConstraint".
-       //
-       // When parsing a certificate, a positive non-zero MaxPathLen
-       // means that the field was specified, -1 means it was unset,
-       // and MaxPathLenZero being true mean that the field was
-       // explicitly set to zero. The case of MaxPathLen==0 with MaxPathLenZero==false
-       // should be treated equivalent to -1 (unset).
-       //
-       // When generating a certificate, an unset pathLenConstraint
-       // can be requested with either MaxPathLen == -1 or using the
-       // zero value for both MaxPathLen and MaxPathLenZero.
-       MaxPathLen int
-       // MaxPathLenZero indicates that BasicConstraintsValid==true
-       // and MaxPathLen==0 should be interpreted as an actual
-       // maximum path length of zero. Otherwise, that combination is
-       // interpreted as MaxPathLen not being set.
-       MaxPathLenZero bool
-
-       SubjectKeyId []byte
-
-       PolicyIdentifiers []asn1.ObjectIdentifier
 }
 
 // These structures reflect the ASN.1 structure of X.509 certificate
@@ -2801,25 +2736,6 @@ func parseCertificateRequest(in *certificateRequest) (*CertificateRequest, error
                        }
                case extension.Id.Equal(oidExtensionKeyUsage):
                        out.KeyUsage, err = parseKeyUsageExtension(extension.Value)
-               case extension.Id.Equal(oidExtensionExtendedKeyUsage):
-                       out.ExtKeyUsage, out.UnknownExtKeyUsage, err = parseExtKeyUsageExtension(extension.Value)
-                       if err != nil {
-                               return nil, err
-                       }
-               case extension.Id.Equal(oidExtensionBasicConstraints):
-                       out.IsCA, out.MaxPathLen, err = parseBasicConstraintsExtension(extension.Value)
-                       if err != nil {
-                               return nil, err
-                       }
-                       out.BasicConstraintsValid = true
-                       out.MaxPathLenZero = out.MaxPathLen == 0
-               case extension.Id.Equal(oidExtensionSubjectKeyId):
-                       out.SubjectKeyId, err = parseSubjectKeyIdExtension(extension.Value)
-                       if err != nil {
-                               return nil, err
-                       }
-               case extension.Id.Equal(oidExtensionCertificatePolicies):
-                       out.PolicyIdentifiers, err = parseCertificatePoliciesExtension(extension.Value)
                        if err != nil {
                                return nil, err
                        }
index 65d105db3441607d5b4c392b388b79a4599f4631..d5c7ec466b852d3baef3ecfb6503e262eecf5ada 100644 (file)
@@ -2964,44 +2964,38 @@ func certPoolEqual(a, b *CertPool) bool {
 }
 
 func TestCertificateRequestRoundtripFields(t *testing.T) {
+       urlA, err := url.Parse("https://example.com/_")
+       if err != nil {
+               t.Fatal(err)
+       }
+       urlB, err := url.Parse("https://example.org/_")
+       if err != nil {
+               t.Fatal(err)
+       }
        in := &CertificateRequest{
-               KeyUsage:              KeyUsageCertSign,
-               ExtKeyUsage:           []ExtKeyUsage{ExtKeyUsageAny},
-               UnknownExtKeyUsage:    []asn1.ObjectIdentifier{{1, 2, 3}},
-               BasicConstraintsValid: true,
-               IsCA:                  true,
-               MaxPathLen:            0,
-               MaxPathLenZero:        true,
-               SubjectKeyId:          []byte{1, 2, 3},
-               PolicyIdentifiers:     []asn1.ObjectIdentifier{{1, 2, 3}},
+               DNSNames:       []string{"example.com", "example.org"},
+               EmailAddresses: []string{"a@example.com", "b@example.com"},
+               IPAddresses:    []net.IP{net.IPv4(192, 0, 2, 0), net.IPv6loopback},
+               URIs:           []*url.URL{urlA, urlB},
+               KeyUsage:       KeyUsageCertSign,
        }
        out := marshalAndParseCSR(t, in)
 
-       if in.KeyUsage != out.KeyUsage {
-               t.Fatalf("Unexpected KeyUsage: got %v, want %v", out.KeyUsage, in.KeyUsage)
-       }
-       if !reflect.DeepEqual(in.ExtKeyUsage, out.ExtKeyUsage) {
-               t.Fatalf("Unexpected ExtKeyUsage: got %v, want %v", out.ExtKeyUsage, in.ExtKeyUsage)
-       }
-       if !reflect.DeepEqual(in.UnknownExtKeyUsage, out.UnknownExtKeyUsage) {
-               t.Fatalf("Unexpected UnknownExtKeyUsage: got %v, want %v", out.UnknownExtKeyUsage, in.UnknownExtKeyUsage)
+       if !reflect.DeepEqual(in.DNSNames, out.DNSNames) {
+               t.Fatalf("Unexpected DNSNames: got %v, want %v", out.DNSNames, in.DNSNames)
        }
-       if in.BasicConstraintsValid != out.BasicConstraintsValid {
-               t.Fatalf("Unexpected BasicConstraintsValid: got %v, want %v", out.BasicConstraintsValid, in.BasicConstraintsValid)
+       if !reflect.DeepEqual(in.EmailAddresses, out.EmailAddresses) {
+               t.Fatalf("Unexpected EmailAddresses: got %v, want %v", out.EmailAddresses, in.EmailAddresses)
        }
-       if in.IsCA != out.IsCA {
-               t.Fatalf("Unexpected IsCA: got %v, want %v", out.IsCA, in.IsCA)
+       if len(in.IPAddresses) != len(out.IPAddresses) ||
+               !in.IPAddresses[0].Equal(out.IPAddresses[0]) ||
+               !in.IPAddresses[1].Equal(out.IPAddresses[1]) {
+               t.Fatalf("Unexpected IPAddresses: got %v, want %v", out.IPAddresses, in.IPAddresses)
        }
-       if in.MaxPathLen != out.MaxPathLen {
-               t.Fatalf("Unexpected MaxPathLen: got %v, want %v", out.MaxPathLen, in.MaxPathLen)
+       if !reflect.DeepEqual(in.URIs, out.URIs) {
+               t.Fatalf("Unexpected URIs: got %v, want %v", out.URIs, in.URIs)
        }
-       if in.MaxPathLenZero != out.MaxPathLenZero {
-               t.Fatalf("Unexpected MaxPathLenZero: got %v, want %v", out.MaxPathLenZero, in.MaxPathLenZero)
-       }
-       if !reflect.DeepEqual(in.SubjectKeyId, out.SubjectKeyId) {
-               t.Fatalf("Unexpected SubjectKeyId: got %v, want %v", out.SubjectKeyId, in.SubjectKeyId)
-       }
-       if !reflect.DeepEqual(in.PolicyIdentifiers, out.PolicyIdentifiers) {
-               t.Fatalf("Unexpected PolicyIdentifiers: got %v, want %v", out.PolicyIdentifiers, in.PolicyIdentifiers)
+       if in.KeyUsage != out.KeyUsage {
+               t.Fatalf("Unexpected KeyUsage: got %v, want %v", out.KeyUsage, in.KeyUsage)
        }
 }