create ephemeral policies for task access to Variables #15875
Labels
stage/needs-discussion
theme/auth
theme/variables
Variables feature
theme/workload-identity
type/enhancement
Comments
tgross
added
stage/needs-discussion
theme/auth
theme/variables
tgross
added a commit
that referenced
this issue
Oct 12, 2023
The RPC handlers expect to see `nil` ACL objects whenever ACLs are disabled. By using `nil` as a sentinel value, we have the risk of nil pointer exceptions and improper handling of `nil` when returned from our various auth methods that can lead to privilege escalation bugs. This is the third in a series to eliminate the use of `nil` ACLs as a sentinel value for when ACLs are disabled. This patch involves leveraging the refactored `auth` package to remove the weird "mixed auth" helper functions that only support the Variables read/list RPC handlers. Instead, pass the ACL object and claim together into the `AllowVariableOperations` method in the usual `acl` package. Ref: hashicorp/nomad-enterprise#1218 Ref: #18703 Ref: #18715 Ref: #16799 Ref: #18730 Fixes: #15875
|
Will be closed by #18744. That doesn't exactly do what's described here, but instead makes it unnecessary. |
tgross
added a commit
that referenced
this issue
Oct 12, 2023
The RPC handlers expect to see `nil` ACL objects whenever ACLs are disabled. By using `nil` as a sentinel value, we have the risk of nil pointer exceptions and improper handling of `nil` when returned from our various auth methods that can lead to privilege escalation bugs. This is the third in a series to eliminate the use of `nil` ACLs as a sentinel value for when ACLs are disabled. This patch involves leveraging the refactored `auth` package to remove the weird "mixed auth" helper functions that only support the Variables read/list RPC handlers. Instead, pass the ACL object and claim together into the `AllowVariableOperations` method in the usual `acl` package. Ref: hashicorp/nomad-enterprise#1218 Ref: #18703 Ref: #18715 Ref: #16799 Ref: #18730 Fixes: #15875
|
I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Workload-Associated ACL Policies are ordinary ACL policies with the appropriate job/group/task fields set. This is suitable for use with most APIs, as shown in #15870. But the implicit Task Access to Variables pseudo-policies need special handling for the Variables RPC, which feels likely to result in a security bug eventually.
The
acl.ACLobjects are expensive to create, so we cache them on the server after they've been resolved from the token (or claim). The cache is keyed by a hash of the policies and not the token, so without considering Task Access to Variables mostacl.ACLobjects we get from WIs are going to be the same and will cache efficiently. Unfortunately when you add Task Access to Variables implicit policies, every WI will have a different cache key and that'll explode the number ofacl.ACLobjects we need to cache if there are a lot of them. This will push user-tokens out of the LRU.A couple of ideas we'll want to explore:
The text was updated successfully, but these errors were encountered: