Repository: smallstep/cli
Pull request #799 (2022-11-14 10:28:01)

Fix `mergeSans` to include the CSR URIs
This fixes the root cause of https://github.com/smallstep/certificates/issues/1181.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9c142934..8fb6c564 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -28,6 +28,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
+### Fixed
+
+- Generation of OTT when signing a CSR with URIs.
+
 ## [v0.23.0] - 2022-11-11
 
 ### Added
diff --git a/command/ca/sign.go b/command/ca/sign.go
index 592de47f..e973d3b4 100644
--- a/command/ca/sign.go
+++ b/command/ca/sign.go
@@ -185,7 +185,8 @@ func signCertificateAction(ctx *cli.Context) error {
 		if ctx.IsSet("acme") {
 			return cautils.ACMESignCSRFlow(ctx, csr, crtFile, "")
 		}
-		sans := mergeSans(ctx, csr)
+		sans := ctx.StringSlice("san")
+		sans = mergeSans(sans, csr)
 		if tok, err = flow.GenerateToken(ctx, csr.Subject.CommonName, sans); err != nil {
 			var acmeTokenErr *cautils.ACMETokenError
 			if errors.As(err, &acmeTokenErr) {
@@ -219,10 +220,10 @@ func signCertificateAction(ctx *cli.Context) error {
 	return nil
 }
 
-func mergeSans(ctx *cli.Context, csr *x509.CertificateRequest) []string {
+func mergeSans(sans []string, csr *x509.CertificateRequest) []string {
 	uniq := make([]string, 0)
 	m := make(map[string]bool)
-	for _, s := range ctx.StringSlice("san") {
+	for _, s := range sans {
 		if _, ok := m[s]; !ok {
 			uniq = append(uniq, s)
 			m[s] = true
@@ -247,5 +248,12 @@ func mergeSans(ctx *cli.Context, csr *x509.CertificateRequest) []string {
 			m[s] = true
 		}
 	}
+	for _, u := range csr.URIs {
+		s := u.String()
+		if _, ok := m[s]; !ok {
+			uniq = append(uniq, s)
+			m[s] = true
+		}
+	}
 	return uniq
 }
diff --git a/go.mod b/go.mod
index 7fc9fb66..6830dbeb 100644
--- a/go.mod
+++ b/go.mod
@@ -6,6 +6,7 @@ require (
 	github.com/Microsoft/go-winio v0.6.0
 	github.com/ThomasRooney/gexpect v0.0.0-20161231170123-5482f0350944
 	github.com/fxamacker/cbor/v2 v2.4.0
+	github.com/google/go-cmp v0.5.9
 	github.com/google/uuid v1.3.0
 	github.com/icrowley/fake v0.0.0-20180203215853-4178557ae428
 	github.com/manifoldco/promptui v0.9.0
@@ -98,7 +99,6 @@ require (
 	github.com/golang/snappy v0.0.4 // indirect
 	github.com/google/btree v1.0.1 // indirect
 	github.com/google/certificate-transparency-go v1.1.3 // indirect
-	github.com/google/go-cmp v0.5.9 // indirect
 	github.com/google/trillian v1.4.1 // indirect
 	github.com/googleapis/enterprise-certificate-proxy v0.2.0 // indirect
 	github.com/googleapis/gax-go/v2 v2.7.0 // indirect


Test patch:
diff --git a/command/ca/sign_test.go b/command/ca/sign_test.go
new file mode 100644
index 00000000..24c1c1f8
--- /dev/null
+++ b/command/ca/sign_test.go
@@ -0,0 +1,87 @@
+package ca
+
+import (
+	"crypto/x509"
+	"net"
+	"net/url"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func mustParseURI(t *testing.T, u string) (parsed *url.URL) {
+	t.Helper()
+	parsed, err := url.Parse(u)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return
+}
+
+func Test_mergeSans(t *testing.T) {
+	type args struct {
+		sans []string
+		csr  *x509.CertificateRequest
+	}
+	tests := []struct {
+		name string
+		args args
+		want []string
+	}{
+		{
+			name: "empty",
+			args: args{
+				sans: nil,
+				csr:  &x509.CertificateRequest{},
+			},
+			want: []string{},
+		},
+		{
+			name: "context-only",
+			args: args{
+				sans: []string{"www.example.com"},
+				csr:  &x509.CertificateRequest{},
+			},
+			want: []string{"www.example.com"},
+		},
+		{
+			name: "csr-only",
+			args: args{
+				sans: nil,
+				csr: &x509.CertificateRequest{
+					DNSNames: []string{"www.example.com"},
+				},
+			},
+			want: []string{"www.example.com"},
+		},
+		{
+			name: "full",
+			args: args{
+				sans: []string{"www1.test.local", "mail+1@local", "127.0.0.1", "https://www1.test.local", "www1.test.local", "mail+1@local", "127.0.0.1", "https://www1.test.local"},
+				csr: &x509.CertificateRequest{
+					DNSNames: []string{"www.example.com", "www1.test.local", "www.example.com"},
+					IPAddresses: []net.IP{
+						net.ParseIP("127.0.0.2"),
+						net.ParseIP("127.0.0.1"),
+						net.ParseIP("127.0.0.2"),
+					},
+					EmailAddresses: []string{"mail+2@local", "mail+1@local", "mail+2@local"},
+					URIs: []*url.URL{
+						mustParseURI(t, "https://www2.test.local"),
+						mustParseURI(t, "https://www1.test.local"),
+						mustParseURI(t, "https://www2.test.local"),
+					},
+				},
+			},
+			want: []string{"www1.test.local", "mail+1@local", "127.0.0.1", "https://www1.test.local",
+				"www.example.com", "127.0.0.2", "mail+2@local", "https://www2.test.local"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := mergeSans(tt.args.sans, tt.args.csr); !cmp.Equal(tt.want, got) {
+				t.Errorf("mergeSans() diff =\n%s", cmp.Diff(tt.want, got))
+			}
+		})
+	}
+}