From 274360ed2119b9d4ad5958d43ce3631c4d0c4565 Mon Sep 17 00:00:00 2001
From: Vincent Janelle <vjanelle@salesforce.com>
Date: Sun, 11 Nov 2018 09:57:51 -0800
Subject: [PATCH] (#16) Validate certificate before attempting to cache it

* If you change the allow list, a cached cert will be allowed for non-privileged actions.  This is not intuitive.
* Make tests pass

```
filesec/file_security.go:404: Entry.Errorf format %s arg s.caPath is a func value, not called
```

* Allow list tests to ensure this doesn't happen again
---
 filesec/file_security.go      | 12 ++++++------
 filesec/file_security_test.go | 17 +++++++++++++++++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/filesec/file_security.go b/filesec/file_security.go
index fb98aac..63cae05 100644
--- a/filesec/file_security.go
+++ b/filesec/file_security.go
@@ -319,6 +319,10 @@ func (s *FileSecurity) CachePublicData(data []byte, identity string) error {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
+	if !s.shouldCacheClientCert(data, identity) {
+		return fmt.Errorf("certificate '%s' did not pass validation", identity)
+	}
+
 	err := os.MkdirAll(s.certCacheDir(), os.FileMode(int(0755)))
 	if err != nil {
 		return fmt.Errorf("could not create Client Certificate Cache Directory: %s", err)
@@ -335,10 +339,6 @@ func (s *FileSecurity) CachePublicData(data []byte, identity string) error {
 		return nil
 	}
 
-	if !s.shouldCacheClientCert(data, identity) {
-		return fmt.Errorf("certificate '%s' did not pass validation", identity)
-	}
-
 	err = ioutil.WriteFile(certfile, []byte(data), os.FileMode(int(0644)))
 	if err != nil {
 		return fmt.Errorf("could not cache client public certificate: %s", err.Error())
@@ -401,7 +401,7 @@ func (s *FileSecurity) VerifyCertificate(certpem []byte, name string) error {
 	ca := s.caPath()
 	capem, err := ioutil.ReadFile(ca)
 	if err != nil {
-		s.log.Errorf("Could not read CA '%s': %s", s.caPath, err)
+		s.log.Errorf("Could not read CA '%s': %s", ca, err)
 		return err
 	}
 
@@ -623,7 +623,7 @@ func (s *FileSecurity) certCacheDir() string {
 
 func (s *FileSecurity) shouldCacheClientCert(data []byte, name string) bool {
 	if err := s.VerifyCertificate(data, ""); err != nil {
-		s.log.Warnf("Received certificate '%s' certiicate did not pass verification: %s", name, err)
+		s.log.Warnf("Received certificate '%s' certificate did not pass verification: %s", name, err)
 		return false
 	}
 
diff --git a/filesec/file_security_test.go b/filesec/file_security_test.go
index 1a9e527..80c1212 100644
--- a/filesec/file_security_test.go
+++ b/filesec/file_security_test.go
@@ -504,6 +504,23 @@ var _ = Describe("FileSSL", func() {
 			Expect(err).ToNot(HaveOccurred())
 			Expect(stat.Size()).To(Equal(int64(16)))
 		})
+
+		It("Should fail cache validation if allow lists change", func() {
+			cfg.Cache = os.TempDir()
+			cfg.Cache = os.TempDir()
+			pub := prov.publicCertPath()
+
+			pd, err := ioutil.ReadFile(pub)
+			Expect(err).ToNot(HaveOccurred())
+
+			err = prov.CachePublicData(pd, "rip.mcollective")
+			Expect(err).ToNot(HaveOccurred())
+
+			cfg.AllowList = []string{"^bees$"}
+
+			err = prov.CachePublicData(pd, "rip.mcollective")
+			Expect(err).To(HaveOccurred())
+		})
 	})
 
 	Describe("CachedPublicData", func() {