crypto/x509: rollback new CertificateRequest fields

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 <[email protected]>
Run-TryBot: Filippo Valsorda <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>

Author: FiloSottile

api/go1.16.txt

@@ -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

doc/go1.16.html

@@ -590,14 +590,6 @@ <h3 id="minor_library_changes">Minor changes to the library</h3>
       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.

src/crypto/x509/x509.go

@@ -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
 			}

src/crypto/x509/x509_test.go

@@ -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{"[email protected]", "[email protected]"},
+		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)
 	}
 }