updated per code review

Signed-off-by: Patrick Zheng <[email protected]>

cmd/notation/sign.go

@@ -230,7 +230,10 @@ func prepareSigningOpts(ctx context.Context, opts *signOpts) (notation.SignOptio
 			return notation.SignOptions{}, err
 		}
 		if len(rootCerts) == 0 {
-			return notation.SignOptions{}, fmt.Errorf("cannot find any tsa root certificate from %q. Expecting x509 certificate in PEM or DER format from the file", opts.tsaRootCertificatePath)
+			return notation.SignOptions{}, fmt.Errorf("cannot find any certificate from %q. Expecting one x509 certificate in PEM or DER format from the file", opts.tsaRootCertificatePath)
+		}
+		if len(rootCerts) > 1 {
+			return notation.SignOptions{}, fmt.Errorf("find more than one certificates from %q. Expecting one x509 certificate in PEM or DER format from the file", opts.tsaRootCertificatePath)
 		}
 		rootCAs := x509.NewCertPool()
 		rootCAs.AddCert(rootCerts[0])

internal/testdata/intermediate.pem

@@ -0,0 +1,17 @@
+-----BEGIN CERTIFICATE-----
+MIICyjCCAbKgAwIBAgIBATANBgkqhkiG9w0BAQsFADAPMQ0wCwYDVQQDDARSb290
+MCAXDTIyMDYzMDE5MjAwM1oYDzMwMjExMDMxMTkyMDAzWjAYMRYwFAYDVQQDDA1J
+bnRlcm1lZGlhdGUxMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1JTs
+aiC/7+bho43kMVyHDwCsuocYp4PvYahB59NsKDR4QbrImU5ziaQ94D0DQqthe9pm
+qOW0SxN/vSRJAZFELxacrB9hc1y4MjiDYaRSt/LVx7astylBV/QRpmxWSEqp0Avu
+6nMJivIa1sD0WIEchizx6jG9BI5ULr9LbJICYvMgDalQR+0JGG+rKWnf1mPZyxEu
+9zEh215LCg5K56P3W5kC8fKBXSdSgTqZAvHzp6u78qet9S8gARtOEfS03A/7y7MC
+U0Sn2wdQyQdci0PBsR2sTZvUw179Cr93r5aRbb3I6jXgMWHAP2vvIndb9CM9ePyY
+yEy4Je7oWVVfMQ3CWQIDAQABoyYwJDASBgNVHRMBAf8ECDAGAQH/AgEBMA4GA1Ud
+DwEB/wQEAwICBDANBgkqhkiG9w0BAQsFAAOCAQEALR0apUQVbWGmagLUz4Y/bRsl
+mY9EJJXCiLuSxVWd3offjZfQTlGkQkCAW9FOQnm7JhEtaaHF1+AEVLo56/Gsd/hk
+sXsrBagYGi72jun7QTb6j7iZ3X9zanrP3SjdkpjVnqxRfH83diSh0r68Xruq1NSK
+qhUy1V+KQaXF0SSEutPqdTCoXUyxyXohVLU78uqZX/jx9Nc1XDuW9AZd+hMsLdk8
+qGJqHYFvj2vOHGMTeYk8dWgMBthQeL0wdsg2AvKtAvn6FQXCN7mKCWjpFTtYsU8v
+NsesS9M/i+geJjR/8/DDT3RP7S100BtCMm4XfHfmKcjXVaBh5evQVqGsa6TKLw==
+-----END CERTIFICATE-----

internal/testdata/self-signed.crt

@@ -0,0 +1,20 @@
+-----BEGIN CERTIFICATE-----
+MIIDPjCCAiagAwIBAgIBeTANBgkqhkiG9w0BAQsFADBOMQswCQYDVQQGEwJVUzEL
+MAkGA1UECBMCV0ExEDAOBgNVBAcTB1NlYXR0bGUxDzANBgNVBAoTBk5vdGFyeTEP
+MA0GA1UEAxMGYWxwaW5lMB4XDTIzMDUwOTA0NTUxMloXDTMzMDUxMDA0NTUxMlow
+TjELMAkGA1UEBhMCVVMxCzAJBgNVBAgTAldBMRAwDgYDVQQHEwdTZWF0dGxlMQ8w
+DQYDVQQKEwZOb3RhcnkxDzANBgNVBAMTBmFscGluZTCCASIwDQYJKoZIhvcNAQEB
+BQADggEPADCCAQoCggEBAK5hpq1229GGLjMK6i9KZhuUO+SV7rUFnWIDiIPO5yWx
+YDkl+bGroeAvJYu6MVCMQ6FMRXD9jhnG6R+sAHwY7gVgcJ1OXak87PkLp/Ii1Cr7
+XkkySZeD+Br1vSQzfxs3pFG+iBCeVVkeZdsg+xqwnAlqAILXwIbTGRyJP1Xiu9nw
+OeuX1YmxPl2m29Pt1EtfVCL9COsVKt5LgOVyWP/9ISWevOBqSCU9bk35HFo9VTeU
+f6+ffhSMjv0Y9uwkFFOKXpcV8Sa3ArqyBmgQlUfGg1iwYlqiDE0fTYxiB3gLgETA
+lmTm50J+WB9LoDrnrQpbXFLoegm+JV+uSD8J8H7DL2sCAwEAAaMnMCUwDgYDVR0P
+AQH/BAQDAgeAMBMGA1UdJQQMMAoGCCsGAQUFBwMDMA0GCSqGSIb3DQEBCwUAA4IB
+AQAt0Nvna1c4pPn8kzoN5VvmFmeIgdO/BJpmdhdg0WIQ9aeN/xPXXaVjPp1Mk7ed
+XHAvBwQr0Gyzqyy7g/h0gdnAFG7f6blrRNzbrRBCq6cNqX8iwgK/9+2OYKxk1QWj
+8Gx0cvu1DN1aXjPPGgQ2j3tHjJvJv32J/zuZa8gU40RPPSLaBlc5ZjpFmyi29sKl
+TeeZ+F/Ssic51qXXw2CsYGGWK5yQ3xSCxbw6bb2G/s/YI7/KlWg9BktBJHzRu04Z
+NR77W7/dyJ3Lj17PlW1XKmMOFHsQivagXeRCbmYZ43fX4ugFRFKL7KE0EgmGOWpJ
+0xv+6ig93sqHzQ/0uv1YgFov
+-----END CERTIFICATE-----

internal/x509/cert.go

@@ -0,0 +1,30 @@
+// Copyright The Notary Project Authors.
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package x509
+
+import (
+	"bytes"
+	"crypto/x509"
+)
+
+// IsRootCertificate returns true if cert is a root certificate.
+// A root certificate MUST be a self-signed and self-issued CA certificate with
+// valid BasicConstraints.
+func IsRootCertificate(cert *x509.Certificate) (bool, error) {
+	// CheckSignatureFrom also checks cert.BasicConstraintsValid
+	if err := cert.CheckSignatureFrom(cert); err != nil {
+		return false, err
+	}
+	return cert.IsCA && bytes.Equal(cert.RawSubject, cert.RawIssuer), nil
+}

internal/x509/cert_test.go

@@ -0,0 +1,54 @@
+// Copyright The Notary Project Authors.
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package x509
+
+import (
+	"testing"
+
+	corex509 "github.com/notaryproject/notation-core-go/x509"
+)
+
+func TestIsRootCertificate(t *testing.T) {
+	tsaRoot, err := corex509.ReadCertificateFile("../testdata/tsaRootCA.cer")
+	if err != nil {
+		t.Fatal(err)
+	}
+	isRoot, err := IsRootCertificate(tsaRoot[0])
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !isRoot {
+		t.Fatal("expected IsRootCertificate to return true")
+	}
+
+	intermediate, err := corex509.ReadCertificateFile("../testdata/intermediate.pem")
+	if err != nil {
+		t.Fatal(err)
+	}
+	expectedErrMsg := "crypto/rsa: verification error"
+	_, err = IsRootCertificate(intermediate[0])
+	if err == nil || err.Error() != expectedErrMsg {
+		t.Fatalf("expected %s, but got %s", expectedErrMsg, err)
+	}
+
+	selfSigned, err := corex509.ReadCertificateFile("../testdata/self-signed.crt")
+	if err != nil {
+		t.Fatal(err)
+	}
+	expectedErrMsg = "x509: invalid signature: parent certificate cannot sign this kind of certificate"
+	_, err = IsRootCertificate(selfSigned[0])
+	if err == nil || err.Error() != expectedErrMsg {
+		t.Fatalf("expected %s, but got %s", expectedErrMsg, err)
+	}
+}

test/e2e/suite/command/sign.go

@@ -308,4 +308,20 @@ var _ = Describe("notation sign", func() {
 				MatchErrKeyWords("Error: x509: malformed certificate")
 		})
 	})
+
+	It("with more than certificates in tsa root certificate file", func() {
+		Host(BaseOptions(), func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) {
+			notation.ExpectFailure().Exec("sign", "--timestamp-url", "http://timestamp.digicert.com", "--timestamp-root-cert", filepath.Join(NotationE2EConfigPath, "timestamp", "CertChain.pem"), artifact.ReferenceWithDigest()).
+				MatchErrKeyWords("find more than one certificates").
+				MatchErrKeyWords("Expecting one x509 certificate in PEM or DER format from the file")
+		})
+	})
+
+	It("with empty tsa root certificate file", func() {
+		Host(BaseOptions(), func(notation *utils.ExecOpts, artifact *Artifact, vhost *utils.VirtualHost) {
+			notation.ExpectFailure().Exec("sign", "--timestamp-url", "http://timestamp.digicert.com", "--timestamp-root-cert", filepath.Join(NotationE2EConfigPath, "timestamp", "empty.txt"), artifact.ReferenceWithDigest()).
+				MatchErrKeyWords("cannot find any certificate from").
+				MatchErrKeyWords("Expecting one x509 certificate in PEM or DER format from the file")
+		})
+	})
 })

test/e2e/testdata/config/timestamp/CertChain.pem

@@ -0,0 +1,22 @@
+-----BEGIN CERTIFICATE-----
+MIIBljCCATugAwIBAgIQJOyDt70f+HOyQMEt06yvpjAKBggqhkjOPQQDAjAkMRAw
+DgYDVQQKEwdBY21lIENvMRAwDgYDVQQDEwdSb290IENBMB4XDTIyMDcyMjA1MjEz
+N1oXDTIzMDcyMjA1MjEzN1owKDEQMA4GA1UEChMHQWNtZSBDbzEUMBIGA1UEAwwL
+dGVzdF9jZXJ0XzEwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATnf3lSRtUYOeph
+UQZvUm5niB8kpm7kn6iAm2zwCTBeqKbUtgESCbN+x6TTpWZIaEo+CDu1rPUdicB3
+FUwXNzz8o0swSTAOBgNVHQ8BAf8EBAMCBaAwEwYDVR0lBAwwCgYIKwYBBQUHAwEw
+DAYDVR0TAQH/BAIwADAUBgNVHREEDTALgglsb2NhbGhvc3QwCgYIKoZIzj0EAwID
+SQAwRgIhAMgdV/zJnwK0J4ZBXZVwAB6abpgNcESFScDeQQyIzRs8AiEAjjLTfkXp
+CuoXnu5/hYy6Li7Smw3UbW3XKkekOELMFYo=
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIBnjCCAUOgAwIBAgIQBUvhbMcjM35qmJzncyZ5tzAKBggqhkjOPQQDAjAkMRAw
+DgYDVQQKEwdBY21lIENvMRAwDgYDVQQDEwdSb290IENBMB4XDTIyMDcyMjA1MjEz
+N1oXDTIzMDcyMjA1MjEzN1owJDEQMA4GA1UEChMHQWNtZSBDbzEQMA4GA1UEAxMH
+Um9vdCBDQTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABGmby5GUiBDR+Ge+s/R9
+EqOfoDwEdDBPYU0emJg8j8CPJGM0ldalI1Sk7YMTIi34clvfTqEixE7nDwQj8FjQ
+VvCjVzBVMA4GA1UdDwEB/wQEAwICBDATBgNVHSUEDDAKBggrBgEFBQcDATAPBgNV
+HRMBAf8EBTADAQH/MB0GA1UdDgQWBBTcOHZMx0z3I9Hi8oa2Kp0umdXOsTAKBggq
+hkjOPQQDAgNJADBGAiEArHTaO3f6vaiI+4IOrR7SYSzeHIAqoFAWFcf1yOzxDA4C
+IQDRcDIPWJd7pXvFJT/Q++Vkq9QuUhqrigCQDkgksnxf5w==
+-----END CERTIFICATE-----