diff --git a/plugins/certs/functions b/plugins/certs/functions index 2ace1f924..30736a5a3 100755 --- a/plugins/certs/functions +++ b/plugins/certs/functions @@ -20,7 +20,7 @@ get_ssl_hostnames() { local APP=$1 local SSL_PATH="$DOKKU_ROOT/$APP/tls" - local SSL_HOSTNAME=$(openssl x509 -in "$SSL_PATH/server.crt" -noout -subject | tr '/' '\n' | grep CN= | cut -c4-) + local SSL_HOSTNAME=$(openssl x509 -in "$SSL_PATH/server.crt" -noout -subject -nameopt RFC2253 | sed -n 's/.*CN=\([^,]*\).*/\1/p') local SSL_HOSTNAME_ALT=$(openssl x509 -in "$SSL_PATH/server.crt" -noout -text | grep --after-context=1 '509v3 Subject Alternative Name:' | tail -n 1 | sed -e "s/[[:space:]]*DNS://g" | tr ',' '\n' || true) if [[ -n "$SSL_HOSTNAME_ALT" ]]; then local SSL_HOSTNAMES="${SSL_HOSTNAME}\n${SSL_HOSTNAME_ALT}" diff --git a/plugins/certs/report.go b/plugins/certs/report.go index 02ee5ec15..6bf3f2392 100644 --- a/plugins/certs/report.go +++ b/plugins/certs/report.go @@ -146,15 +146,22 @@ func reportSSLSubject(appName string) string { result, err := common.CallExecCommand(common.ExecCommandInput{ Command: "openssl", - Args: []string{"x509", "-in", filepath.Join(certTLSPath(appName), "server.crt"), "-noout", "-subject"}, + Args: []string{"x509", "-in", filepath.Join(certTLSPath(appName), "server.crt"), "-noout", "-subject", "-nameopt", "compat"}, }) if err != nil { return "" } - subject := strings.Replace(result.StdoutContents(), "subject= ", "", 1) - subject = strings.TrimPrefix(subject, "/") - return strings.ReplaceAll(subject, "/", "; ") + return formatSSLSubject(result.StdoutContents()) +} + +// formatSSLSubject normalizes an openssl "-subject -nameopt compat" line into a +// "; "-joined RDN string. The compat nameopt reproduces the legacy "/"-delimited, +// order-preserving subject form on every OpenSSL/LibreSSL version. +func formatSSLSubject(out string) string { + out = strings.TrimPrefix(strings.TrimSpace(out), "subject=") + out = strings.TrimPrefix(strings.TrimSpace(out), "/") + return strings.ReplaceAll(out, "/", "; ") } func reportSSLVerified(appName string) string { @@ -196,21 +203,45 @@ func reportSSLHostnames(appName string) string { return "" } - hostnameSet := map[string]bool{} - + subject := "" subjectResult, err := common.CallExecCommand(common.ExecCommandInput{ Command: "openssl", - Args: []string{"x509", "-in", filepath.Join(certTLSPath(appName), "server.crt"), "-noout", "-subject"}, + Args: []string{"x509", "-in", filepath.Join(certTLSPath(appName), "server.crt"), "-noout", "-subject", "-nameopt", "RFC2253"}, }) if err == nil { - for _, part := range strings.Split(subjectResult.StdoutContents(), "/") { - if strings.Contains(part, "CN=") && len(part) > 3 { - hostnameSet[part[3:]] = true - } + subject = subjectResult.StdoutContents() + } + + return strings.Join(sslHostnames(subject, opensslCertText(appName)), " ") +} + +// subjectCommonName extracts the CN value from an openssl "-subject" line. It is +// tolerant of the "CN=value" (RFC2253) and "CN = value" (OpenSSL 3.x default) +// renderings, the legacy "/"-delimited compat form, and a leading "subject=" +// prefix. The CN value of a certificate is a hostname, so splitting on "/" as +// well as "," never truncates it. +func subjectCommonName(subject string) string { + subject = strings.TrimPrefix(strings.TrimSpace(subject), "subject=") + for _, rdn := range strings.FieldsFunc(subject, func(r rune) bool { return r == ',' || r == '/' }) { + key, value, found := strings.Cut(rdn, "=") + if found && strings.TrimSpace(key) == "CN" { + return strings.TrimSpace(value) } } - textLines := strings.Split(opensslCertText(appName), "\n") + return "" +} + +// sslHostnames returns the sorted, de-duplicated set of hostnames a certificate +// covers, merging the subject Common Name (from an RFC2253 "-subject" line) with +// every Subject Alternative Name DNS entry (from the "-text" rendering). +func sslHostnames(subject string, certText string) []string { + hostnameSet := map[string]bool{} + if cn := subjectCommonName(subject); cn != "" { + hostnameSet[cn] = true + } + + textLines := strings.Split(certText, "\n") for i, line := range textLines { if strings.Contains(line, "509v3 Subject Alternative Name:") && i+1 < len(textLines) { sanLine := dnsPrefixRegex.ReplaceAllString(textLines[i+1], "") @@ -229,5 +260,5 @@ func reportSSLHostnames(appName string) string { } sort.Strings(hostnames) - return strings.Join(hostnames, " ") + return hostnames } diff --git a/plugins/certs/report_test.go b/plugins/certs/report_test.go new file mode 100644 index 000000000..e5182d097 --- /dev/null +++ b/plugins/certs/report_test.go @@ -0,0 +1,114 @@ +package certs + +import ( + "reflect" + "testing" +) + +func TestSubjectCommonName(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"rfc2253 cn only", "subject=CN=dokku.me", "dokku.me"}, + {"rfc2253 multi rdn reversed", "subject=CN=node-js-app.dokku.me,OU=Operations,O=Expa,L=San Francisco,ST=California,C=US", "node-js-app.dokku.me"}, + {"rfc2253 wildcard", "subject=CN=*.dokku.me", "*.dokku.me"}, + {"openssl 3.x default spaced", "subject=CN = cn-only.example.com", "cn-only.example.com"}, + {"openssl 3.x default multi rdn", "subject=C=US, ST=California, L=San Francisco, O=Expa, OU=Operations, CN=node-js-app.dokku.me", "node-js-app.dokku.me"}, + {"legacy compat slash prefix", "subject=/CN=dokku.me", "dokku.me"}, + {"no subject prefix", "CN=dokku.me", "dokku.me"}, + {"no common name", "subject=OU=Operations,O=Expa,C=US", ""}, + {"empty", "", ""}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := subjectCommonName(tc.in); got != tc.want { + t.Errorf("subjectCommonName(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +const sanCertText = `Certificate: + Data: + X509v3 extensions: + X509v3 Subject Alternative Name: + DNS:www.test.dokku.me, DNS:www.test.app.dokku.me +` + +func TestSSLHostnames(t *testing.T) { + cases := []struct { + name string + subject string + certText string + want []string + }{ + { + name: "cn only no san", + subject: "subject=CN=dokku.me", + want: []string{"dokku.me"}, + }, + { + name: "cn plus sans sorted", + subject: "subject=CN=test.dokku.me", + certText: sanCertText, + want: []string{"test.dokku.me", "www.test.app.dokku.me", "www.test.dokku.me"}, + }, + { + name: "sans only no cn", + subject: "subject=OU=Operations", + certText: sanCertText, + want: []string{"www.test.app.dokku.me", "www.test.dokku.me"}, + }, + { + name: "dedupes cn present in san", + subject: "subject=CN=www.test.dokku.me", + certText: sanCertText, + want: []string{"www.test.app.dokku.me", "www.test.dokku.me"}, + }, + { + name: "no cn no san", + subject: "subject=OU=Operations", + want: []string{}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := sslHostnames(tc.subject, tc.certText); !reflect.DeepEqual(got, tc.want) { + t.Errorf("sslHostnames(%q, ...) = %#v, want %#v", tc.subject, got, tc.want) + } + }) + } +} + +func TestFormatSSLSubject(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"compat single", "subject=/CN=dokku.me", "CN=dokku.me"}, + { + "compat multi rdn", + "subject=/C=US/ST=California/L=San Francisco/O=Expa/OU=Operations/CN=node-js-app.dokku.me", + "C=US; ST=California; L=San Francisco; O=Expa; OU=Operations; CN=node-js-app.dokku.me", + }, + { + "compat wildcard multi rdn", + "subject=/OU=Domain Control Validated/OU=PositiveSSL Wildcard/CN=*.dokku.me", + "OU=Domain Control Validated; OU=PositiveSSL Wildcard; CN=*.dokku.me", + }, + {"legacy spaced prefix", "subject= /CN=dokku.me", "CN=dokku.me"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := formatSSLSubject(tc.in); got != tc.want { + t.Errorf("formatSSLSubject(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} diff --git a/tests.mk b/tests.mk index a5f54a96d..a56a36045 100644 --- a/tests.mk +++ b/tests.mk @@ -146,6 +146,7 @@ lint-shfmt: shfmt lint: lint-shfmt lint-ci ci-go-coverage: + @$(MAKE) ci-go-coverage-plugin PLUGIN_NAME=certs @$(MAKE) ci-go-coverage-plugin PLUGIN_NAME=common @$(MAKE) ci-go-coverage-plugin PLUGIN_NAME=config @$(MAKE) ci-go-coverage-plugin PLUGIN_NAME=network @@ -176,6 +177,7 @@ ci-go-coverage-plugin: (godacov -r ./../../test-results/coverage/$(PLUGIN_NAME).out -c $$CIRCLE_SHA1 -t $$CODACY_TOKEN || true)" || exit $$? go-tests: + @$(MAKE) go-test-plugin PLUGIN_NAME=certs @$(MAKE) go-test-plugin PLUGIN_NAME=common @$(MAKE) go-test-plugin PLUGIN_NAME=config @$(MAKE) go-test-plugin PLUGIN_NAME=network diff --git a/tests/unit/certs.bats b/tests/unit/certs.bats index 2a3c8cc49..eb9d74802 100644 --- a/tests/unit/certs.bats +++ b/tests/unit/certs.bats @@ -80,6 +80,102 @@ teardown() { assert_output_contains "Invalid flag passed" } +@test "(certs:report) reports hostnames and subject for a CN-only certificate" { + run /bin/bash -c "dokku certs:add $TEST_APP $BATS_TMPDIR/tls/server.crt $BATS_TMPDIR/tls/server.key" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku certs:report $TEST_APP --ssl-hostnames" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "dokku.me" + + run /bin/bash -c "dokku certs:report $TEST_APP --ssl-subject" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "CN=dokku.me" +} + +@test "(certs:report) preserves multi-field subject order" { + run /bin/bash -c "dokku certs:add $TEST_APP $BATS_TMPDIR/tls/domain.com.crt $BATS_TMPDIR/tls/domain.com.key" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku certs:report $TEST_APP --ssl-subject" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "C=US; ST=California; L=San Francisco; O=Expa; OU=Operations; CN=node-js-app.dokku.me" + + run /bin/bash -c "dokku certs:report $TEST_APP --ssl-hostnames" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "node-js-app.dokku.me" +} + +@test "(certs:report) reports the CN and all SANs as hostnames" { + local SANS_TLS="$BATS_TMPDIR/tls-sans" + mkdir -p "$SANS_TLS" + tar xf "$BATS_TEST_DIRNAME/server_ssl_sans.tar" -C "$SANS_TLS" + sudo chown -R dokku:dokku "$SANS_TLS" + + run /bin/bash -c "dokku certs:add $TEST_APP $SANS_TLS/server.crt $SANS_TLS/server.key" + echo "output: $output" + echo "status: $status" + assert_success + + run /bin/bash -c "dokku certs:report $TEST_APP --ssl-hostnames" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "test.dokku.me www.test.app.dokku.me www.test.dokku.me" + + rm -rf "$SANS_TLS" +} + +@test "(certs) get_ssl_hostnames parses a CN-only certificate" { + run /bin/bash -c "dokku certs:add $TEST_APP $BATS_TMPDIR/tls/server.crt $BATS_TMPDIR/tls/server.key" + echo "output: $output" + echo "status: $status" + assert_success + + source "$PLUGIN_CORE_AVAILABLE_PATH/certs/functions" + run get_ssl_hostnames "$TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_output "dokku.me" +} + +@test "(certs) get_ssl_hostnames includes the CN and SANs" { + local SANS_TLS="$BATS_TMPDIR/tls-sans" + mkdir -p "$SANS_TLS" + tar xf "$BATS_TEST_DIRNAME/server_ssl_sans.tar" -C "$SANS_TLS" + sudo chown -R dokku:dokku "$SANS_TLS" + + run /bin/bash -c "dokku certs:add $TEST_APP $SANS_TLS/server.crt $SANS_TLS/server.key" + echo "output: $output" + echo "status: $status" + assert_success + + source "$PLUGIN_CORE_AVAILABLE_PATH/certs/functions" + run get_ssl_hostnames "$TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_line "test.dokku.me" + assert_line "www.test.dokku.me" + assert_line "www.test.app.dokku.me" + assert_line_count 3 + + rm -rf "$SANS_TLS" +} + @test "(certs) certs:add" { run /bin/bash -c "dokku certs:add $TEST_APP $BATS_TMPDIR/tls/server.crt $BATS_TMPDIR/tls/server.key" echo "output: $output"