feat: allow enforcing naming conventions for key names, limiting whic…

…h keys can be fetched from backends (#230) * feat(naming-convention): enforcing naming conventions for AWS Secrets Manager entries closes #178 * chore: fix typo namspace to namespace * chore(naming-convention): early return on isPermitted also improved wording for naming permitted annotation related #178 * docs(naming-convention): initial documentation related #178
external-secrets · Dec 9, 2019 · c4fdea6 · c4fdea6
1 parent 234e907
commit c4fdea6
Show file tree

Hide file tree

Showing 8 changed files with 130 additions and 7 deletions.
diff --git a/README.md b/README.md
@@ -184,6 +184,32 @@ data:
   password: MTIzNA==
 ```
 
+## Enforcing naming conventions for backend keys
+
+by default an `ExternalSecret` may access arbitrary keys from the backend e.g.
+
+```yml
+  data:
+    - key: /dev/cluster1/core-namespace/hello-service/password
+      name: password
+```
+
+An enforced naming convention helps to keep the structure tidy and limits the access according 
+to your naming schema. 
+
+Configure the schema as regular expression in the namespace using an annotation. 
+This allows `ExternalSecrets` in `core-namespace` just to access secrets that start with 
+`/dev/cluster1/core-namespace/`:
+
+```yaml
+kind: Namespace
+metadata:
+  name: core-namespace
+  annotations:
+    # annotation key is configurable
+    externalsecrets.kubernetes-client.io/permitted-key-name: "/dev/cluster1/core-namespace/.*"
+```
+
 ## Deprecations
 
 A few properties has changed name overtime, we still maintain backwards compatbility with these but they will eventually be removed, and they are not validated using the CRD validation.

diff --git a/bin/daemon.js b/bin/daemon.js
@@ -23,7 +23,8 @@ const {
   metricsPort,
   pollerIntervalMilliseconds,
   pollingDisabled,
-  rolePermittedAnnotation
+  rolePermittedAnnotation,
+  namingPermittedAnnotation
 } = require('../config')
 
 async function main () {
@@ -49,6 +50,7 @@ async function main () {
     metrics,
     pollerIntervalMilliseconds,
     rolePermittedAnnotation,
+    namingPermittedAnnotation,
     customResourceManifest,
     pollingDisabled,
     logger

diff --git a/config/environment.js b/config/environment.js
@@ -24,6 +24,7 @@ const logLevel = process.env.LOG_LEVEL || 'info'
 const pollingDisabled = 'DISABLE_POLLING' in process.env
 
 const rolePermittedAnnotation = process.env.ROLE_PERMITTED_ANNOTATION || 'iam.amazonaws.com/permitted'
+const namingPermittedAnnotation = process.env.NAMING_PERMITTED_ANNOTATION || 'externalsecrets.kubernetes-client.io/permitted-key-name'
 
 const metricsPort = process.env.METRICS_PORT || 3001
 
@@ -33,6 +34,7 @@ module.exports = {
   pollerIntervalMilliseconds,
   metricsPort,
   rolePermittedAnnotation,
+  namingPermittedAnnotation,
   pollingDisabled,
   logLevel
 }
diff --git a/e2e/tests/secrets-manager.test.js b/e2e/tests/secrets-manager.test.js
@@ -200,7 +200,7 @@ describe('secretsmanager', async () => {
         .externalsecrets(`e2e-secretmanager-permitted-tls-${uuid}`)
         .get()
       expect(result).to.not.equal(undefined)
-      expect(result.body.status.status).to.contain('namspace does not allow to assume role let-me-be-root')
+      expect(result.body.status.status).to.contain('namespace does not allow to assume role let-me-be-root')
     })
   })
 })
diff --git a/e2e/tests/ssm.test.js b/e2e/tests/ssm.test.js
@@ -122,7 +122,7 @@ describe('ssm', async () => {
         .externalsecrets(`e2e-ssm-permitted-${uuid}`)
         .get()
       expect(result).to.not.equal(undefined)
-      expect(result.body.status.status).to.contain('namspace does not allow to assume role let-me-be-root')
+      expect(result.body.status.status).to.contain('namespace does not allow to assume role let-me-be-root')
     })
   })
 })
diff --git a/lib/poller-factory.js b/lib/poller-factory.js
@@ -19,6 +19,7 @@ class PollerFactory {
     metrics,
     pollerIntervalMilliseconds,
     rolePermittedAnnotation,
+    namingPermittedAnnotation,
     customResourceManifest,
     pollingDisabled,
     logger
@@ -30,6 +31,7 @@ class PollerFactory {
     this._pollerIntervalMilliseconds = pollerIntervalMilliseconds
     this._customResourceManifest = customResourceManifest
     this._rolePermittedAnnotation = rolePermittedAnnotation
+    this._namingPermittedAnnotation = namingPermittedAnnotation
     this._pollingDisabled = pollingDisabled
   }
 
@@ -46,6 +48,7 @@ class PollerFactory {
       metrics: this._metrics,
       customResourceManifest: this._customResourceManifest,
       rolePermittedAnnotation: this._rolePermittedAnnotation,
+      namingPermittedAnnotation: this._namingPermittedAnnotation,
       pollingDisabled: this._pollingDisabled,
       externalSecret
     })

diff --git a/lib/poller.js b/lib/poller.js
@@ -38,6 +38,7 @@ class Poller {
     metrics,
     customResourceManifest,
     rolePermittedAnnotation,
+    namingPermittedAnnotation,
     pollingDisabled,
     externalSecret
   }) {
@@ -49,6 +50,7 @@ class Poller {
     this._metrics = metrics
     this._pollingDisabled = pollingDisabled
     this._rolePermittedAnnotation = rolePermittedAnnotation
+    this._namingPermittedAnnotation = namingPermittedAnnotation
     this._customResourceManifest = customResourceManifest
 
     this._externalSecret = externalSecret
@@ -187,21 +189,49 @@ class Poller {
    * @param {Object} descriptor secret descriptor
    */
   _isPermitted (namespace, descriptor) {
-    const role = descriptor.roleArn
     let allowed = true
     let reason = ''
 
-    if (!namespace.metadata.annotations || !role) {
+    if (!namespace.metadata.annotations) {
       return {
         allowed, reason
       }
     }
+
+    // 1. testing naming convention if defined in namespace
+
+    const externalData = descriptor.data || descriptor.properties
+    const namingConvention = namespace.metadata.annotations[this._namingPermittedAnnotation]
+
+    if (namingConvention) {
+      externalData.forEach((secretProperty, index) => {
+        const reNaming = new RegExp(namingConvention)
+        if (!reNaming.test(secretProperty.key)) {
+          allowed = false
+          reason = `key name does not match naming convention ${namingConvention}`
+          return {
+            allowed, reason
+          }
+        }
+      })
+    }
+
+    // 2. testing assume role if configured
+
+    const role = descriptor.roleArn
+
+    if (!role) {
+      return {
+        allowed, reason
+      }
+    }
+
     // an empty annotation value allows access to all roles
     const re = new RegExp(namespace.metadata.annotations[this._rolePermittedAnnotation])
 
     if (!re.test(role)) {
       allowed = false
-      reason = `namspace does not allow to assume role ${role}`
+      reason = `namespace does not allow to assume role ${role}`
     }
 
     return {

diff --git a/lib/poller.test.js b/lib/poller.test.js
@@ -26,6 +26,7 @@ describe('Poller', () => {
   })
 
   const rolePermittedAnnotation = 'iam.amazonaws.com/permitted'
+  const namingPermittedAnnotation = 'externalsecrets.kubernetes-client.io/permitted-key-name'
 
   beforeEach(() => {
     backendMock = sinon.mock()
@@ -93,6 +94,7 @@ describe('Poller', () => {
         logger: loggerMock,
         externalSecret: fakeExternalSecret,
         rolePermittedAnnotation,
+        namingPermittedAnnotation,
         customResourceManifest: fakeCustomResourceManifest
       })
     }
@@ -636,7 +638,7 @@ describe('Poller', () => {
       }
 
       expect(error).to.not.equal(undefined)
-      expect(error.message).equals('not allowed to fetch secret: fakeNamespace/fakeSecretName: namspace does not allow to assume role arn:aws:iam::123456789012:role/test-role')
+      expect(error.message).equals('not allowed to fetch secret: fakeNamespace/fakeSecretName: namespace does not allow to assume role arn:aws:iam::123456789012:role/test-role')
     })
 
     it('fails storing secret', async () => {
@@ -745,6 +747,64 @@ describe('Poller', () => {
         }
       ]
 
+      for (let i = 0; i < testcases.length; i++) {
+        const testcase = testcases[i]
+        const verdict = poller._isPermitted(testcase.ns, testcase.descriptor)
+        expect(verdict.allowed).to.equal(testcase.permitted)
+      }
+    })
+  })
+  describe('nameing conventions', () => {
+    let poller
+    beforeEach(() => {
+      poller = pollerFactory()
+    })
+    it('should restrict access as defined in namespace naming convention ', () => {
+      const testcases = [
+        {
+          // no annotations at all
+          ns: { metadata: {} },
+          descriptor: {},
+          permitted: true
+        },
+        {
+          // empty annotation
+          ns: { metadata: { annotations: { [namingPermittedAnnotation]: '' } } },
+          descriptor: {},
+          permitted: true
+        },
+        {
+          // test regex
+          ns: { metadata: { annotations: { [namingPermittedAnnotation]: '.*' } } },
+          descriptor: {
+            data: [
+              { key: 'whatever', name: 'somethingelse' }
+            ]
+          },
+          permitted: true
+        },
+        {
+          // test regex
+          ns: { metadata: { annotations: { [namingPermittedAnnotation]: 'dev/team-a/.*' } } },
+          descriptor: {
+            data: [
+              { key: 'dev/team-a/secret', name: 'somethingelse' }
+            ]
+          },
+          permitted: true
+        },
+        {
+          // test regex
+          ns: { metadata: { annotations: { [namingPermittedAnnotation]: 'dev/team-a/.*' } } },
+          descriptor: {
+            data: [
+              { key: 'dev/team-b/secret', name: 'somethingelse' }
+            ]
+          },
+          permitted: false
+        }
+      ]
+
       for (let i = 0; i < testcases.length; i++) {
         const testcase = testcases[i]
         const verdict = poller._isPermitted(testcase.ns, testcase.descriptor)