Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate root rotations against trust pinning where appropriate #800

Merged
merged 5 commits into from
Sep 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -870,8 +870,9 @@ func (r *NotaryRepository) bootstrapClient(checkInitialized bool) (*TUFClient, e
if err := newBuilder.Load(data.CanonicalRootRole, rootJSON, minVersion, false); err != nil {
// Ok, the old root is expired - we want to download a new one. But we want to use the
// old root to verify the new root, so bootstrap a new builder with the old builder
// but use the trustpinning to validate the new root
minVersion = oldBuilder.GetLoadedVersion(data.CanonicalRootRole)
newBuilder = oldBuilder.BootstrapNewBuilder()
newBuilder = oldBuilder.BootstrapNewBuilderWithNewTrustpin(r.trustPinning)
}
}

Expand Down
46 changes: 26 additions & 20 deletions trustpinning/certs.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ func ValidateRoot(prevRoot *data.SignedRoot, root *data.Signed, gun string, trus
logrus.Debugf("found %d leaf certs, of which %d are valid leaf certs for %s", len(allLeafCerts), len(certsFromRoot), gun)

// If we have a previous root, let's try to use it to validate that this new root is valid.
if prevRoot != nil {
havePrevRoot := prevRoot != nil
if havePrevRoot {
// 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)
Expand All @@ -126,34 +127,38 @@ func ValidateRoot(prevRoot *data.SignedRoot, root *data.Signed, gun string, trus
if !ok {
return nil, &ErrValidationFail{Reason: "could not retrieve previous root role data"}
}

err = signed.VerifySignatures(
root, data.BaseRole{Keys: utils.CertsToKeys(trustedLeafCerts, allTrustedIntCerts), Threshold: prevRootRoleData.Threshold})
if err != nil {
logrus.Debugf("failed to verify TUF data for: %s, %v", gun, err)
return nil, &ErrRootRotationFail{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 nil, &ErrValidationFail{Reason: err.Error()}
// Clear the IsValid marks we could have received from VerifySignatures
for i := range root.Signatures {
root.Signatures[i].IsValid = false
}
}

validPinnedCerts := map[string]*x509.Certificate{}
for id, cert := range certsFromRoot {
logrus.Debugf("checking trust-pinning for cert: %s", id)
if ok := trustPinCheckFunc(cert, validIntCerts[id]); !ok {
logrus.Debugf("trust-pinning check failed for cert: %s", id)
continue
}
validPinnedCerts[id] = cert
}
if len(validPinnedCerts) == 0 {
return nil, &ErrValidationFail{Reason: "unable to match any certificates to trust_pinning config"}
// Regardless of having a previous root or not, confirm that the new root validates against the trust pinning
logrus.Debugf("checking root against trust_pinning config", gun)
trustPinCheckFunc, err := NewTrustPinChecker(trustPinning, gun, !havePrevRoot)
if err != nil {
return nil, &ErrValidationFail{Reason: err.Error()}
}

validPinnedCerts := map[string]*x509.Certificate{}
for id, cert := range certsFromRoot {
logrus.Debugf("checking trust-pinning for cert: %s", id)
if ok := trustPinCheckFunc(cert, validIntCerts[id]); !ok {
logrus.Debugf("trust-pinning check failed for cert: %s", id)
continue
}
certsFromRoot = validPinnedCerts
validPinnedCerts[id] = cert
}
if len(validPinnedCerts) == 0 {
return nil, &ErrValidationFail{Reason: "unable to match any certificates to trust_pinning config"}
}
certsFromRoot = validPinnedCerts

// Validate the integrity of the new root (does it have valid signatures)
// Note that certsFromRoot is guaranteed to be unchanged only if we had prior cert data for this GUN or enabled TOFUS
Expand All @@ -166,7 +171,8 @@ func ValidateRoot(prevRoot *data.SignedRoot, root *data.Signed, gun string, trus
}

logrus.Debugf("root validation succeeded for %s", gun)
return signedRoot, nil
// Call RootFromSigned to make sure we pick up on the IsValid markings from VerifySignatures
return data.RootFromSigned(root)
}

// validRootLeafCerts returns a list of possibly (if checkExpiry is true) non-expired, non-sha1 certificates
Expand Down
232 changes: 214 additions & 18 deletions trustpinning/certs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func TestValidateRootWithPinnedCert(t *testing.T) {
// This call to trustpinning.ValidateRoot should succeed with the correct Cert ID (same as root public key ID)
validatedSignedRoot, err := trustpinning.ValidateRoot(nil, &testSignedRoot, "docker.com/notary", trustpinning.TrustPinConfig{Certs: map[string][]string{"docker.com/notary": {rootPubKeyID}}, DisableTOFU: true})
require.NoError(t, err)
generateRootKeyIDs(typedSignedRoot)
typedSignedRoot.Signatures[0].IsValid = true
require.Equal(t, validatedSignedRoot, typedSignedRoot)

// This call to trustpinning.ValidateRoot should also succeed with the correct Cert ID (same as root public key ID), even though we passed an extra bad one
Expand All @@ -190,7 +190,7 @@ func TestValidateRootWithPinnedCert(t *testing.T) {
require.Equal(t, typedSignedRoot, validatedSignedRoot)
}

func TestValidateRootWithPinnerCertAndIntermediates(t *testing.T) {
func TestValidateRootWithPinnedCertAndIntermediates(t *testing.T) {
now := time.Now()
serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128)

Expand Down Expand Up @@ -361,7 +361,11 @@ func TestValidateRootWithPinnerCertAndIntermediates(t *testing.T) {
},
)
require.NoError(t, err, "failed to validate certID with intermediate")
generateRootKeyIDs(typedSignedRoot)
for idx, sig := range typedSignedRoot.Signatures {
if sig.KeyID == ecdsax509Key.ID() {
typedSignedRoot.Signatures[idx].IsValid = true
}
}
require.Equal(t, typedSignedRoot, validatedRoot)
}

Expand Down Expand Up @@ -402,7 +406,7 @@ func TestValidateRootFailuresWithPinnedCert(t *testing.T) {
// This call to trustpinning.ValidateRoot should succeed because we fall through to TOFUS because we have no matching GUNs under Certs
validatedRoot, err := trustpinning.ValidateRoot(nil, &testSignedRoot, "docker.com/notary", trustpinning.TrustPinConfig{Certs: map[string][]string{"not_a_gun": {rootPubKeyID}}, DisableTOFU: false})
require.NoError(t, err)
generateRootKeyIDs(typedSignedRoot)
typedSignedRoot.Signatures[0].IsValid = true
require.Equal(t, typedSignedRoot, validatedRoot)
}

Expand Down Expand Up @@ -433,7 +437,7 @@ func TestValidateRootWithPinnedCA(t *testing.T) {
// This call to trustpinning.ValidateRoot will succeed because we have no valid GUNs to use and we fall back to enabled TOFUS
validatedRoot, err := trustpinning.ValidateRoot(nil, &testSignedRoot, "docker.com/notary", trustpinning.TrustPinConfig{CA: map[string]string{"othergun": filepath.Join(tempBaseDir, "nonexistent")}, DisableTOFU: false})
require.NoError(t, err)
generateRootKeyIDs(typedSignedRoot)
typedSignedRoot.Signatures[0].IsValid = true
require.Equal(t, typedSignedRoot, validatedRoot)

// Write an invalid CA cert (not even a PEM) to the tempDir and ensure validation fails when using it
Expand Down Expand Up @@ -503,7 +507,11 @@ func TestValidateRootWithPinnedCA(t *testing.T) {
// Check that we validate correctly against a pinned CA and provided bundle
validatedRoot, err = trustpinning.ValidateRoot(nil, newTestSignedRoot, "notary-signer", trustpinning.TrustPinConfig{CA: map[string]string{"notary-signer": validCAFilepath}, DisableTOFU: true})
require.NoError(t, err)
generateRootKeyIDs(newTypedSignedRoot)
for idx, sig := range newTypedSignedRoot.Signatures {
if sig.KeyID == newRootKey.ID() {
newTypedSignedRoot.Signatures[idx].IsValid = true
}
}
require.Equal(t, newTypedSignedRoot, validatedRoot)

// Add an expired CA for the same gun to our previous pinned bundle, ensure that we still validate correctly
Expand All @@ -525,8 +533,6 @@ func TestValidateRootWithPinnedCA(t *testing.T) {
// Check that we validate correctly against a pinned CA and provided bundle
validatedRoot, err = trustpinning.ValidateRoot(nil, newTestSignedRoot, "notary-signer", trustpinning.TrustPinConfig{CA: map[string]string{"notary-signer": bundleWithExpiredCertPath}, DisableTOFU: true})
require.NoError(t, err)
// this extra assignment is necessary because ValidateRoot calls through to a successful VerifySignature which marks IsValid
newTypedSignedRoot.Signatures[0].IsValid = true
require.Equal(t, newTypedSignedRoot, validatedRoot)

testPubKey2, err := cryptoService.Create("root", "notary-signer", data.ECDSAKey)
Expand Down Expand Up @@ -634,7 +640,11 @@ func testValidateSuccessfulRootRotation(t *testing.T, keyAlg, rootKeyType string
// encoded certificate, and have no other certificates for this CN
validatedRoot, err := trustpinning.ValidateRoot(prevRoot, signedTestRoot, gun, trustpinning.TrustPinConfig{})
require.NoError(t, err)
generateRootKeyIDs(typedSignedRoot)
for idx, sig := range typedSignedRoot.Signatures {
if sig.KeyID == replRootKey.ID() {
typedSignedRoot.Signatures[idx].IsValid = true
}
}
require.Equal(t, typedSignedRoot, validatedRoot)
}

Expand Down Expand Up @@ -795,6 +805,201 @@ func testValidateRootRotationMissingNewSig(t *testing.T, keyAlg, rootKeyType str
require.Error(t, err, "insufficient signatures on root")
}

// TestValidateRootRotationTrustPinning runs a full root certificate rotation but ensures that
// the specified trust pinning is respected with the new root for the Certs and TOFUs settings
func TestValidateRootRotationTrustPinning(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay thanks for these awesome tests!

// The gun to test
gun := "docker.com/notary"

memKeyStore := trustmanager.NewKeyMemoryStore(passphraseRetriever)
cs := cryptoservice.NewCryptoService(memKeyStore)

// TUF key with PEM-encoded x509 certificate
origRootKey, err := testutils.CreateKey(cs, gun, data.CanonicalRootRole, data.RSAKey)
require.NoError(t, err)

origRootRole, err := data.NewRole(data.CanonicalRootRole, 1, []string{origRootKey.ID()}, nil)
require.NoError(t, err)

origTestRoot, err := data.NewRoot(
map[string]data.PublicKey{origRootKey.ID(): origRootKey},
map[string]*data.RootRole{
data.CanonicalRootRole: &origRootRole.RootRole,
data.CanonicalTargetsRole: &origRootRole.RootRole,
data.CanonicalSnapshotRole: &origRootRole.RootRole,
data.CanonicalTimestampRole: &origRootRole.RootRole,
},
false,
)
origTestRoot.Signed.Version = 1
require.NoError(t, err, "Failed to create new root")

signedOrigTestRoot, err := origTestRoot.ToSigned()
require.NoError(t, err)

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, err := testutils.CreateKey(cs, gun, data.CanonicalRootRole, data.RSAKey)
require.NoError(t, err)

rootRole, err := data.NewRole(data.CanonicalRootRole, 1, []string{replRootKey.ID()}, nil)
require.NoError(t, err)

testRoot, err := data.NewRoot(
map[string]data.PublicKey{replRootKey.ID(): replRootKey},
map[string]*data.RootRole{
data.CanonicalRootRole: &rootRole.RootRole,
data.CanonicalTimestampRole: &rootRole.RootRole,
data.CanonicalTargetsRole: &rootRole.RootRole,
data.CanonicalSnapshotRole: &rootRole.RootRole},
false,
)
testRoot.Signed.Version = 1
require.NoError(t, err, "Failed to create new root")

signedTestRoot, err := testRoot.ToSigned()
require.NoError(t, err)

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 trustpinning.ValidateRoot will fail due to the trust pinning mismatch in certs
invalidCertConfig := trustpinning.TrustPinConfig{
Certs: map[string][]string{
gun: {origRootKey.ID()},
},
DisableTOFU: true,
}
_, err = trustpinning.ValidateRoot(prevRoot, signedTestRoot, gun, invalidCertConfig)
require.Error(t, err)

// This call will succeed since we include the new root cert ID (and the old one)
validCertConfig := trustpinning.TrustPinConfig{
Certs: map[string][]string{
gun: {origRootKey.ID(), replRootKey.ID()},
},
DisableTOFU: true,
}

validatedRoot, err := trustpinning.ValidateRoot(prevRoot, signedTestRoot, gun, validCertConfig)
require.NoError(t, err)
for idx, sig := range typedSignedRoot.Signatures {
if sig.KeyID == replRootKey.ID() {
typedSignedRoot.Signatures[idx].IsValid = true
}
}
require.Equal(t, typedSignedRoot, validatedRoot)

// This call will also succeed since we only need the new replacement root ID to be pinned
validCertConfig = trustpinning.TrustPinConfig{
Certs: map[string][]string{
gun: {replRootKey.ID()},
},
DisableTOFU: true,
}
validatedRoot, err = trustpinning.ValidateRoot(prevRoot, signedTestRoot, gun, validCertConfig)
require.NoError(t, err)
require.Equal(t, typedSignedRoot, validatedRoot)

// Even if we disable TOFU in the trustpinning, since we have a previously trusted root we should honor a valid rotation
validatedRoot, err = trustpinning.ValidateRoot(prevRoot, signedTestRoot, gun, trustpinning.TrustPinConfig{DisableTOFU: true})
require.NoError(t, err)
require.Equal(t, typedSignedRoot, validatedRoot)
}

// TestValidateRootRotationTrustPinningInvalidCA runs a full root certificate rotation but ensures that
// the specified trust pinning rejects the new root for not being signed by the specified CA
func TestValidateRootRotationTrustPinningInvalidCA(t *testing.T) {
gun := "notary-signer"
keyAlg := data.RSAKey
// Temporary directory where test files will be created
tempBaseDir, err := ioutil.TempDir("", "notary-test-")
defer os.RemoveAll(tempBaseDir)
require.NoError(t, err, "failed to create a temporary directory: %s", err)

leafCert, err := utils.LoadCertFromFile("../fixtures/notary-signer.crt")
require.NoError(t, err)

intermediateCert, err := utils.LoadCertFromFile("../fixtures/intermediate-ca.crt")
require.NoError(t, err)

pemChainBytes, err := utils.CertChainToPEM([]*x509.Certificate{leafCert, intermediateCert})
require.NoError(t, err)

origRootKey := data.NewPublicKey(data.RSAx509Key, pemChainBytes)

rootRole, err := data.NewRole(data.CanonicalRootRole, 1, []string{origRootKey.ID()}, nil)
require.NoError(t, err)

testRoot, err := data.NewRoot(
map[string]data.PublicKey{origRootKey.ID(): origRootKey},
map[string]*data.RootRole{
data.CanonicalRootRole: &rootRole.RootRole,
data.CanonicalTimestampRole: &rootRole.RootRole,
data.CanonicalTargetsRole: &rootRole.RootRole,
data.CanonicalSnapshotRole: &rootRole.RootRole},
false,
)
testRoot.Signed.Version = 1
require.NoError(t, err, "Failed to create new root")

pemBytes, err := ioutil.ReadFile("../fixtures/notary-signer.key")
require.NoError(t, err, "could not read key file")
privKey, err := utils.ParsePEMPrivateKey(pemBytes, "")
require.NoError(t, err)

store, err := trustmanager.NewKeyFileStore(tempBaseDir, passphraseRetriever)
require.NoError(t, err)
cs := cryptoservice.NewCryptoService(store)

err = store.AddKey(trustmanager.KeyInfo{Role: data.CanonicalRootRole, Gun: gun}, privKey)
require.NoError(t, err)

origSignedTestRoot, err := testRoot.ToSigned()
require.NoError(t, err)

err = signed.Sign(cs, origSignedTestRoot, []data.PublicKey{origRootKey}, 1, nil)
require.NoError(t, err)
prevRoot, err := data.RootFromSigned(origSignedTestRoot)
require.NoError(t, err)

// generate a new TUF key with PEM-encoded x509 certificate, not signed by our pinned CA
replRootKey, err := testutils.CreateKey(cs, gun, data.CanonicalRootRole, keyAlg)
require.NoError(t, err)

_, err = data.NewRole(data.CanonicalRootRole, 1, []string{replRootKey.ID()}, nil)
require.NoError(t, err)
newRoot, err := data.NewRoot(
map[string]data.PublicKey{replRootKey.ID(): replRootKey},
map[string]*data.RootRole{
data.CanonicalRootRole: &rootRole.RootRole,
data.CanonicalTimestampRole: &rootRole.RootRole,
data.CanonicalTargetsRole: &rootRole.RootRole,
data.CanonicalSnapshotRole: &rootRole.RootRole},
false,
)
newRoot.Signed.Version = 1
require.NoError(t, err, "Failed to create new root")

newSignedTestRoot, err := newRoot.ToSigned()
require.NoError(t, err)

err = signed.Sign(cs, newSignedTestRoot, []data.PublicKey{replRootKey, origRootKey}, 2, nil)
require.NoError(t, err)

// Check that we respect the trust pinning on rotation
validCAFilepath := "../fixtures/root-ca.crt"
_, err = trustpinning.ValidateRoot(prevRoot, newSignedTestRoot, gun, trustpinning.TrustPinConfig{CA: map[string]string{gun: validCAFilepath}, DisableTOFU: true})
require.Error(t, err)
}

func generateTestingCertificate(rootKey data.PrivateKey, gun string, timeToExpire time.Duration) (*x509.Certificate, error) {
startTime := time.Now()
return cryptoservice.GenerateCertificate(rootKey, gun, startTime, startTime.Add(timeToExpire))
Expand All @@ -805,15 +1010,6 @@ func generateExpiredTestingCertificate(rootKey data.PrivateKey, gun string) (*x5
return cryptoservice.GenerateCertificate(rootKey, gun, startTime, startTime.AddDate(1, 0, 0))
}

// Helper function for explicitly generating key IDs and unexported fields for equality testing
func generateRootKeyIDs(r *data.SignedRoot) {
for _, keyID := range r.Signed.Roles[data.CanonicalRootRole].KeyIDs {
if k, ok := r.Signed.Keys[keyID]; ok {
_ = k.ID()
}
}
}

func TestCheckingCertExpiry(t *testing.T) {
gun := "notary"
pass := func(keyName, alias string, createNew bool, attempts int) (passphrase string, giveup bool, err error) {
Expand Down
Loading