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

Machine ID: Provide existing identity when rejoining as part of renewals #43732

Merged
merged 9 commits into from
Jul 23, 2024

Conversation

strideynet
Copy link
Contributor

@strideynet strideynet commented Jul 2, 2024

Part of #41456

Uses the existing identity (e.g an authenticated client) during renewal using non-renewable join methods, rather than an unauthenticated client. This is supporting work for the upcoming Bot Instance feature. This is safe to merge without the other changes.

Manually tested:

  • token
    • initially registers, renews as expected on next interval
    • loaded from store, renews
    • expired value, leads to error as token cannot be used to re-register
    • changed value, re-registered
  • gcp
    • initially registers, renews as expected on next interval
    • loaded from store, renews
    • expired value, succeeds as it rejoins

changelog: Machine ID now attempts to load the existing identity when using delegated join methods - this will support tracking a single tbot instance across its lifetime.

@strideynet strideynet added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v15 backport/branch/v16 labels Jul 2, 2024
@strideynet strideynet mentioned this pull request Jul 2, 2024
10 tasks
// If no identity is found, or the identity is no longer valid, a new identity
// is requested using the configured onboarding settings.
// If there is no identity, or the identity is invalid, we'll join using the
// configured onboarding settings.
func (s *identityService) Initialize(ctx context.Context) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One of the key behavioural changes here is that previously, we wouldn't even attempt to load from persisted storage if a bot was configured with a renewable join method. Now we will. This does introduce a little bit of risk, but, this is hedged by the fact that we handle being unable to load the identity by just attempting to join, and if we think the identity might be invalid (e.g expired), we will also just attempt to join.

@strideynet strideynet marked this pull request as ready for review July 2, 2024 13:59
@strideynet strideynet removed the no-changelog Indicates that a PR does not require a changelog entry label Jul 2, 2024
timothyb89 added a commit that referenced this pull request Jul 16, 2024
Traditionally, bots using non-`token` join methods "renew" their
certs by rejoining, taking advantage of their repeatable join methods
to avoid relying on `token` rejoin logic. However, as part of
[RFD 162], we need a way for rejoining bots to assert an existing
identity (if any) even when rejoining "from scratch".

When paired with #43732, this change allows bots to use an
authenticated auth client for all "initial" joins. If present, an
existing `BotInstanceID` is extracted from the client identity and
used to associate the bot with its previous identity.

As certain join methods (TPM, Azure) are initially handled by the
`JoinService` using gRPC, this additionally adds a new
`BotInstanceID` field to `RegisterUsingTokenRequest` so the
`JoinService` can verify the client identity and extract the
`BotInstanceID` if available. Like `RemoteAddr`, this value is only
trusted when set by the proxy and should be ignored in all other
cases.

[RFD 162]: #36510
Copy link
Contributor

@timothyb89 timothyb89 left a comment

Choose a reason for hiding this comment

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

This has been working great along with #44257

@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from fheinecke July 23, 2024 08:17
@strideynet strideynet added this pull request to the merge queue Jul 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 23, 2024
@strideynet strideynet added this pull request to the merge queue Jul 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 23, 2024
@strideynet strideynet added this pull request to the merge queue Jul 23, 2024
Merged via the queue into master with commit 73793e7 Jul 23, 2024
38 checks passed
@strideynet strideynet deleted the strideynet/reuse-client-when-reregistering branch July 23, 2024 12:25
@public-teleport-github-review-bot

@strideynet See the table below for backport results.

Branch Result
branch/v15 Failed
branch/v16 Failed

github-merge-queue bot pushed a commit that referenced this pull request Jul 23, 2024
* Allow bots to provide client cert authentication when rejoining

Traditionally, bots using non-`token` join methods "renew" their
certs by rejoining, taking advantage of their repeatable join methods
to avoid relying on `token` rejoin logic. However, as part of
[RFD 162], we need a way for rejoining bots to assert an existing
identity (if any) even when rejoining "from scratch".

When paired with #43732, this change allows bots to use an
authenticated auth client for all "initial" joins. If present, an
existing `BotInstanceID` is extracted from the client identity and
used to associate the bot with its previous identity.

As certain join methods (TPM, Azure) are initially handled by the
`JoinService` using gRPC, this additionally adds a new
`BotInstanceID` field to `RegisterUsingTokenRequest` so the
`JoinService` can verify the client identity and extract the
`BotInstanceID` if available. Like `RemoteAddr`, this value is only
trusted when set by the proxy and should be ignored in all other
cases.

[RFD 162]: #36510

* Add tests for authenticated bot rejoining

* Create a new bot instance if the requested instance is not found

Also, includes a new check in for this behavior in
`TestRegisterBotWithInvalidInstanceID`.

* Fix imports

* Update lib/auth/auth_with_roles.go

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Fix `cmp` import

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
timothyb89 pushed a commit that referenced this pull request Aug 6, 2024
…als (#43732)

* Allow existing bot identity to be used for token based renewals

* Add test for register with auth client

* Fix tests

* Fix test

* Fix partial assignment to interface

* Improve loggging

* Fix spelling

* Fix missing import
timothyb89 added a commit that referenced this pull request Aug 6, 2024
* Allow bots to provide client cert authentication when rejoining

Traditionally, bots using non-`token` join methods "renew" their
certs by rejoining, taking advantage of their repeatable join methods
to avoid relying on `token` rejoin logic. However, as part of
[RFD 162], we need a way for rejoining bots to assert an existing
identity (if any) even when rejoining "from scratch".

When paired with #43732, this change allows bots to use an
authenticated auth client for all "initial" joins. If present, an
existing `BotInstanceID` is extracted from the client identity and
used to associate the bot with its previous identity.

As certain join methods (TPM, Azure) are initially handled by the
`JoinService` using gRPC, this additionally adds a new
`BotInstanceID` field to `RegisterUsingTokenRequest` so the
`JoinService` can verify the client identity and extract the
`BotInstanceID` if available. Like `RemoteAddr`, this value is only
trusted when set by the proxy and should be ignored in all other
cases.

[RFD 162]: #36510

* Add tests for authenticated bot rejoining

* Create a new bot instance if the requested instance is not found

Also, includes a new check in for this behavior in
`TestRegisterBotWithInvalidInstanceID`.

* Fix imports

* Update lib/auth/auth_with_roles.go

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Fix `cmp` import

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
timothyb89 pushed a commit that referenced this pull request Aug 8, 2024
…als (#43732)

* Allow existing bot identity to be used for token based renewals

* Add test for register with auth client

* Fix tests

* Fix test

* Fix partial assignment to interface

* Improve loggging

* Fix spelling

* Fix missing import
timothyb89 added a commit that referenced this pull request Aug 8, 2024
* Allow bots to provide client cert authentication when rejoining

Traditionally, bots using non-`token` join methods "renew" their
certs by rejoining, taking advantage of their repeatable join methods
to avoid relying on `token` rejoin logic. However, as part of
[RFD 162], we need a way for rejoining bots to assert an existing
identity (if any) even when rejoining "from scratch".

When paired with #43732, this change allows bots to use an
authenticated auth client for all "initial" joins. If present, an
existing `BotInstanceID` is extracted from the client identity and
used to associate the bot with its previous identity.

As certain join methods (TPM, Azure) are initially handled by the
`JoinService` using gRPC, this additionally adds a new
`BotInstanceID` field to `RegisterUsingTokenRequest` so the
`JoinService` can verify the client identity and extract the
`BotInstanceID` if available. Like `RemoteAddr`, this value is only
trusted when set by the proxy and should be ignored in all other
cases.

[RFD 162]: #36510

* Add tests for authenticated bot rejoining

* Create a new bot instance if the requested instance is not found

Also, includes a new check in for this behavior in
`TestRegisterBotWithInvalidInstanceID`.

* Fix imports

* Update lib/auth/auth_with_roles.go

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Fix `cmp` import

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
github-merge-queue bot pushed a commit that referenced this pull request Aug 26, 2024
* Add basic CRUD for BotInstanceService (#42899)

* Add basic CRUD for BotInstanceService

This adds an early backend for the bot instance service, with resource
definitions and basic CRUD operations.

* Update api/proto/teleport/machineid/v1/bot_instance.proto

Co-authored-by: Noah Stride <noah.stride@goteleport.com>

* Further work on BotInstance boilerplate

Among other things, this adds event definitions, RBAC and presets,
and client definitions. This also includes a few proto changes,
namely moving bot name and instance ID from .Status to .Spec so
metadata can be uniformly generated.

This also adds tests for the backend service showing functional CRUD.

* gRPC tests; fix filtering; fix import lints; misc cleanup

* Consistently use `instance_id` in gRPC fields

* Add tctl support for bot_instance resource

* protos update

* Various typo fixes

* Fix more comment typos

* More doc comments and minor test tweaks

---------

Co-authored-by: Noah Stride <noah.stride@goteleport.com>

* Create Bot Instances during initial bot join (#43577)

* Create Bot Instances during initial bot join

This creates new instances for bots when they initially join the
cluster, and persists instance IDs in new certificate fields on join
and during renewal.

Note that this does not yet handle instance reuse for non-token join
methods.

Additionally, bot instance creation is locked behind a
`BOT_INSTANCE_EXPERIMENT` flag; it must be set to `1` to enable
creation.

* Proto cleanup, and update bot auth records on cert renewal

This makes various (admittedly breaking) protobuf changes, including
removing the TTL field (calculating resource expiry based on cert
requests), removing public key fingerprints, and changing the data
type of the generation counter to match the preexisting internal
datatype. These changes _should_ be safe as no consumers of the proto
API currently exist.

Additionally, this also updates bot authentications on renewal.

* Fix proto lints

* Fix misleading doc comment in the bot instance experiment

* Create bot instances for old bots on join; other fixes

This now creates bot instances for bots whose certs are missing the
BotInstanceID field. Additionally, it fixes two related bugs:
expiration dates are extended on renewal, the generated UUID
is properly appended to certs on initial join, and instances are
only created or updated when the experiment is enabled.

* Add a minimal test for bot instance creation on initial join

* Validate bot instance state in generation counter checks

* Remove outdated TODO comment and fix test lints

* Add an expiration change check to the generation test

* Machine ID: Provide existing identity when rejoining as part of renewals (#43732)

* Allow existing bot identity to be used for token based renewals

* Add test for register with auth client

* Fix tests

* Fix test

* Fix partial assignment to interface

* Improve loggging

* Fix spelling

* Fix missing import

* Fix built gRPC

* Machine ID: Bot Instance heartbeating (#43463)

* Add basic CRUD for BotInstanceService

This adds an early backend for the bot instance service, with resource
definitions and basic CRUD operations.

* Update api/proto/teleport/machineid/v1/bot_instance.proto

Co-authored-by: Noah Stride <noah.stride@goteleport.com>

* Further work on BotInstance boilerplate

Among other things, this adds event definitions, RBAC and presets,
and client definitions. This also includes a few proto changes,
namely moving bot name and instance ID from .Status to .Spec so
metadata can be uniformly generated.

This also adds tests for the backend service showing functional CRUD.

* gRPC tests; fix filtering; fix import lints; misc cleanup

* Consistently use `instance_id` in gRPC fields

* Add tctl support for bot_instance resource

* protos update

* Various typo fixes

* Fix more comment typos

* More doc comments and minor test tweaks

* Add SubmitHeartbeat RPC implementation

* Fix log keys

* Update to use proper BotInstanceID field in identiy

* Add rough outline of heartbeat service

---------

Co-authored-by: Tim Buckley <tim@goteleport.com>

* Allow bots to provide client cert authentication when rejoining (#44257)

* Allow bots to provide client cert authentication when rejoining

Traditionally, bots using non-`token` join methods "renew" their
certs by rejoining, taking advantage of their repeatable join methods
to avoid relying on `token` rejoin logic. However, as part of
[RFD 162], we need a way for rejoining bots to assert an existing
identity (if any) even when rejoining "from scratch".

When paired with #43732, this change allows bots to use an
authenticated auth client for all "initial" joins. If present, an
existing `BotInstanceID` is extracted from the client identity and
used to associate the bot with its previous identity.

As certain join methods (TPM, Azure) are initially handled by the
`JoinService` using gRPC, this additionally adds a new
`BotInstanceID` field to `RegisterUsingTokenRequest` so the
`JoinService` can verify the client identity and extract the
`BotInstanceID` if available. Like `RemoteAddr`, this value is only
trusted when set by the proxy and should be ignored in all other
cases.

[RFD 162]: #36510

* Add tests for authenticated bot rejoining

* Create a new bot instance if the requested instance is not found

Also, includes a new check in for this behavior in
`TestRegisterBotWithInvalidInstanceID`.

* Fix imports

* Update lib/auth/auth_with_roles.go

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Fix `cmp` import

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Machine ID: Include Bot Instance ID in audit logs (#43786)

* Create Bot Instances during initial bot join

This creates new instances for bots when they initially join the
cluster, and persists instance IDs in new certificate fields on join
and during renewal.

Note that this does not yet handle instance reuse for non-token join
methods.

Additionally, bot instance creation is locked behind a
`BOT_INSTANCE_EXPERIMENT` flag; it must be set to `1` to enable
creation.

* Proto cleanup, and update bot auth records on cert renewal

This makes various (admittedly breaking) protobuf changes, including
removing the TTL field (calculating resource expiry based on cert
requests), removing public key fingerprints, and changing the data
type of the generation counter to match the preexisting internal
datatype. These changes _should_ be safe as no consumers of the proto
API currently exist.

Additionally, this also updates bot authentications on renewal.

* Fix proto lints

* Fix misleading doc comment in the bot instance experiment

* Create bot instances for old bots on join; other fixes

This now creates bot instances for bots whose certs are missing the
BotInstanceID field. Additionally, it fixes two related bugs:
expiration dates are extended on renewal, the generated UUID
is properly appended to certs on initial join, and instances are
only created or updated when the experiment is enabled.

* Add a minimal test for bot instance creation on initial join

* Validate bot instance state in generation counter checks

* Remove outdated TODO comment and fix test lints

* Add an expiration change check to the generation test

* Add BotInstanceID to audit events

* Fix borked conflict resolution

* Fix further borked conflict resolution

* Add test for cert create/join

---------

Co-authored-by: Tim Buckley <tim@goteleport.com>

* Show bot instance ID in tbot log output (#44578)

This tweaks the "fetched new bot identity" message to show the bot
name and instance ID as embedded in the bot's certificate.

Example:

```
2024-07-23T15:51:20-06:00 INFO [TBOT:IDEN] Fetched new bot identity identity:tpm-test, id=5a2865d3-d3dc-4eaa-853c-5377a1fe83f6 | valid: after=2024-07-23T21:50:20Z, before=2024-07-23T21:56:19Z, duration=5m59s | kind=tls, renewable=false, disallow-reissue=false, roles=[bot-tpm-test], principals=[-teleport-internal-join], generation=0 tbot/service_bot_identity.go:223
```

* Machine ID: Validate generation counter using bot instances (#44583)

* Show bot instance ID in tbot log output

This tweaks the "fetched new bot identity" message to show the bot
name and instance ID as embedded in the bot's certificate.

Example:

```
2024-07-23T15:51:20-06:00 INFO [TBOT:IDEN] Fetched new bot identity identity:tpm-test, id=5a2865d3-d3dc-4eaa-853c-5377a1fe83f6 | valid: after=2024-07-23T21:50:20Z, before=2024-07-23T21:56:19Z, duration=5m59s | kind=tls, renewable=false, disallow-reissue=false, roles=[bot-tpm-test], principals=[-teleport-internal-join], generation=0 tbot/service_bot_identity.go:223
```

* Machine ID: Validate generation counter using bot instances

This change moves the generation counter from bot user labels to a
field in `BotInstanceStatusAuthentication`. This allows for tracking
of individual generations regardless of join method, allows for
multiple `token`-type joins per bot, and opens the door for multi-use
`token`-type tokens in the future.

For now, the new behavior remains behind the `BOT_INSTANCE_EXPERIMENT`
feature flag, and generation counter handling is largely unchanged
when it is unset.

* Update legacy generation counter for downgrade compatibility

This updates the legacy user label generation counter to support some
downgrade compatibility if a user either disables the bot instance
experiment or downgrades to a Teleport version predating it.

Also includes some logger cleanup, and adds a warning when a
generation counter mismatch is detected but not enforced.

* Set initial generation value for legacy mode

`validateGenerationLabel` expects a nonzero generation counter value
for renewable certs to populate the counter, so this provides it
again.

* Add new generation counter tests

This adds various new tests to validate new per-instance generation
counter behavior.

Also includes backcompat fixes to import old generation counters, and
a small fix to ensure downgrades don't leave bots in a bad state.

* Fix imports

* Tweak TODO message

* Fix HostCAs not being returned during bot renewal (#44832)

* Machine ID: Remove `BOT_INSTANCE_EXPERIMENT` feature flag (#44904)

This removes the `BOT_INSTANCE_EXPERIMENT` feature flag to enable the
feature universally. It also adjusts some comments to add deprecation
notices.

* Machine ID: Only set legacy generation counter for renewable joins (#44993)

We unconditionally committed the generation counter to the legacy
user label to allow for downgrade compatibility. This is not valid
for non-token join methods as previous Teleport versions will attempt
to check the generation counter whenever the label is nonzero, and
because these older versions don't support authenticated rejoining,
will never be able to compare to the client's actual generation
value. (Plus, it was hard-coded to zero.)

This disables the legacy generation counter for non-renewable join
methods. This should not create a later upgrade problem as bots will
lose their instance ID upon their first rejoin after downgrading, and
so will be issued a fresh ID and generation counter if and when the
cluster is upgraded again.

* Remove unused function sshPublicKeyToPKIXPEM

Left-over function from backport

* Always use PEM PKIX DER representation of public keys for BotInstances (#43845)

* Always use PEM PKIX DER representation of public keys for BotInstances

* Prefer using `keys.MarshalPublicKey`

* Remove mistakenly included TestVerifyALPNUpgradedConn

The recent merge included an unrelated unit test. This removes it.

---------

Co-authored-by: Noah Stride <noah.stride@goteleport.com>
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants