Use utf8 string for private key subject with non-printable characters (#710)

Author: areed

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

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))
+		})
+	}
+}

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 {

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}, "[email protected]")
+		require.NoError(t, err)
+	})
 }
 
 func TestNSSDB_AddPrivateKey(t *testing.T) {

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

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)

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