Skip to content
This repository has been archived by the owner on Jul 26, 2022. It is now read-only.

Enforcing naming conventions for AWS Secrets Manager (and HashiCorp Vault) entries #230

Merged
merged 4 commits into from
Dec 9, 2019
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
26 changes: 26 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion bin/daemon.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ const {
metricsPort,
pollerIntervalMilliseconds,
pollingDisabled,
rolePermittedAnnotation
rolePermittedAnnotation,
namingPermittedAnnotation
} = require('../config')

async function main () {
Expand All @@ -49,6 +50,7 @@ async function main () {
metrics,
pollerIntervalMilliseconds,
rolePermittedAnnotation,
namingPermittedAnnotation,
customResourceManifest,
pollingDisabled,
logger
Expand Down
2 changes: 2 additions & 0 deletions config/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -33,6 +34,7 @@ module.exports = {
pollerIntervalMilliseconds,
metricsPort,
rolePermittedAnnotation,
namingPermittedAnnotation,
pollingDisabled,
logLevel
}
2 changes: 1 addition & 1 deletion e2e/tests/secrets-manager.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
})
})
2 changes: 1 addition & 1 deletion e2e/tests/ssm.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
})
})
3 changes: 3 additions & 0 deletions lib/poller-factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class PollerFactory {
metrics,
pollerIntervalMilliseconds,
rolePermittedAnnotation,
namingPermittedAnnotation,
customResourceManifest,
pollingDisabled,
logger
Expand All @@ -30,6 +31,7 @@ class PollerFactory {
this._pollerIntervalMilliseconds = pollerIntervalMilliseconds
this._customResourceManifest = customResourceManifest
this._rolePermittedAnnotation = rolePermittedAnnotation
this._namingPermittedAnnotation = namingPermittedAnnotation
this._pollingDisabled = pollingDisabled
}

Expand All @@ -46,6 +48,7 @@ class PollerFactory {
metrics: this._metrics,
customResourceManifest: this._customResourceManifest,
rolePermittedAnnotation: this._rolePermittedAnnotation,
namingPermittedAnnotation: this._namingPermittedAnnotation,
pollingDisabled: this._pollingDisabled,
externalSecret
})
Expand Down
36 changes: 33 additions & 3 deletions lib/poller.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class Poller {
metrics,
customResourceManifest,
rolePermittedAnnotation,
namingPermittedAnnotation,
pollingDisabled,
externalSecret
}) {
Expand All @@ -49,6 +50,7 @@ class Poller {
this._metrics = metrics
this._pollingDisabled = pollingDisabled
this._rolePermittedAnnotation = rolePermittedAnnotation
this._namingPermittedAnnotation = namingPermittedAnnotation
this._customResourceManifest = customResourceManifest

this._externalSecret = externalSecret
Expand Down Expand Up @@ -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}`
muenchhausen marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down
62 changes: 61 additions & 1 deletion lib/poller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -93,6 +94,7 @@ describe('Poller', () => {
logger: loggerMock,
externalSecret: fakeExternalSecret,
rolePermittedAnnotation,
namingPermittedAnnotation,
customResourceManifest: fakeCustomResourceManifest
})
}
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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)
Expand Down