Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

auth: enforce use of node secret and remove legacy auth #23838

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

tgross
Copy link
Member

@tgross tgross commented Aug 16, 2024

As of Nomad 1.6.0, Nomad client agents send their secret with all the RPCs (other than registration). But for backwards compatibility we had to keep a legacy auth method that didn't require the node secret. We've previously announced that this legacy auth method would be removed and that nodes older than 1.6.0 would not be supported with Nomad 1.9.0.

This changeset removes the legacy auth method.

Ref: #16799
Ref: https://hashicorp.atlassian.net/browse/NET-10760
Ref: https://hashicorp.atlassian.net/browse/NET-10009
Ref: https://developer.hashicorp.com/nomad/docs/release-notes/nomad/upcoming#nomad-1-9-0

Copy link
Member

@gulducat gulducat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

As of Nomad 1.6.0, Nomad client agents send their secret with all the
RPCs (other than registration). But for backwards compatibility we had to keep
a legacy auth method that didn't require the node secret. We've previously
announced that this legacy auth method would be removed and that nodes older
than 1.6.0 would not be supported with Nomad 1.9.0.

This changeset removes the legacy auth method.

Ref: https://developer.hashicorp.com/nomad/docs/release-notes/nomad/upcoming#nomad-1-9-0
@tgross tgross force-pushed the node-endpoint-auth-updates branch from 4bf12d8 to 6a68eaf Compare September 5, 2024 17:53
@tgross
Copy link
Member Author

tgross commented Sep 5, 2024

Had to resolve a merge conflict on the upgrade guide. Will merge once CI is green again.

tgross added a commit that referenced this pull request Oct 17, 2024
In #23838 we updated the `Node.Update` RPC handler we use for heartbeats to be
more strict about requiring node secrets. But when a node goes down, it's the
leader that sends the request to mark the node down via `Node.Update` (to
itself), and this request was missing the leader ACL needed to authenticate to
itself.

Add the leader ACL to the request and update the RPC handler test for
disconnected-clients to use ACLs, which would have detected this bug. Also added
a note to the `Authenticate` comment about how that authentication path requires
the leader ACL.

Fixes: #24231
Ref: https://hashicorp.atlassian.net/browse/NET-11384
tgross added a commit that referenced this pull request Oct 17, 2024
In #23838 we updated the `Node.Update` RPC handler we use for heartbeats to be
more strict about requiring node secrets. But when a node goes down, it's the
leader that sends the request to mark the node down via `Node.Update` (to
itself), and this request was missing the leader ACL needed to authenticate to
itself.

Add the leader ACL to the request and update the RPC handler test for
disconnected-clients to use ACLs, which would have detected this bug. Also added
a note to the `Authenticate` comment about how that authentication path requires
the leader ACL.

Fixes: #24231
Ref: https://hashicorp.atlassian.net/browse/NET-11384
Copy link

github-actions bot commented Jan 4, 2025

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 4, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants