diff --git a/internal/utils/asn1.go b/internal/utils/asn1.go new file mode 100644 index 00000000..798dbee8 --- /dev/null +++ b/internal/utils/asn1.go @@ -0,0 +1,33 @@ +package utils + +// IsPrintableString reports whether the given s is a valid ASN.1 PrintableString. +// If asterisk is allowAsterisk then '*' is also allowed, reflecting existing +// practice. If ampersand is allowAmpersand then '&' is allowed as well. +func IsPrintableString(s string, asterisk, ampersand bool) bool { + for _, b := range s { + valid := 'a' <= b && b <= 'z' || + 'A' <= b && b <= 'Z' || + '0' <= b && b <= '9' || + '\'' <= b && b <= ')' || + '+' <= b && b <= '/' || + b == ' ' || + b == ':' || + b == '=' || + b == '?' || + // This is technically not allowed in a PrintableString. + // However, x509 certificates with wildcard strings don't + // always use the correct string type so we permit it. + (asterisk && b == '*') || + // This is not technically allowed either. However, not + // only is it relatively common, but there are also a + // handful of CA certificates that contain it. At least + // one of which will not expire until 2027. + (ampersand && b == '&') + + if !valid { + return false + } + } + + return true +} diff --git a/internal/utils/asn1_test.go b/internal/utils/asn1_test.go new file mode 100644 index 00000000..bbd8704f --- /dev/null +++ b/internal/utils/asn1_test.go @@ -0,0 +1,34 @@ +package utils + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsPrintableString(t *testing.T) { + type args struct { + s string + asterisk bool + ampersand bool + } + tests := []struct { + name string + args args + want bool + }{ + {"empty", args{"", false, false}, true}, + {"a", args{"a", false, false}, true}, + {"spaces and caps", args{"My Leaf", false, false}, true}, + {"default allowed punctuation", args{`(Hi+,-./):=?`, false, false}, true}, + {"asterisk not allowed", args{"*", false, false}, false}, + {"ampersand not allowed", args{"&", false, false}, false}, + {"asterisk allowed", args{"*", true, false}, true}, + {"ampersand allowed", args{"&", false, true}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, IsPrintableString(tt.args.s, tt.args.asterisk, tt.args.ampersand)) + }) + } +} diff --git a/nssdb/keys.go b/nssdb/keys.go index aece23ea..16f36709 100644 --- a/nssdb/keys.go +++ b/nssdb/keys.go @@ -10,6 +10,7 @@ import ( "fmt" "math/big" + "go.step.sm/crypto/internal/utils" "golang.org/x/crypto/cryptobyte" ) @@ -99,8 +100,13 @@ type certCN struct { CommonName string `asn1:"printable"` } +type certCNUTF8 struct { + OID asn1.ObjectIdentifier + CommonName string `asn1:"utf8"` +} + type privateKeySubject struct { - List []certCN `asn1:"set"` + List []any `asn1:"set"` } func ecPrivKeyToObject(priv *ecdsa.PrivateKey, name string, id []byte, certCNs ...string) (*Object, error) { @@ -141,10 +147,17 @@ func ecPrivKeyToObject(priv *ecdsa.PrivateKey, name string, id []byte, certCNs . if len(certCNs) > 0 { sub := privateKeySubject{} for _, cn := range certCNs { - sub.List = append(sub.List, certCN{ - OID: asn1.ObjectIdentifier{2, 5, 4, 3}, - CommonName: cn, - }) + if utils.IsPrintableString(cn, false, false) { + sub.List = append(sub.List, certCN{ + OID: asn1.ObjectIdentifier{2, 5, 4, 3}, + CommonName: cn, + }) + } else { + sub.List = append(sub.List, certCNUTF8{ + OID: asn1.ObjectIdentifier{2, 5, 4, 3}, + CommonName: cn, + }) + } } subASN1, err := asn1.Marshal(sub) if err != nil { diff --git a/nssdb/keys_test.go b/nssdb/keys_test.go index 5e3981dc..b507dabd 100644 --- a/nssdb/keys_test.go +++ b/nssdb/keys_test.go @@ -67,12 +67,19 @@ func TestEcPrivKeyToObject(t *testing.T) { ecdsaPrivKey, ok := privKey.(*ecdsa.PrivateKey) require.True(t, ok) - obj, err := ecPrivKeyToObject(ecdsaPrivKey, "leafkey", []byte{7}, "leaf") - require.NoError(t, err) - assert.NoError(t, obj.ValidateULong("CKA_CLASS", CKO_PRIVATE_KEY)) - sub, err := hex.DecodeString("300f310d300b060355040313046c656166") - require.NoError(t, err) - assert.NoError(t, obj.Validate("CKA_SUBJECT", sub)) + t.Run("printable subject", func(t *testing.T) { + obj, err := ecPrivKeyToObject(ecdsaPrivKey, "leafkey", []byte{7}, "leaf") + require.NoError(t, err) + assert.NoError(t, obj.ValidateULong("CKA_CLASS", CKO_PRIVATE_KEY)) + sub, err := hex.DecodeString("300f310d300b060355040313046c656166") + require.NoError(t, err) + assert.NoError(t, obj.Validate("CKA_SUBJECT", sub)) + }) + + t.Run("utf8 subject", func(t *testing.T) { + _, err := ecPrivKeyToObject(ecdsaPrivKey, "leafkey", []byte{7}, "andrew@smallstep.com") + require.NoError(t, err) + }) } func TestNSSDB_AddPrivateKey(t *testing.T) { diff --git a/x509util/certificate_request.go b/x509util/certificate_request.go index b2ade54a..f0ecbf07 100644 --- a/x509util/certificate_request.go +++ b/x509util/certificate_request.go @@ -10,6 +10,7 @@ import ( "encoding/json" "github.com/pkg/errors" + "go.step.sm/crypto/internal/utils" "golang.org/x/crypto/cryptobyte" cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1" ) @@ -176,7 +177,7 @@ func (c *CertificateRequest) addChallengePassword(asn1Data []byte) ([]byte, erro child.AddASN1ObjectIdentifier(oidChallengePassword) child.AddASN1(cryptobyte_asn1.SET, func(value *cryptobyte.Builder) { switch { - case isPrintableString(c.ChallengePassword, true, true): + case utils.IsPrintableString(c.ChallengePassword, true, true): value.AddASN1(cryptobyte_asn1.PrintableString, func(s *cryptobyte.Builder) { s.AddBytes([]byte(c.ChallengePassword)) }) diff --git a/x509util/extensions.go b/x509util/extensions.go index c2075a70..5f513436 100644 --- a/x509util/extensions.go +++ b/x509util/extensions.go @@ -16,6 +16,7 @@ import ( "time" "github.com/pkg/errors" + "go.step.sm/crypto/internal/utils" ) func convertName(s string) string { @@ -559,7 +560,7 @@ func marshalValue(value, params string) ([]byte, error) { } return asn1.MarshalWithParams(value, p.Params) case "printable": - if !isPrintableString(value, true, true) { + if !utils.IsPrintableString(value, true, true) { return nil, fmt.Errorf("invalid printable value") } return asn1.MarshalWithParams(value, p.Params) @@ -581,7 +582,7 @@ func marshalValue(value, params string) ([]byte, error) { } return asn1.MarshalWithParams(b, p.Params) default: // if it's an unknown type, default to printable - if !isPrintableString(value, true, true) { + if !utils.IsPrintableString(value, true, true) { return nil, fmt.Errorf("invalid printable value") } return asn1.MarshalWithParams(value, p.Params) diff --git a/x509util/utils.go b/x509util/utils.go index 55fe7e8a..bb7cba17 100644 --- a/x509util/utils.go +++ b/x509util/utils.go @@ -179,35 +179,3 @@ func isNumericString(s string) bool { return true } - -// isPrintableString reports whether the given s is a valid ASN.1 PrintableString. -// If asterisk is allowAsterisk then '*' is also allowed, reflecting existing -// practice. If ampersand is allowAmpersand then '&' is allowed as well. -func isPrintableString(s string, asterisk, ampersand bool) bool { - for _, b := range s { - valid := 'a' <= b && b <= 'z' || - 'A' <= b && b <= 'Z' || - '0' <= b && b <= '9' || - '\'' <= b && b <= ')' || - '+' <= b && b <= '/' || - b == ' ' || - b == ':' || - b == '=' || - b == '?' || - // This is technically not allowed in a PrintableString. - // However, x509 certificates with wildcard strings don't - // always use the correct string type so we permit it. - (asterisk && b == '*') || - // This is not technically allowed either. However, not - // only is it relatively common, but there are also a - // handful of CA certificates that contain it. At least - // one of which will not expire until 2027. - (ampersand && b == '&') - - if !valid { - return false - } - } - - return true -}