diff --git a/client/client.go b/client/client.go index d1586a707d..eafedef333 100644 --- a/client/client.go +++ b/client/client.go @@ -881,12 +881,7 @@ func (r *NotaryRepository) validateRoot(rootJSON []byte, fromRemote bool) (*data } } } - err = trustpinning.ValidateRoot(prevRoot, root, r.gun, r.trustPinning) - if err != nil { - return nil, err - } - - return data.RootFromSigned(root) + return trustpinning.ValidateRoot(prevRoot, root, r.gun, r.trustPinning) } // RotateKey removes all existing keys associated with the role, and either diff --git a/client/client_update_test.go b/client/client_update_test.go index 5051e40dfd..9f19d7ba55 100644 --- a/client/client_update_test.go +++ b/client/client_update_test.go @@ -1374,7 +1374,7 @@ func TestValidateRootRotationWithOldRole(t *testing.T) { keyIDs := make([]string, len(threeKeys)) for i := 0; i < len(threeKeys); i++ { threeKeys[i], err = testutils.CreateKey( - serverSwizzler.CryptoService, "docker.com/notary", data.CanonicalRootRole) + serverSwizzler.CryptoService, "docker.com/notary", data.CanonicalRootRole, data.ECDSAKey) require.NoError(t, err) keyIDs[i] = threeKeys[i].ID() signedRoot.Signed.Keys[keyIDs[i]] = threeKeys[i] @@ -1392,7 +1392,7 @@ func TestValidateRootRotationWithOldRole(t *testing.T) { // --- threshold back to 1 replacementKey, err := testutils.CreateKey( - serverSwizzler.CryptoService, "docker.com/notary", data.CanonicalRootRole) + serverSwizzler.CryptoService, "docker.com/notary", data.CanonicalRootRole, data.ECDSAKey) require.NoError(t, err) signedRoot.Signed.Version++ signedRoot.Signed.Keys[replacementKey.ID()] = replacementKey @@ -1416,7 +1416,7 @@ func TestValidateRootRotationWithOldRole(t *testing.T) { // --- latest root role) signedRoot.Signed.Version++ snapKey, err := testutils.CreateKey( - serverSwizzler.CryptoService, "docker.com/notary", data.CanonicalSnapshotRole) + serverSwizzler.CryptoService, "docker.com/notary", data.CanonicalSnapshotRole, data.ECDSAKey) require.NoError(t, err) signedRoot.Signed.Keys[snapKey.ID()] = snapKey signedRoot.Signed.Roles[data.CanonicalSnapshotRole].KeyIDs = []string{snapKey.ID()} diff --git a/trustpinning/certs.go b/trustpinning/certs.go index e327e51514..a5fb291d0a 100644 --- a/trustpinning/certs.go +++ b/trustpinning/certs.go @@ -53,8 +53,9 @@ func prettyFormatCertIDs(certs []*x509.Certificate) string { ValidateRoot receives a new root, validates its correctness and attempts to do root key rotation if needed. -First we list the current trusted certificates we have for a particular GUN. If -that list is non-empty means that we've already seen this repository before, and +First we check if we have any trusted certificates for a particular GUN in +a previous root, if we have one. If the previous root is not nil and we find +certificates for this GUN, we've already seen this repository before, and have a list of trusted certificates for it. In this case, we use this list of certificates to attempt to validate this root file. @@ -86,16 +87,16 @@ We shall call this: TOFUS. Validation failure at any step will result in an ErrValidationFailed error. */ -func ValidateRoot(prevRoot *data.SignedRoot, root *data.Signed, gun string, trustPinning TrustPinConfig) error { +func ValidateRoot(prevRoot *data.SignedRoot, root *data.Signed, gun string, trustPinning TrustPinConfig) (*data.SignedRoot, error) { logrus.Debugf("entered ValidateRoot with dns: %s", gun) signedRoot, err := data.RootFromSigned(root) if err != nil { - return err + return nil, err } rootRole, err := signedRoot.BuildBaseRole(data.CanonicalRootRole) if err != nil { - return err + return nil, err } // Retrieve all the leaf and intermediate certificates in root for which the CN matches the GUN @@ -103,29 +104,38 @@ func ValidateRoot(prevRoot *data.SignedRoot, root *data.Signed, gun string, trus certsFromRoot, err := validRootLeafCerts(allLeafCerts, gun, true) if err != nil { logrus.Debugf("error retrieving valid leaf certificates for: %s, %v", gun, err) - return &ErrValidationFail{Reason: "unable to retrieve valid leaf certificates"} + return nil, &ErrValidationFail{Reason: "unable to retrieve valid leaf certificates"} } - // Retrieve all the trusted certificates from our previous root - // Note that we do not validate expiries here since our originally trusted root might have expired certs - allTrustedLeafCerts, allTrustedIntCerts := parseAllCerts(prevRoot) - trustedLeafCerts, err := validRootLeafCerts(allTrustedLeafCerts, gun, false) - // If we have certificates that match this specific GUN, let's make sure to - // use them first to validate that this new root is valid. - if len(trustedLeafCerts) != 0 { + // If we have a previous root, let's try to use it to validate that this new root is valid. + if prevRoot != nil { + // Retrieve all the trusted certificates from our previous root + // Note that we do not validate expiries here since our originally trusted root might have expired certs + allTrustedLeafCerts, allTrustedIntCerts := parseAllCerts(prevRoot) + trustedLeafCerts, err := validRootLeafCerts(allTrustedLeafCerts, gun, false) + + // Use the certificates we found in the previous root for the GUN to verify its signatures + // This could potentially be an empty set, in which case we will fail to verify logrus.Debugf("found %d valid root leaf certificates for %s: %s", len(trustedLeafCerts), gun, prettyFormatCertIDs(trustedLeafCerts)) + + // Extract the previous root's threshold for signature verification + prevRootRoleData, ok := prevRoot.Signed.Roles[data.CanonicalRootRole] + if !ok { + return nil, &ErrValidationFail{Reason: "could not retrieve previous root role data"} + } + err = signed.VerifySignatures( - root, data.BaseRole{Keys: trustmanager.CertsToKeys(trustedLeafCerts, allTrustedIntCerts), Threshold: 1}) + root, data.BaseRole{Keys: trustmanager.CertsToKeys(trustedLeafCerts, allTrustedIntCerts), Threshold: prevRootRoleData.Threshold}) if err != nil { logrus.Debugf("failed to verify TUF data for: %s, %v", gun, err) - return &ErrValidationFail{Reason: "failed to validate data with current trusted certificates"} + return nil, &ErrValidationFail{Reason: "failed to validate data with current trusted certificates"} } } else { logrus.Debugf("found no currently valid root certificates for %s, using trust_pinning config to bootstrap trust", gun) trustPinCheckFunc, err := NewTrustPinChecker(trustPinning, gun) if err != nil { - return &ErrValidationFail{Reason: err.Error()} + return nil, &ErrValidationFail{Reason: err.Error()} } validPinnedCerts := []*x509.Certificate{} @@ -140,7 +150,7 @@ func ValidateRoot(prevRoot *data.SignedRoot, root *data.Signed, gun string, trus validPinnedCerts = append(validPinnedCerts, cert) } if len(validPinnedCerts) == 0 { - return &ErrValidationFail{Reason: "unable to match any certificates to trust_pinning config"} + return nil, &ErrValidationFail{Reason: "unable to match any certificates to trust_pinning config"} } certsFromRoot = validPinnedCerts } @@ -152,11 +162,11 @@ func ValidateRoot(prevRoot *data.SignedRoot, root *data.Signed, gun string, trus Keys: trustmanager.CertsToKeys(certsFromRoot, allIntCerts), Threshold: rootRole.Threshold}) if err != nil { logrus.Debugf("failed to verify TUF data for: %s, %v", gun, err) - return &ErrValidationFail{Reason: "failed to validate integrity of roots"} + return nil, &ErrValidationFail{Reason: "failed to validate integrity of roots"} } logrus.Debugf("Root validation succeeded for %s", gun) - return nil + return signedRoot, nil } // validRootLeafCerts returns a list of possibly (if checkExpiry is true) non-expired, non-sha1 certificates diff --git a/trustpinning/certs_test.go b/trustpinning/certs_test.go index 8f59de920c..316ceec8d6 100644 --- a/trustpinning/certs_test.go +++ b/trustpinning/certs_test.go @@ -22,6 +22,7 @@ import ( "github.com/docker/notary/trustmanager" "github.com/docker/notary/tuf/data" "github.com/docker/notary/tuf/signed" + "github.com/docker/notary/tuf/testutils" "github.com/stretchr/testify/require" ) @@ -61,12 +62,12 @@ func TestValidateRoot(t *testing.T) { // This call to ValidateRoot will succeed since we are using a valid PEM // encoded certificate, and have no other certificates for this CN - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{}) + _, err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{}) require.NoError(t, err) // This call to ValidateRoot will fail since we are passing in a dnsName that // doesn't match the CN of the certificate. - err = ValidateRoot(nil, &testSignedRoot, "diogomonica.com/notary", TrustPinConfig{}) + _, err = ValidateRoot(nil, &testSignedRoot, "diogomonica.com/notary", TrustPinConfig{}) require.Error(t, err, "An error was expected") require.Equal(t, err, &ErrValidationFail{Reason: "unable to retrieve valid leaf certificates"}) @@ -80,7 +81,7 @@ func TestValidateRoot(t *testing.T) { // Unmarshal our signedroot json.Unmarshal(signedRootBytes.Bytes(), &testSignedRoot) - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{}) + _, err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{}) require.Error(t, err, "illegal base64 data at input byte") // @@ -93,7 +94,7 @@ func TestValidateRoot(t *testing.T) { // Unmarshal our signedroot json.Unmarshal(signedRootBytes.Bytes(), &testSignedRoot) - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{}) + _, err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{}) require.Error(t, err, "An error was expected") require.Equal(t, err, &ErrValidationFail{Reason: "unable to retrieve valid leaf certificates"}) @@ -108,7 +109,7 @@ func TestValidateRoot(t *testing.T) { // Unmarshal our signedroot json.Unmarshal(signedRootBytes.Bytes(), &testSignedRoot) - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{}) + _, err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{}) require.Error(t, err, "An error was expected") require.Equal(t, err, &ErrValidationFail{Reason: "unable to retrieve valid leaf certificates"}) @@ -127,7 +128,7 @@ func TestValidateRoot(t *testing.T) { // Unmarshal our signedroot json.Unmarshal(signedRootBytes.Bytes(), &testSignedRoot) - err = ValidateRoot(nil, &testSignedRoot, "secure.example.com", TrustPinConfig{}) + _, err = ValidateRoot(nil, &testSignedRoot, "secure.example.com", TrustPinConfig{}) require.Error(t, err, "An error was expected") require.Equal(t, err, &ErrValidationFail{Reason: "unable to retrieve valid leaf certificates"}) } @@ -149,7 +150,7 @@ func TestValidateRootWithoutTOFUS(t *testing.T) { json.Unmarshal(signedRootBytes.Bytes(), &testSignedRoot) // This call to ValidateRoot will fail since we are explicitly disabling TOFU and have no local certs - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{DisableTOFU: true}) + _, err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{DisableTOFU: true}) require.Error(t, err) } @@ -168,14 +169,18 @@ func TestValidateRootWithPinnedCert(t *testing.T) { // Unmarshal our signedroot json.Unmarshal(signedRootBytes.Bytes(), &testSignedRoot) + typedSignedRoot, err := data.RootFromSigned(&testSignedRoot) + require.NoError(t, err) // This call to ValidateRoot should succeed with the correct Cert ID (same as root public key ID) - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"docker.com/notary": {rootPubKeyID}}, DisableTOFU: true}) + validatedSignedRoot, err := ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"docker.com/notary": {rootPubKeyID}}, DisableTOFU: true}) require.NoError(t, err) + require.Equal(t, validatedSignedRoot, typedSignedRoot) // This call to ValidateRoot should also succeed with the correct Cert ID (same as root public key ID), even though we passed an extra bad one - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"docker.com/notary": {rootPubKeyID, "invalidID"}}, DisableTOFU: true}) + validatedSignedRoot, err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"docker.com/notary": {rootPubKeyID, "invalidID"}}, DisableTOFU: true}) require.NoError(t, err) + require.Equal(t, validatedSignedRoot, typedSignedRoot) } func TestValidateRootWithPinnerCertAndIntermediates(t *testing.T) { @@ -328,11 +333,14 @@ func TestValidateRootWithPinnerCertAndIntermediates(t *testing.T) { err = signed.Sign(cs, signedRoot, []data.PublicKey{ecdsax509Key}, 1, nil) require.NoError(t, err) + typedSignedRoot, err := data.RootFromSigned(signedRoot) + require.NoError(t, err) + tempBaseDir, err := ioutil.TempDir("", "notary-test-") defer os.RemoveAll(tempBaseDir) require.NoError(t, err, "failed to create a temporary directory: %s", err) - err = ValidateRoot( + validatedRoot, err := ValidateRoot( nil, signedRoot, "docker.io/notary/test", @@ -344,6 +352,7 @@ func TestValidateRootWithPinnerCertAndIntermediates(t *testing.T) { }, ) require.NoError(t, err, "failed to validate certID with intermediate") + require.Equal(t, typedSignedRoot, validatedRoot) } func TestValidateRootFailuresWithPinnedCert(t *testing.T) { @@ -361,26 +370,29 @@ func TestValidateRootFailuresWithPinnedCert(t *testing.T) { // Unmarshal our signedroot json.Unmarshal(signedRootBytes.Bytes(), &testSignedRoot) + typedSignedRoot, err := data.RootFromSigned(&testSignedRoot) + require.NoError(t, err) // This call to ValidateRoot should fail due to an incorrect cert ID - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"docker.com/notary": {"ABSOLUTELY NOT A CERT ID"}}, DisableTOFU: true}) + _, err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"docker.com/notary": {"ABSOLUTELY NOT A CERT ID"}}, DisableTOFU: true}) require.Error(t, err) // This call to ValidateRoot should fail due to an empty cert ID - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"docker.com/notary": {""}}, DisableTOFU: true}) + _, err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"docker.com/notary": {""}}, DisableTOFU: true}) require.Error(t, err) // This call to ValidateRoot should fail due to an invalid GUN (even though the cert ID is correct), and TOFUS is set to false - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"not_a_gun": {rootPubKeyID}}, DisableTOFU: true}) + _, err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"not_a_gun": {rootPubKeyID}}, DisableTOFU: true}) require.Error(t, err) // This call to ValidateRoot should fail due to an invalid cert ID, even though it's a valid key ID for targets - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"docker.com/notary": {targetsPubKeyID}}, DisableTOFU: true}) + _, err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"docker.com/notary": {targetsPubKeyID}}, DisableTOFU: true}) require.Error(t, err) // This call to ValidateRoot should succeed because we fall through to TOFUS because we have no matching GUNs under Certs - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"not_a_gun": {rootPubKeyID}}, DisableTOFU: false}) + validatedRoot, err := ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"not_a_gun": {rootPubKeyID}}, DisableTOFU: false}) require.NoError(t, err) + require.Equal(t, typedSignedRoot, validatedRoot) } func TestValidateRootWithPinnedCA(t *testing.T) { @@ -396,31 +408,34 @@ func TestValidateRootWithPinnedCA(t *testing.T) { templ.Execute(&signedRootBytes, SignedRSARootTemplate{RootPem: validPEMEncodedRSARoot}) // Unmarshal our signedRoot json.Unmarshal(signedRootBytes.Bytes(), &testSignedRoot) + typedSignedRoot, err := data.RootFromSigned(&testSignedRoot) + require.NoError(t, err) // This call to ValidateRoot will fail because we have an invalid path for the CA - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{CA: map[string]string{"docker.com/notary": filepath.Join(tempBaseDir, "nonexistent")}}) + _, err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{CA: map[string]string{"docker.com/notary": filepath.Join(tempBaseDir, "nonexistent")}}) require.Error(t, err) // This call to ValidateRoot will fail because we have no valid GUNs to use, and TOFUS is disabled - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{CA: map[string]string{"othergun": filepath.Join(tempBaseDir, "nonexistent")}, DisableTOFU: true}) + _, err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{CA: map[string]string{"othergun": filepath.Join(tempBaseDir, "nonexistent")}, DisableTOFU: true}) require.Error(t, err) // This call to ValidateRoot will succeed because we have no valid GUNs to use and we fall back to enabled TOFUS - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{CA: map[string]string{"othergun": filepath.Join(tempBaseDir, "nonexistent")}, DisableTOFU: false}) + validatedRoot, err := ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{CA: map[string]string{"othergun": filepath.Join(tempBaseDir, "nonexistent")}, DisableTOFU: false}) require.NoError(t, err) + require.Equal(t, typedSignedRoot, validatedRoot) // Write an invalid CA cert (not even a PEM) to the tempDir and ensure validation fails when using it invalidCAFilepath := filepath.Join(tempBaseDir, "invalid.ca") require.NoError(t, ioutil.WriteFile(invalidCAFilepath, []byte("ABSOLUTELY NOT A PEM"), 0644)) // Using this invalid CA cert should fail on ValidateRoot - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{CA: map[string]string{"docker.com/notary": invalidCAFilepath}, DisableTOFU: true}) + _, err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{CA: map[string]string{"docker.com/notary": invalidCAFilepath}, DisableTOFU: true}) require.Error(t, err) validCAFilepath := "../fixtures/root-ca.crt" // If we pass an invalid Certs entry in addition to this valid CA entry, since Certs has priority for pinning we will fail - err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"docker.com/notary": {"invalidID"}}, CA: map[string]string{"docker.com/notary": validCAFilepath}, DisableTOFU: true}) + _, err = ValidateRoot(nil, &testSignedRoot, "docker.com/notary", TrustPinConfig{Certs: map[string][]string{"docker.com/notary": {"invalidID"}}, CA: map[string]string{"docker.com/notary": validCAFilepath}, DisableTOFU: true}) require.Error(t, err) // Now construct a new root with a valid cert chain, such that signatures are correct over the 'notary-signer' GUN. Pin the root-ca and validate @@ -469,10 +484,14 @@ func TestValidateRootWithPinnedCA(t *testing.T) { err = signed.Sign(cs, newTestSignedRoot, []data.PublicKey{newRootKey}, 1, nil) require.NoError(t, err) - // Check that we validate correctly against a pinned CA and provided bundle - err = ValidateRoot(nil, newTestSignedRoot, "notary-signer", TrustPinConfig{CA: map[string]string{"notary-signer": validCAFilepath}, DisableTOFU: true}) + newTypedSignedRoot, err := data.RootFromSigned(newTestSignedRoot) require.NoError(t, err) + // Check that we validate correctly against a pinned CA and provided bundle + validatedRoot, err = ValidateRoot(nil, newTestSignedRoot, "notary-signer", TrustPinConfig{CA: map[string]string{"notary-signer": validCAFilepath}, DisableTOFU: true}) + require.NoError(t, err) + require.Equal(t, newTypedSignedRoot, validatedRoot) + // Add an expired CA for the same gun to our previous pinned bundle, ensure that we still validate correctly goodRootCABundle, err := trustmanager.LoadCertBundleFromFile(validCAFilepath) require.NoError(t, err) @@ -490,8 +509,9 @@ func TestValidateRootWithPinnedCA(t *testing.T) { require.NoError(t, ioutil.WriteFile(bundleWithExpiredCertPath, bundleWithExpiredCert, 0644)) // Check that we validate correctly against a pinned CA and provided bundle - err = ValidateRoot(nil, newTestSignedRoot, "notary-signer", TrustPinConfig{CA: map[string]string{"notary-signer": bundleWithExpiredCertPath}, DisableTOFU: true}) + validatedRoot, err = ValidateRoot(nil, newTestSignedRoot, "notary-signer", TrustPinConfig{CA: map[string]string{"notary-signer": bundleWithExpiredCertPath}, DisableTOFU: true}) require.NoError(t, err) + require.Equal(t, newTypedSignedRoot, validatedRoot) testPubKey2, err := cryptoService.Create("root", "notary-signer", data.ECDSAKey) require.NoError(t, err) @@ -504,7 +524,7 @@ func TestValidateRootWithPinnedCA(t *testing.T) { allExpiredCertPath := filepath.Join(tempBaseDir, "all_expired_cert.pem") require.NoError(t, ioutil.WriteFile(allExpiredCertPath, allExpiredCertBundle, 0644)) // Now only use expired certs in the bundle, we should fail - err = ValidateRoot(nil, newTestSignedRoot, "notary-signer", TrustPinConfig{CA: map[string]string{"notary-signer": allExpiredCertPath}, DisableTOFU: true}) + _, err = ValidateRoot(nil, newTestSignedRoot, "notary-signer", TrustPinConfig{CA: map[string]string{"notary-signer": allExpiredCertPath}, DisableTOFU: true}) require.Error(t, err) // Add a CA cert for a that won't validate against the root leaf certificate @@ -518,7 +538,7 @@ func TestValidateRootWithPinnedCA(t *testing.T) { require.NoError(t, err) bundleWithWrongCertPath := filepath.Join(tempBaseDir, "bundle_with_expired_cert.pem") require.NoError(t, ioutil.WriteFile(bundleWithWrongCertPath, bundleWithWrongCert, 0644)) - err = ValidateRoot(nil, newTestSignedRoot, "notary-signer", TrustPinConfig{CA: map[string]string{"notary-signer": bundleWithWrongCertPath}, DisableTOFU: true}) + _, err = ValidateRoot(nil, newTestSignedRoot, "notary-signer", TrustPinConfig{CA: map[string]string{"notary-signer": bundleWithWrongCertPath}, DisableTOFU: true}) require.Error(t, err) } @@ -531,49 +551,16 @@ func TestValidateSuccessfulRootRotation(t *testing.T) { } } -// Generates certificates for two keys which have been added to the keystore. -// Also returns the temporary directory so it can be cleaned up. -func generateTwoCerts(t *testing.T, gun, keyAlg string) ( - string, *cryptoservice.CryptoService, []*x509.Certificate) { - tempBaseDir, err := ioutil.TempDir("", "notary-test-") - require.NoError(t, err, "failed to create a temporary directory: %s", err) - - fileKeyStore, err := trustmanager.NewKeyFileStore(tempBaseDir, passphraseRetriever) - require.NoError(t, err) - - cryptoService := cryptoservice.NewCryptoService(fileKeyStore) - - certificates := make([]*x509.Certificate, 2) - for i := 0; i < 2; i++ { - pubKey, err := cryptoService.Create("root", gun, keyAlg) - require.NoError(t, err) - - key, _, err := fileKeyStore.GetKey(pubKey.ID()) - require.NoError(t, err) - - cert, err := generateTestingCertificate(key, gun) - require.NoError(t, err) - - certificates[i] = cert - } - return tempBaseDir, cryptoService, certificates -} - func testValidateSuccessfulRootRotation(t *testing.T, keyAlg, rootKeyType string) { // The gun to test gun := "docker.com/notary" - tempBaseDir, cs, certificates := generateTwoCerts(t, gun, keyAlg) - defer os.RemoveAll(tempBaseDir) - origRootCert := certificates[0] - replRootCert := certificates[1] - - // Set up the previous root prior to rotating - // We need the PEM representation of the original key to put it into the TUF data - origRootPEMCert := trustmanager.CertToPEM(origRootCert) + memKeyStore := trustmanager.NewKeyMemoryStore(passphraseRetriever) + cs := cryptoservice.NewCryptoService(memKeyStore) // Tuf key with PEM-encoded x509 certificate - origRootKey := data.NewPublicKey(rootKeyType, origRootPEMCert) + origRootKey, err := testutils.CreateKey(cs, gun, data.CanonicalRootRole, keyAlg) + require.NoError(t, err) origRootRole, err := data.NewRole(data.CanonicalRootRole, 1, []string{origRootKey.ID()}, nil) require.NoError(t, err) @@ -593,17 +580,14 @@ func testValidateSuccessfulRootRotation(t *testing.T, keyAlg, rootKeyType string signedOrigTestRoot, err := origTestRoot.ToSigned() require.NoError(t, err) - // We only sign with the new key, and not with the original one. err = signed.Sign(cs, signedOrigTestRoot, []data.PublicKey{origRootKey}, 1, nil) require.NoError(t, err) prevRoot, err := data.RootFromSigned(signedOrigTestRoot) require.NoError(t, err) - // We need the PEM representation of the replacement key to put it into the TUF data - replRootPEMCert := trustmanager.CertToPEM(replRootCert) - // Tuf key with PEM-encoded x509 certificate - replRootKey := data.NewPublicKey(rootKeyType, replRootPEMCert) + replRootKey, err := testutils.CreateKey(cs, gun, data.CanonicalRootRole, keyAlg) + require.NoError(t, err) rootRole, err := data.NewRole(data.CanonicalRootRole, 1, []string{replRootKey.ID()}, nil) require.NoError(t, err) @@ -625,10 +609,14 @@ func testValidateSuccessfulRootRotation(t *testing.T, keyAlg, rootKeyType string err = signed.Sign(cs, signedTestRoot, []data.PublicKey{replRootKey, origRootKey}, 2, nil) require.NoError(t, err) + typedSignedRoot, err := data.RootFromSigned(signedTestRoot) + require.NoError(t, err) + // This call to ValidateRoot will succeed since we are using a valid PEM // encoded certificate, and have no other certificates for this CN - err = ValidateRoot(prevRoot, signedTestRoot, gun, TrustPinConfig{}) + validatedRoot, err := ValidateRoot(prevRoot, signedTestRoot, gun, TrustPinConfig{}) require.NoError(t, err) + require.Equal(t, typedSignedRoot, validatedRoot) } // TestValidateRootRotationMissingOrigSig runs through a full root certificate rotation @@ -644,18 +632,12 @@ func TestValidateRootRotationMissingOrigSig(t *testing.T) { func testValidateRootRotationMissingOrigSig(t *testing.T, keyAlg, rootKeyType string) { gun := "docker.com/notary" - tempBaseDir, cryptoService, certificates := generateTwoCerts( - t, gun, keyAlg) - defer os.RemoveAll(tempBaseDir) - origRootCert := certificates[0] - replRootCert := certificates[1] - - // Set up the previous root prior to rotating - // We need the PEM representation of the original key to put it into the TUF data - origRootPEMCert := trustmanager.CertToPEM(origRootCert) + memKeyStore := trustmanager.NewKeyMemoryStore(passphraseRetriever) + cs := cryptoservice.NewCryptoService(memKeyStore) // Tuf key with PEM-encoded x509 certificate - origRootKey := data.NewPublicKey(rootKeyType, origRootPEMCert) + origRootKey, err := testutils.CreateKey(cs, gun, data.CanonicalRootRole, keyAlg) + require.NoError(t, err) origRootRole, err := data.NewRole(data.CanonicalRootRole, 1, []string{origRootKey.ID()}, nil) require.NoError(t, err) @@ -675,17 +657,14 @@ func testValidateRootRotationMissingOrigSig(t *testing.T, keyAlg, rootKeyType st signedOrigTestRoot, err := origTestRoot.ToSigned() require.NoError(t, err) - // We only sign with the new key, and not with the original one. - err = signed.Sign(cryptoService, signedOrigTestRoot, []data.PublicKey{origRootKey}, 1, nil) + err = signed.Sign(cs, signedOrigTestRoot, []data.PublicKey{origRootKey}, 1, nil) require.NoError(t, err) prevRoot, err := data.RootFromSigned(signedOrigTestRoot) require.NoError(t, err) - // We need the PEM representation of the replacement key to put it into the TUF data - replRootPEMCert := trustmanager.CertToPEM(replRootCert) - // Tuf key with PEM-encoded x509 certificate - replRootKey := data.NewPublicKey(rootKeyType, replRootPEMCert) + replRootKey, err := testutils.CreateKey(cs, gun, data.CanonicalRootRole, keyAlg) + require.NoError(t, err) rootRole, err := data.NewRole(data.CanonicalRootRole, 1, []string{replRootKey.ID()}, nil) require.NoError(t, err) @@ -706,12 +685,12 @@ func testValidateRootRotationMissingOrigSig(t *testing.T, keyAlg, rootKeyType st require.NoError(t, err) // We only sign with the new key, and not with the original one. - err = signed.Sign(cryptoService, signedTestRoot, []data.PublicKey{replRootKey}, 1, nil) + err = signed.Sign(cs, signedTestRoot, []data.PublicKey{replRootKey}, 1, nil) require.NoError(t, err) // This call to ValidateRoot will succeed since we are using a valid PEM // encoded certificate, and have no other certificates for this CN - err = ValidateRoot(prevRoot, signedTestRoot, gun, TrustPinConfig{}) + _, err = ValidateRoot(prevRoot, signedTestRoot, gun, TrustPinConfig{}) require.Error(t, err, "insuficient signatures on root") } @@ -728,21 +707,12 @@ func TestValidateRootRotationMissingNewSig(t *testing.T) { func testValidateRootRotationMissingNewSig(t *testing.T, keyAlg, rootKeyType string) { gun := "docker.com/notary" - tempBaseDir, cryptoService, certificates := generateTwoCerts( - t, gun, keyAlg) - defer os.RemoveAll(tempBaseDir) - origRootCert := certificates[0] - replRootCert := certificates[1] - - // We need the PEM representation of the replacement key to put it into the TUF data - replRootPEMCert := trustmanager.CertToPEM(replRootCert) - - // Set up the previous root prior to rotating - // We need the PEM representation of the original key to put it into the TUF data - origRootPEMCert := trustmanager.CertToPEM(origRootCert) + memKeyStore := trustmanager.NewKeyMemoryStore(passphraseRetriever) + cs := cryptoservice.NewCryptoService(memKeyStore) // Tuf key with PEM-encoded x509 certificate - origRootKey := data.NewPublicKey(rootKeyType, origRootPEMCert) + origRootKey, err := testutils.CreateKey(cs, gun, data.CanonicalRootRole, keyAlg) + require.NoError(t, err) origRootRole, err := data.NewRole(data.CanonicalRootRole, 1, []string{origRootKey.ID()}, nil) require.NoError(t, err) @@ -762,14 +732,14 @@ func testValidateRootRotationMissingNewSig(t *testing.T, keyAlg, rootKeyType str signedOrigTestRoot, err := origTestRoot.ToSigned() require.NoError(t, err) - // We only sign with the new key, and not with the original one. - err = signed.Sign(cryptoService, signedOrigTestRoot, []data.PublicKey{origRootKey}, 1, nil) + err = signed.Sign(cs, signedOrigTestRoot, []data.PublicKey{origRootKey}, 1, nil) require.NoError(t, err) prevRoot, err := data.RootFromSigned(signedOrigTestRoot) require.NoError(t, err) // Tuf key with PEM-encoded x509 certificate - replRootKey := data.NewPublicKey(rootKeyType, replRootPEMCert) + replRootKey, err := testutils.CreateKey(cs, gun, data.CanonicalRootRole, keyAlg) + require.NoError(t, err) rootRole, err := data.NewRole(data.CanonicalRootRole, 1, []string{replRootKey.ID()}, nil) require.NoError(t, err) @@ -790,12 +760,12 @@ func testValidateRootRotationMissingNewSig(t *testing.T, keyAlg, rootKeyType str require.NoError(t, err) // We only sign with the old key, and not with the new one - err = signed.Sign(cryptoService, signedTestRoot, []data.PublicKey{origRootKey}, 1, nil) + err = signed.Sign(cs, signedTestRoot, []data.PublicKey{origRootKey}, 1, nil) require.NoError(t, err) // This call to ValidateRoot will succeed since we are using a valid PEM // encoded certificate, and have no other certificates for this CN - err = ValidateRoot(prevRoot, signedTestRoot, gun, TrustPinConfig{}) + _, err = ValidateRoot(prevRoot, signedTestRoot, gun, TrustPinConfig{}) require.Error(t, err, "insuficient signatures on root") } diff --git a/tuf/testutils/repo.go b/tuf/testutils/repo.go index 62ad793e4e..045e5a2734 100644 --- a/tuf/testutils/repo.go +++ b/tuf/testutils/repo.go @@ -19,8 +19,8 @@ import ( // CreateKey creates a new key inside the cryptoservice for the given role and gun, // returning the public key. If the role is a root role, create an x509 key. -func CreateKey(cs signed.CryptoService, gun, role string) (data.PublicKey, error) { - key, err := cs.Create(role, gun, data.ECDSAKey) +func CreateKey(cs signed.CryptoService, gun, role, keyAlgorithm string) (data.PublicKey, error) { + key, err := cs.Create(role, gun, keyAlgorithm) if err != nil { return nil, err } @@ -36,7 +36,13 @@ func CreateKey(cs signed.CryptoService, gun, role string) (data.PublicKey, error if err != nil { return nil, err } - key = data.NewECDSAx509PublicKey(trustmanager.CertToPEM(cert)) + // Keep the x509 key type consistent with the key's algorithm + if keyAlgorithm == data.RSAKey { + key = data.NewRSAx509PublicKey(trustmanager.CertToPEM(cert)) + } else { + key = data.NewECDSAx509PublicKey(trustmanager.CertToPEM(cert)) + } + } return key, nil } @@ -50,7 +56,7 @@ func EmptyRepo(gun string, delegationRoles ...string) (*tuf.Repo, signed.CryptoS baseRoles := map[string]data.BaseRole{} for _, role := range data.BaseRoles { - key, err := CreateKey(cs, gun, role) + key, err := CreateKey(cs, gun, role, data.ECDSAKey) if err != nil { return nil, nil, err } @@ -77,7 +83,7 @@ func EmptyRepo(gun string, delegationRoles ...string) (*tuf.Repo, signed.CryptoS sort.Strings(delegationRoles) for _, delgName := range delegationRoles { // create a delegations key and a delegation in the tuf repo - delgKey, err := CreateKey(cs, gun, delgName) + delgKey, err := CreateKey(cs, gun, delgName, data.ECDSAKey) if err != nil { return nil, nil, err } diff --git a/tuf/testutils/swizzler.go b/tuf/testutils/swizzler.go index 49ef0a56cb..2f746c81d5 100644 --- a/tuf/testutils/swizzler.go +++ b/tuf/testutils/swizzler.go @@ -269,7 +269,7 @@ func (m *MetadataSwizzler) SignMetadataWithInvalidKey(role string) error { // create an invalid key, but not in the existing CryptoService cs := cryptoservice.NewCryptoService(trustmanager.NewKeyMemoryStore(passphrase.ConstantRetriever(""))) - key, err := CreateKey(cs, m.Gun, role) + key, err := CreateKey(cs, m.Gun, role, data.ECDSAKey) if err != nil { return err } @@ -466,7 +466,7 @@ func (m *MetadataSwizzler) RotateKey(role string, key data.PublicKey) error { // ChangeRootKey swaps out the root key with a new key, and re-signs the metadata // with the new key func (m *MetadataSwizzler) ChangeRootKey() error { - key, err := CreateKey(m.CryptoService, m.Gun, data.CanonicalRootRole) + key, err := CreateKey(m.CryptoService, m.Gun, data.CanonicalRootRole, data.ECDSAKey) if err != nil { return err }