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 2 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
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 || 'iam.amazonaws.com/naming'

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
29 changes: 26 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,42 @@ 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
}
}

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}`
}
})
}

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 = 'iam.amazonaws.com/naming'

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