crypto/x509: Apply code review improvements

This commit is contained in:
Lokesh Kumar 2025-06-05 22:40:18 +02:00
parent d8a6c99394
commit 81ed1389d4
2 changed files with 40 additions and 31 deletions

View File

@ -150,26 +150,24 @@ func parseName(raw cryptobyte.String) (*pkix.RDNSequence, error) {
for !raw.Empty() {
var rdnSet pkix.RelativeDistinguishedNameSET
var set cryptobyte.String
var rawSet cryptobyte.String
if !raw.ReadASN1Element(&rawSet, cryptobyte_asn1.SET) {
if !raw.ReadASN1(&set, cryptobyte_asn1.SET) {
return nil, errors.New("x509: invalid RDNSequence")
}
if !rawSet.ReadASN1(&set, cryptobyte_asn1.SET) {
return nil, errors.New("x509: invalid RDNSequence")
}
var rawAttrs []cryptobyte.String
var prevAttr cryptobyte.String
for !set.Empty() {
var atav cryptobyte.String
var rawAttr cryptobyte.String
if !set.ReadASN1Element(&rawAttr, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: invalid RDNSequence: invalid attribute")
}
rawAttrs = append(rawAttrs, rawAttr)
// Compare each attribute with the previous one
// In DER, they must be in ascending order when compared as octet strings
if prevAttr != nil && bytes.Compare(prevAttr, rawAttr) > 0 {
return nil, errors.New("x509: invalid RDNSequence: SET values not in ascending order")
}
prevAttr = rawAttr
if !rawAttr.ReadASN1(&atav, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: invalid RDNSequence: invalid attribute")
@ -192,18 +190,6 @@ func parseName(raw cryptobyte.String) (*pkix.RDNSequence, error) {
rdnSet = append(rdnSet, attr)
}
// Verify that the SET values are sorted according to DER encoding rules
// as required by X.690 section 11.6
if len(rawAttrs) > 1 {
for i := 1; i < len(rawAttrs); i++ {
// Compare each attribute with the previous one
// In DER, they must be in ascending order when compared as octet strings
if bytes.Compare(rawAttrs[i-1], rawAttrs[i]) > 0 {
return nil, errors.New("x509: invalid RDNSequence: SET values not in ascending order")
}
}
}
rdnSeq = append(rdnSeq, rdnSet)
}

View File

@ -6,7 +6,6 @@ package x509
import (
"encoding/asn1"
"encoding/base64"
"encoding/pem"
"os"
"testing"
@ -255,17 +254,41 @@ d5l1tRhScKu2NBgm74nYmJxJYgvuTA38wGhRrGU=
func TestUnsortedSETInRDN(t *testing.T) {
// This certificate has an unsorted SET in its RDN
certB64 := "MIIFFDCCAvygAwIBAgIUb6hhfTZ9YpBB9FUvC1IUFrL3KAgwDQYJKoZIhvcNAQELBQAwUjELMAkGA1UEBhMCQ04xCzAJBgNVBAgMAkJKMQ0wCwYDVQQKDARKZWZlMRUwEwYDVQQDDAxKZWZlIFJvb3QgQ0ExEDAOBgNVBAcTB0JlaWppbmcwHhcNMjUwNTE2MjEwMjE2WhcNMjYwNTE2MjEwMjE2WjBSMQswCQYDVQQGEwJDTjELMAkGA1UECAwCQkoxDTALBgNVBAoMBEplZmUxFTATBgNVBAMMDEplZmUgUm9vdCBDQTEQMA4GA1UEBxMHQmVpamluZzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAONdnqNcvwTNTKLCJMQzfBW8CjfMRxZI96NU+AYvvwTaSlEXxGY93KD1HsrqXRb4lUhxXVSdbdGGtCwF20zKSoJmcikMW21+9dW6hxkDJVp/E2BKgb1nBJj7d0FgVZyEcjgX2xbHcUdvBJg5IB13MPxcfRfGdHJ8vbA3NFJGdxJgqGb1XQHuU5ql3UGK0UMYHoLAA8ZeUZ7RgdCXAyM2XxF5lXDfzn5/DrlcFbMCLtA4JpbU87QnTIZxWQQ0LLz+FJ/M6sqkTL+CsOWRKXH6TPcyXLCrjuDa7pM/8vVkCX/oeyqwMvYEYV/q+JPHQ34UdhX1g7/OXZh+nGcgV4USOQECAwEAAaOCAQEwgf4wHQYDVR0OBBYEFA2Dg0Oa1UgW3qF3Q6cvq6fvp5wlMIHBBgNVHSMEgbkwgbaAFA2Dg0Oa1UgW3qF3Q6cvq6fvp5wloYGApH4wfDELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDTALBgNVBAoMBEplZmUxGDAWBgNVBAMMD3d3dy5leGFtcGxlLmNvbTEXMBUGCSqGSIb3DQEJARYIQUBCLkMuRE2CFG+oYX02fWKQQfRVLwtSFBay9ygIMAwGA1UdEwQFMAMBAf8wCwYDVR0PBAQDAgEGMBEGCWCGSAGG+EIBAQQEAwIBBjANBgkqhkiG9w0BAQsFAAOCAQEAZkWrFDnDN7aJYxgaLbTxvPQiUEw56GZfYaEH/gHSfkUiWvW8/Ub6Gp0rb/UEwu/9pPvs6QnwqLwBHkBpZX6lF1f5ltBbNzPdFVgQN1GdvETofyqQOo3hRbZ3vfEP7Yro7qXWFmwJwM1lMgTWuPpwxeGOqKR0o8C0dEssPJePAJRQHQHyldQ5Ie96KgLqRjxqx/7A4EQyZ3j3kWGnEY+QiHEEH9SgJ/iVkFuQf479VdMVLgcP9eEF+eKczcHINIGLvYL/9XYxKmfKLIKcZTYpxHdXJRIGLQ27IbXdKeZG0l9+ztLNCkG5fqCDZosfYvN0CIIpkQDQxnPnV4MVOXUhZBVW5Q=="
certPEM := `-----BEGIN CERTIFICATE-----
MIIFFDCCAvygAwIBAgIUb6hhfTZ9YpBB9FUvC1IUFrL3KAgwDQYJKoZIhvcNAQEL
BQAwUjELMAkGA1UEBhMCQ04xCzAJBgNVBAgMAkJKMQ0wCwYDVQQKDARKZWZlMRUw
EwYDVQQDDAxKZWZlIFJvb3QgQ0ExEDAOBgNVBAcTB0JlaWppbmcwHhcNMjUwNTE2
MjEwMjE2WhcNMjYwNTE2MjEwMjE2WjBSMQswCQYDVQQGEwJDTjELMAkGA1UECAwC
QkoxDTALBgNVBAoMBEplZmUxFTATBgNVBAMMDEplZmUgUm9vdCBDQTEQMA4GA1UE
BxMHQmVpamluZzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAONdnqNc
vwTNTKLCJMQzfBW8CjfMRxZI96NU+AYvvwTaSlEXxGY93KD1HsrqXRb4lUhxXVSd
bdGGtCwF20zKSoJmcikMW21+9dW6hxkDJVp/E2BKgb1nBJj7d0FgVZyEcjgX2xbH
cUdvBJg5IB13MPxcfRfGdHJ8vbA3NFJGdxJgqGb1XQHuU5ql3UGK0UMYHoLAA8Ze
UZ7RgdCXAyM2XxF5lXDfzn5/DrlcFbMCLtA4JpbU87QnTIZxWQQ0LLz+FJ/M6sqk
TL+CsOWRKXH6TPcyXLCrjuDa7pM/8vVkCX/oeyqwMvYEYV/q+JPHQ34UdhX1g7/O
XZh+nGcgV4USOQECAwEAAaOCAQEwgf4wHQYDVR0OBBYEFA2Dg0Oa1UgW3qF3Q6cv
q6fvp5wlMIHBBgNVHSMEgbkwgbaAFA2Dg0Oa1UgW3qF3Q6cvq6fvp5wloYGApH4w
fDELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcMDVNh
biBGcmFuY2lzY28xDTALBgNVBAoMBEplZmUxGDAWBgNVBAMMD3d3dy5leGFtcGxl
LmNvbTEXMBUGCSqGSIb3DQEJARYIQUBCLkMuRE2CFG+oYX02fWKQQfRVLwtSFBay
9ygIMAwGA1UdEwQFMAMBAf8wCwYDVR0PBAQDAgEGMBEGCWCGSAGG+EIBAQQEAwIB
BjANBgkqhkiG9w0BAQsFAAOCAQEAZkWrFDnDN7aJYxgaLbTxvPQiUEw56GZfYaEH
/gHSfkUiWvW8/Ub6Gp0rb/UEwu/9pPvs6QnwqLwBHkBpZX6lF1f5ltBbNzPdFVgQ
N1GdvETofyqQOo3hRbZ3vfEP7Yro7qXWFmwJwM1lMgTWuPpwxeGOqKR0o8C0dEss
PJePAJRQHQHyldQ5Ie96KgLqRjxqx/7A4EQyZ3j3kWGnEY+QiHEEH9SgJ/iVkFuQ
f479VdMVLgcP9eEF+eKczcHINIGLvYL/9XYxKmfKLIKcZTYpxHdXJRIGLQ27IbXd
KeZG0l9+ztLNCkG5fqCDZosfYvN0CIIpkQDQxnPnV4MVOXUhZBVW5Q==
-----END CERTIFICATE-----`
der, err := base64.StdEncoding.DecodeString(certB64)
if err != nil {
t.Fatalf("Failed to decode certificate: %v", err)
block, _ := pem.Decode([]byte(certPEM))
if block == nil {
t.Fatalf("Failed to decode PEM block")
}
_, err = ParseCertificate(der)
_, err := ParseCertificate(block.Bytes)
if err == nil {
t.Errorf("Expected ParseCertificate to fail due to unsorted SET values in RDN, but it succeeded")
} else if err.Error() != "x509: malformed certificate" {
t.Errorf("Expected error 'x509: malformed certificate', got: %v", err)
}
}
}