Ability to Use Regex-Based Validation for Sentinel, Safe, and Workload SPIFFE IDs #974
Conversation
|
@v0lkan, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
core/env/spiffe.go
Outdated
| @@ -22,3 +22,11 @@ func SpiffeSocketUrl() string { | |||
| } | |||
| return p | |||
| } | |||
|
|
|||
| func SpiffeTrustDomain() string { | |||
There was a problem hiding this comment.
If defined, the system will assume this one.
Of course, SPIRE should be configured accordingly for the new trust domain to work without issues.
core/env/spiffeid.go
Outdated
| trustDomain := SpiffeTrustDomain() | ||
| ns := NamespaceForVSecMSystem() | ||
|
|
||
| p = "spiffe://" + trustDomain + "/workload/" + |
There was a problem hiding this comment.
It makes sense to have trust domain and namespace configurable.
There is not much of a use case that necessitates configuring the service account too; so it has been left hard-coded.
core/validation/validation.go
Outdated
| } | ||
|
|
||
| // IsWorkload returns true if the given SPIFFEID is a WorkloadIds ID. | ||
| // It does this by checking if the SPIFFEID has the SpiffeIdPrefixForWorkload | ||
| // as its prefix. | ||
| func IsWorkload(spiffeid string) bool { | ||
| return strings.HasPrefix(spiffeid, env.SpiffeIdPrefixForWorkload()) | ||
| prefix := env.SpiffeIdPrefixForWorkload() |
There was a problem hiding this comment.
This should be validated both at the workload, and also at VSecM safe.
A malicious workload can easily fake or bypass configuration-based validations that depend on an environment variable or a mounted config file.
The authentication/validation should be mutual, and we should guard the security of core VSecM components with tight RBAC.
Or an alternative is to use a configuration file that can be accessed through a read-only API. But if the workload is a third-party workload that can opt-out from such validation, it is of little use. -- and the env vars that vsecm components use (such as safe and sentinel, etc) cannot be tampered with unless changing the source code of those components and recompiling them (or having a malicious cluster admin misconfiguring them, which then all bets will be off anyway.)
|
@v0lkan, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
Signed-off-by: Volkan Ozcelik <[email protected]>
Signed-off-by: Volkan Ozcelik <[email protected]>
|
Tests are passing… merging this one. |
|
I don’t see some of my changes here. Will create another PR. |
Regex-Based SPIFFE ID Validation
Description
This PR introduces regular-expression-based spiffe id validation.
In addition, we do additional sanitization checks on SPIFFE IDs (such as checking whether a SPIFFE ID starts with 'spiffe://' before accepting it.
Note that we will need a separate integration test, and also documentation updates, which are not a part of this PR and will be addressed by follow-up PRs.
Changes
Test Policy Compliance
Not created tests; I will amend tests as a follow-up PR.
I will also ensure that this branch passes the current tests before merging.
Documentation needs updating too; which, again will be done in a follow-up PR.
Code Quality
to understand.
Documentation
Documentation will be udpated as a follow-up PR.
Checklist
Before you submit this PR, please make sure:
especially the test policy.
under the project's license.
By submitting this pull request, you confirm that my contribution is made under
the terms of the project's license and that you have the authority to grant
these rights.
Thank you for your contribution to VMware Secrets Manager
🐢⚡️!