Changes for PR

Fix to mgf1SHA1Identifier (only had sha1 oid in params but needed
sha1Identifier)
Style changes and comments

Author: DenisKarch

x509/x509.go

@@ -35,6 +35,8 @@
 //  - RPKI support:
 //     - Support for SubjectInfoAccess extension
 //     - Support for RFC3779 extensions (in rpki.go)
+//  - RSAES-OAEP support:
+//     - Support for parsing RSASES-OAEP public keys from certificates
 //  - General improvements:
 //     - Export and use OID values throughout.
 //     - Export OIDFromNamedCurve().
@@ -190,6 +192,9 @@ type tbsCertificate struct {
 	Extensions         []pkix.Extension `asn1:"optional,explicit,tag:3"`
 }
 
+// RFC 4055,  4.1
+// The current ASN.1 parser does not support non-integer defaults so
+// the 'default:' tags here do nothing.
 type rsaesoaepAlgorithmParameters struct {
 	HashFunc    pkix.AlgorithmIdentifier `asn1:"optional,explicit,tag:0,default:sha1Identifier"`
 	MaskgenFunc pkix.AlgorithmIdentifier `asn1:"optional,explicit,tag:1,default:mgf1SHA1Identifier"`
@@ -244,34 +249,36 @@ const (
 	SHA512WithRSAPSS
 )
 
-// RFC 4055
-// Various identifiers
-var (
-	// 2.1.  One-way Hash Functions
-	OIDSHA1 = asn1.ObjectIdentifier{1, 3, 14, 3, 2, 26}
-	// 2.2.  Mask Generation Functions
-	OIDMFGSHA1 = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 1, 8}
-	// 6.    Basic object identifiers
-	OIDpSpecified = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 1, 9}
-)
+// RFC 4055,  6. Basic object identifiers
+var oidpSpecified = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 1, 9}
 
 // These are the default parameters for an RSAES-OAEP pubkey.
 // The current ASN.1 parser does not support non-integer defaults so
 // these currently do nothing.
-// It has been suggested that we switch to cryptobytes but that
-// only supports parsing primitives (tag < 32).
 var (
 	sha1Identifier = pkix.AlgorithmIdentifier{
-		Algorithm:  OIDSHA1,
+		Algorithm:  oidSHA1,
 		Parameters: asn1.NullRawValue,
 	}
 	mgf1SHA1Identifier = pkix.AlgorithmIdentifier{
-		Algorithm:  OIDMFGSHA1,
-		Parameters: asn1.RawValue{0, 6, false, []byte{43, 14, 3, 2, 26}, []byte{6, 5, 43, 14, 3, 2, 26}},
+		Algorithm: oidMGF1,
+		// RFC 4055, 2.1 sha1Identifier
+		Parameters: asn1.RawValue{
+			Class:      asn1.ClassUniversal,
+			Tag:        asn1.TagSequence,
+			IsCompound: false,
+			Bytes:      []byte{6, 5, 43, 14, 3, 2, 26, 5, 0},
+			FullBytes:  []byte{16, 9, 6, 5, 43, 14, 3, 2, 26, 5, 0}},
 	}
 	pSpecifiedEmptyIdentifier = pkix.AlgorithmIdentifier{
-		Algorithm:  OIDpSpecified,
-		Parameters: asn1.RawValue{0, 4, false, []byte{}, []byte{4, 0}},
+		Algorithm: oidpSpecified,
+		// RFC 4055, 4.1 nullOctetString
+		Parameters: asn1.RawValue{
+			Class:      asn1.ClassUniversal,
+			Tag:        asn1.TagOctetString,
+			IsCompound: false,
+			Bytes:      []byte{},
+			FullBytes:  []byte{4, 0}},
 	}
 )
 
@@ -384,6 +391,7 @@ var (
 	oidSignatureECDSAWithSHA384 = asn1.ObjectIdentifier{1, 2, 840, 10045, 4, 3, 3}
 	oidSignatureECDSAWithSHA512 = asn1.ObjectIdentifier{1, 2, 840, 10045, 4, 3, 4}
 
+	oidSHA1   = asn1.ObjectIdentifier{1, 3, 14, 3, 2, 26}
 	oidSHA256 = asn1.ObjectIdentifier{2, 16, 840, 1, 101, 3, 4, 2, 1}
 	oidSHA384 = asn1.ObjectIdentifier{2, 16, 840, 1, 101, 3, 4, 2, 2}
 	oidSHA512 = asn1.ObjectIdentifier{2, 16, 840, 1, 101, 3, 4, 2, 3}
@@ -1286,40 +1294,28 @@ type distributionPointName struct {
 func parsePublicKey(algo PublicKeyAlgorithm, keyData *publicKeyInfo, nfe *NonFatalErrors) (interface{}, error) {
 	asn1Data := keyData.PublicKey.RightAlign()
 	switch algo {
-	case RSA:
+	case RSA, RSAESOAEP:
 		// RSA public keys must have a NULL in the parameters.
 		// See RFC 3279, Section 2.3.1.
-		if !bytes.Equal(keyData.Algorithm.Parameters.FullBytes, asn1.NullBytes) {
+		if algo == RSA && !bytes.Equal(keyData.Algorithm.Parameters.FullBytes, asn1.NullBytes) {
 			nfe.AddError(errors.New("x509: RSA key missing NULL parameters"))
 		}
-
-		p := new(pkcs1PublicKey)
-		rest, err := asn1.Unmarshal(asn1Data, p)
-		if err != nil {
-			var laxErr error
-			rest, laxErr = asn1.UnmarshalWithParams(asn1Data, p, "lax")
-			if laxErr != nil {
-				return nil, laxErr
+		if algo == RSAESOAEP {
+			// We only parse the parameters to ensure it is a valid encoding, we throw out the actually values
+			paramsData := keyData.Algorithm.Parameters.FullBytes
+			params := new(rsaesoaepAlgorithmParameters)
+			params.HashFunc = sha1Identifier
+			params.MaskgenFunc = mgf1SHA1Identifier
+			params.PSourceFunc = pSpecifiedEmptyIdentifier
+			rest, err := asn1.Unmarshal(paramsData, params)
+			if err != nil {
+				return nil, err
+			}
+			if len(rest) != 0 {
+				return nil, errors.New("x509: trailing data after RSAES-OAEP parameters")
 			}
-			nfe.AddError(err)
-		}
-		if len(rest) != 0 {
-			return nil, errors.New("x509: trailing data after RSA public key")
-		}
-
-		if p.N.Sign() <= 0 {
-			nfe.AddError(errors.New("x509: RSA modulus is not a positive number"))
-		}
-		if p.E <= 0 {
-			return nil, errors.New("x509: RSA public exponent is not a positive number")
 		}
 
-		pub := &rsa.PublicKey{
-			E: p.E,
-			N: p.N,
-		}
-		return pub, nil
-	case RSAESOAEP:
 		p := new(pkcs1PublicKey)
 		rest, err := asn1.Unmarshal(asn1Data, p)
 		if err != nil {
@@ -1339,16 +1335,8 @@ func parsePublicKey(algo PublicKeyAlgorithm, keyData *publicKeyInfo, nfe *NonFat
 		if p.E <= 0 {
 			return nil, errors.New("x509: RSA public exponent is not a positive number")
 		}
-		paramsData := keyData.Algorithm.Parameters.FullBytes
-		params := new(rsaesoaepAlgorithmParameters)
-		rest, err = asn1.Unmarshal(paramsData, params)
-		if err != nil {
-			return nil, err
-		}
-		if len(rest) != 0 {
-			return nil, errors.New("x509: trailing data after RSAES-OAEP parameters")
-		}
 
+		// TODO(dkarch): Update to return the parameters once crypto/x509 has come up with permanent solution
 		pub := &rsa.PublicKey{
 			E: p.E,
 			N: p.N,

x509/x509_test.go

@@ -1119,11 +1119,15 @@ func TestRSAPSSSelfSigned(t *testing.T) {
 	}
 }
 
-// Valid EKCert with RSAES-OAEP Public Key
+// Valid EKCert (from a TPM)  with RSAES-OAEP Public Key.
+// TPM1.2 uses RSA keys with OAEP padding (SHA1).
+// The hardware only supports SHA1 so manufacturers have
+// since switched to using rsaEncryption keys but millions
+// of certificates still exist that have this type of key.
 var oaepCertDER = `3082056e30820456a003020102020464f5bda5300d06092a864886f70d01010505003077310b3009060355040613024445310f300d060355040813065361786f6e793121301f060355040a1318496e66696e656f6e20546563686e6f6c6f67696573204147310c300a060355040b130341494d312630240603550403131d4946582054504d20454b20496e7465726d656469617465204341203230301e170d3135303932313231323932385a170d3235303932313231323932385a300030820137302206092a864886f70d0101073015a213301106092a864886f70d0101090404544350410382010f003082010a02820101008e7983105063a3f1c2e77d7c8b4f411db9c76f11366ccbb11757001fa51805bbbf68b6dccd9ad28a82c6a831e1591f06181c2e328c261cf9a14ff1704dc3261cc16da751760141969330a75b3fc5ae185ac76d636a97a339838bd042aa7c5999f63f946c6987b0e8be8f5908d08563f62bc6ee9510e36752bd3b2e7b55281bc92a8dcc8ba8a30d6aaa7580b672d83802449d34bfea0434edea4dbc3a9b201f3de0bf5ab2ca96a5254f4e76a58adc8d3bc385be94e93e0052e4066f238a9c1195eaacda02a6dc78393063340054e3ce99754a8770c8efcca45ad8fc999f7aaa67a8a83960141e5f4b892d3af333f09b6d13e60900e0ea8bd3b5eea30f4f2b889b0203010001a38202623082025e30550603551d110101ff044b3049a447304531163014060567810502010c0b69643a343934363538303031173015060567810502020c0c534c42393636307878312e3231123010060567810502030c0769643a30343238300c0603551d130101ff040230003081bc0603551d200101ff0481b13081ae3081ab060b6086480186f84501072f0130819b303906082b06010505070201162d687474703a2f2f7777772e766572697369676e2e636f6d2f7265706f7369746f72792f696e6465782e68746d6c305e06082b0601050507020230521e5000540043005000410020005400720075007300740065006400200050006c006100740066006f0072006d0020004d006f00640075006c006500200045006e0064006f007200730065006d0065006e00743081a10603551d2304819930819680148ffd47880e239a3a3a20de13edf101e882a9d21da17ba4793077310b3009060355040613024445310f300d060355040813065361786f6e793121301f060355040a1318496e66696e656f6e20546563686e6f6c6f67696573204147310c300a060355040b130341494d312630240603550403131d4946582054504d20454b20496e7465726d6564696174652043412032308201053081930603551d0904818b308188303a06035504343133300b300906052b0e03021a05003024302206092a864886f70d0101073015a213301106092a864886f70d010109040454435041301606056781050210310d300b0c03312e32020102020103303206056781050212312930270101ffa0030a0101a1030a0100a2030a0100a310300e1603332e310a01040a01010101ff0101ff300d06092a864886f70d010105050003820101003b25d3958cf08b2c32cefb40570f845e25fa98fd38a47be8c16e4ef663d8f057824ff550052b4a338e353b08bfe70364cda07961d3f4fb5cbf548497389a5ab59c3469d14b6f5bfdc27db3dd0cbd21ac818a64a14032cd433a2bffb939b5f54555c2b6892b5f6479e3c38bd4b30283d2e8ee57ecad779dae90bd9d6052ad6bf3efbb30f639e03b0a239f539a476b129540bc9806e2dc7011d265dca8a95301aaba872b4e176bdb670e65b750b0afdc2ca97b3f0bef55862ae135bd86b1d3a28d4eedfe5ee445a3f1d65bbad0adae49999e613eabb133166461cf47146de078a7d7f550940a1a475ecc834ee784a6a6e42fa4ad7869ef8d2b2251c830efb38496`
 
 func TestParseCertificateWithRSAESOAEPPublicKey(t *testing.T) {
-	expectedKey := &rsa.PublicKey{
+	wantKey := &rsa.PublicKey{
 		E: 65537,
 		N: bigFromHexString("8e7983105063a3f1c2e77d7c8b4f411db9c76f11366ccbb11757001fa51805bbbf68b6dccd9ad28a82c6a831e1591f06181c2e328c261cf9a14ff1704dc3261cc16da751760141969330a75b3fc5ae185ac76d636a97a339838bd042aa7c5999f63f946c6987b0e8be8f5908d08563f62bc6ee9510e36752bd3b2e7b55281bc92a8dcc8ba8a30d6aaa7580b672d83802449d34bfea0434edea4dbc3a9b201f3de0bf5ab2ca96a5254f4e76a58adc8d3bc385be94e93e0052e4066f238a9c1195eaacda02a6dc78393063340054e3ce99754a8770c8efcca45ad8fc999f7aaa67a8a83960141e5f4b892d3af333f09b6d13e60900e0ea8bd3b5eea30f4f2b889b"),
 	}
@@ -1142,8 +1146,8 @@ func TestParseCertificateWithRSAESOAEPPublicKey(t *testing.T) {
 	if !ok {
 		t.Fatalf("Parsed key was not an RSA key: %s", err)
 	}
-	if expectedKey.E != parsedKey.E ||
-		expectedKey.N.Cmp(parsedKey.N) != 0 {
+	if wantKey.E != parsedKey.E ||
+		wantKey.N.Cmp(parsedKey.N) != 0 {
 		t.Fatal("Parsed key differs from expected key")
 	}
 }