Skip to content

[v17] MWI: Minimal bound-keypair joining implementation (#54371)#55037

Merged
timothyb89 merged 5 commits intobranch/v17from
timothyb89/v17/bound-keypair-minimal-impl
Aug 16, 2025
Merged

[v17] MWI: Minimal bound-keypair joining implementation (#54371)#55037
timothyb89 merged 5 commits intobranch/v17from
timothyb89/v17/bound-keypair-minimal-impl

Conversation

@timothyb89
Copy link
Copy Markdown
Contributor

@timothyb89 timothyb89 commented May 22, 2025

Backport of #54371 for branch/v17

changelog: Machine and Workload ID: Add experimental implementation of new bound_keypair join method for improved bot joining in on-prem environments


MWI: Minimal bound-keypair joining implementation

This includes a minimal implementation of bound-keypair joining. This first iteration requires preregistered public keys, and requires unlimited and insecure flags to be set on bound keypair tokens.

Minimal client-side implementation will be in a follow up PR.

RFD: #52546
Closes #53373

  • Refactor challenge response function, rebase on updated protos branch

This includes a number of changes:

  • Rebases on the latest protos branch. This includes removal of the new keypair field on initial join, and adds messages for interactive keypair rotation.
  • Per the rebase, remaining_joins is removed in favor of using join_count for all calculations. The registration method and validatity checks have been updated to reference that instead.
  • Refactors challenge response function to allow for keypair rotation. We still don't implement rotation but the handler now receives the full proto message and produces a full proto response, so that we can easily handle the rotation case in the future.
  • Challenge validation checks time fields explicitly to ensure the client didn't tamper with them.
  • Added some missing docstrings
  • Add joinserver test

  • Fix lint error and add docstring

  • Add tests for bound keypair challenge validation

  • Remove client side package intended for other PR

  • Fix various lints

  • Add tests for RegisterUsingBoundKeypairMethod()

  • Fix lints

  • Add basic provisioning token CheckAndSetDefaults() tests

  • Include bound public key in RegisterUsingBoundKeypairMethod return

This is passed back to clients as part of the proto certs message as confirmation that rotation succeeded, so the value needed to be plumbed through.

  • Fixes after upstream proto change

We renamed and tweaked a number of proto fields, so this updates field references.

  • Apply suggestions from code review

  • Remove TODO

  • Fix missed field rename

  • Fix broken test

  • Fix lurking nil pointer deref after field rename


@github-actions github-actions bot requested review from greedy52 and gzdunek May 22, 2025 00:53
Base automatically changed from timothyb89/v17/bound-keypair-protos to branch/v17 May 22, 2025 20:55
* MWI: Minimal bound-keypair joining implementation

This includes a minimal implementation of bound-keypair joining. This
first iteration requires preregistered public keys, and requires
`unlimited` and `insecure` flags to be set on bound keypair tokens.

Minimal client-side implementation will be in a follow up PR.

RFD: #52546
Closes #53373

* Refactor challenge response function, rebase on updated protos branch

This includes a number of changes:
- Rebases on the latest protos branch. This includes removal of the
  new keypair field on initial join, and adds messages for
  interactive keypair rotation.
- Per the rebase, remaining_joins is removed in favor of using
  join_count for all calculations. The registration method and
  validatity checks have been updated to reference that instead.
- Refactors challenge response function to allow for keypair
  rotation. We still don't implement rotation but the handler now
  receives the full proto message and produces a full proto response,
  so that we can easily handle the rotation case in the future.
- Challenge validation checks time fields explicitly to ensure the
  client didn't tamper with them.
- Added some missing docstrings

* Add joinserver test

* Fix lint error and add docstring

* Add tests for bound keypair challenge validation

* Remove client side package intended for other PR

* Fix various lints

* Add tests for RegisterUsingBoundKeypairMethod()

* Fix lints

* Add basic provisioning token CheckAndSetDefaults() tests

* Include bound public key in RegisterUsingBoundKeypairMethod return

This is passed back to clients as part of the proto certs message as
confirmation that rotation succeeded, so the value needed to be
plumbed through.

* Fixes after upstream proto change

We renamed and tweaked a number of proto fields, so this updates
field references.

* Apply suggestions from code review

Co-authored-by: Dan Upton <daniel.upton@goteleport.com>

* Remove TODO

* Fix missed field rename

* Fix broken test

* Fix lurking nil pointer deref after field rename

---------

Co-authored-by: Dan Upton <daniel.upton@goteleport.com>
@timothyb89 timothyb89 force-pushed the timothyb89/v17/bound-keypair-minimal-impl branch from a08564c to 1b70a74 Compare May 22, 2025 22:35
@timothyb89
Copy link
Copy Markdown
Contributor Author

timothyb89 commented Aug 16, 2025

This is positively ancient at this point, but I've finally fixed this up and tested it against a cloud tenant. Backports continue in #57961. I don't consider anything in this PR to be risky for any particular v17 release; I think the last PR (removing the experiment flag) is probably the only one that might warrant a minor release.

@timothyb89 timothyb89 added this pull request to the merge queue Aug 16, 2025
Merged via the queue into branch/v17 with commit 0cbe0f0 Aug 16, 2025
39 checks passed
@timothyb89 timothyb89 deleted the timothyb89/v17/bound-keypair-minimal-impl branch August 16, 2025 01:31
@doggydogworld doggydogworld mentioned this pull request Aug 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants