Add bot field to certificates and various usage events #35881

timothyb89 · 2023-12-19T02:17:03Z

This adds a new certificate extension field, teleport-bot, to certificates issued to Machine ID bot users that can definitively identify certificates as having been issued to a bot user, set to the bot's unprefixed name.

Additionally, this uses the new bot name field to mark certain usage events as originating from bot users. As such, it includes a protobuf update from Cloud [1], which also pulled in some small additional (mostly comment) changes.

[1] https://github.com/gravitational/cloud/pull/7060

changelog: Added new certificate extensions and usage reporting flags to explicitly identify Machine ID bots and their cluster activity

This adds a new certificate extension field, `teleport-bot`, to certificates issued to Machine ID bot users that can definitively identify certificates as having been issued to a bot user. Additionally, this uses the new `Bot` identity flag to mark certain usage events as originating from bot users. As such, it includes a protobuf update from Cloud [1], which pulled in some small additional (mostly comment) changes. [1] gravitational/cloud#7060

…-usage-events

timothyb89 · 2023-12-19T02:19:18Z

Outstanding TODOs:

Manually test the event pipeline (needs to emit events with the flag set as expected)
Unit tests for the bot cert flag

…-usage-events

api/proto/teleport/legacy/types/events/events.proto

lib/auth/keygen/keygen.go

This makes a few changes to the bot tagging approach: * The bot name is embedded in the cert rather than just true/false * UserKind is included in events rather than just a bot flag, to allow for an unspecified value for older client nodes.

github-actions · 2023-12-28T02:39:36Z

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

strideynet

Looks generally good. From my reading, we're only currently putting the attribute into the impersonated certs or renewed certs. Could we also make sure it goes onto the bots own certs that are produced by the joining process ?

strideynet · 2023-12-28T11:31:56Z

lib/usagereporter/teleport/aggregating/reporter.go

@@ -211,10 +224,14 @@ func (r *Reporter) run(ctx context.Context) {

 	userActivity := make(map[string]*prehogv1.UserActivityRecord)

-	userRecord := func(userName string) *prehogv1.UserActivityRecord {
+	userRecord := func(userName string, v1AlphaUserKind prehogv1alpha.UserKind) *prehogv1.UserActivityRecord {


Nit: I don't think it's a major concern but I can see a bit of potential raciness here if there's an issue with tracking bot status. The status of the user from the first event will set it for all future events. Perhaps we ought to put some warning or error in if the kind of the user changes ?

I added a warning and had it override the flag. I think the behavior could go either way realistically - and given it's a bug regardless I'm not sure if I expect anything good to come from whichever behavior we use.

Maybe we should leave the record.UserKind as is if v1UserKind is unspecified, just in case?

Ah, good call, we could definitely see this when aggregating events together from an outdated node. I suppose this goes both ways, right? We should allow transition from unspecified -> specified, and disallow transitions from specified -> unspecified, otherwise at least one will be logged as a warning.

I've tweaked the logic here. It now allows identifying a previously unspecified user, ignores unspecified updates, and warns about flipping between human/bot users.

timothyb89 · 2023-12-29T23:46:01Z

From my reading, we're only currently putting the attribute into the impersonated certs or renewed certs. Could we also make sure it goes onto the bots own certs that are produced by the joining process ?

Good catch on the bot flag, I always forget the initial certs are generated from a different codepath. The flag is fixed now.

…-usage-events

constants.go

espadolini · 2024-01-02T13:05:56Z

lib/usagereporter/teleport/aggregating/reporter.go

@@ -211,10 +224,14 @@ func (r *Reporter) run(ctx context.Context) {

 	userActivity := make(map[string]*prehogv1.UserActivityRecord)

-	userRecord := func(userName string) *prehogv1.UserActivityRecord {
+	userRecord := func(userName string, v1AlphaUserKind prehogv1alpha.UserKind) *prehogv1.UserActivityRecord {


Maybe we should leave the record.UserKind as is if v1UserKind is unspecified, just in case?

Renames the `teleport-bot` extension to `bot-name@goteleport.com`, to better follow SSH cert extension naming conventions.

…-usage-events

public-teleport-github-review-bot · 2024-01-04T23:27:59Z

@timothyb89 See the table below for backport results.

Branch	Result
branch/v13	Failed
branch/v14	Failed

* Add bot field to certificates and various usage events This adds a new certificate extension field, `teleport-bot`, to certificates issued to Machine ID bot users that can definitively identify certificates as having been issued to a bot user. Additionally, this uses the new `Bot` identity flag to mark certain usage events as originating from bot users. As such, it includes a protobuf update from Cloud [1], which pulled in some small additional (mostly comment) changes. [1] gravitational/cloud#7060 * Small bot flag plumbing fixes * Convert bot flag to BotName and UserKind enum This makes a few changes to the bot tagging approach: * The bot name is embedded in the cert rather than just true/false * UserKind is included in events rather than just a bot flag, to allow for an unspecified value for older client nodes. * Add a quick unit test for bot cert extensions * Fix outdated grpc * Include bot flag on initial certs * Log a warning and override user kind for usage records if they differ * Fix several unit tests; add a bot metadata test case * Fix unit tests with UserKind zero value * Rename SSH cert extension to use standard format Renames the `teleport-bot` extension to `bot-name@goteleport.com`, to better follow SSH cert extension naming conventions. * Attempt to improve unspecified userkind aggregating logic

…#36366) * Add bot field to certificates and various usage events (#35881) * Add bot field to certificates and various usage events This adds a new certificate extension field, `teleport-bot`, to certificates issued to Machine ID bot users that can definitively identify certificates as having been issued to a bot user. Additionally, this uses the new `Bot` identity flag to mark certain usage events as originating from bot users. As such, it includes a protobuf update from Cloud [1], which pulled in some small additional (mostly comment) changes. [1] gravitational/cloud#7060 * Small bot flag plumbing fixes * Convert bot flag to BotName and UserKind enum This makes a few changes to the bot tagging approach: * The bot name is embedded in the cert rather than just true/false * UserKind is included in events rather than just a bot flag, to allow for an unspecified value for older client nodes. * Add a quick unit test for bot cert extensions * Fix outdated grpc * Include bot flag on initial certs * Log a warning and override user kind for usage records if they differ * Fix several unit tests; add a bot metadata test case * Fix unit tests with UserKind zero value * Rename SSH cert extension to use standard format Renames the `teleport-bot` extension to `bot-name@goteleport.com`, to better follow SSH cert extension naming conventions. * Attempt to improve unspecified userkind aggregating logic * Fix failing tests

…#36313) * Add bot field to certificates and various usage events (#35881) * Add bot field to certificates and various usage events This adds a new certificate extension field, `teleport-bot`, to certificates issued to Machine ID bot users that can definitively identify certificates as having been issued to a bot user. Additionally, this uses the new `Bot` identity flag to mark certain usage events as originating from bot users. As such, it includes a protobuf update from Cloud [1], which pulled in some small additional (mostly comment) changes. [1] gravitational/cloud#7060 * Small bot flag plumbing fixes * Convert bot flag to BotName and UserKind enum This makes a few changes to the bot tagging approach: * The bot name is embedded in the cert rather than just true/false * UserKind is included in events rather than just a bot flag, to allow for an unspecified value for older client nodes. * Add a quick unit test for bot cert extensions * Fix outdated grpc * Include bot flag on initial certs * Log a warning and override user kind for usage records if they differ * Fix several unit tests; add a bot metadata test case * Fix unit tests with UserKind zero value * Rename SSH cert extension to use standard format Renames the `teleport-bot` extension to `bot-name@goteleport.com`, to better follow SSH cert extension naming conventions. * Attempt to improve unspecified userkind aggregating logic * Fix broken tests

* Add bot field to certificates and various usage events This adds a new certificate extension field, `teleport-bot`, to certificates issued to Machine ID bot users that can definitively identify certificates as having been issued to a bot user. Additionally, this uses the new `Bot` identity flag to mark certain usage events as originating from bot users. As such, it includes a protobuf update from Cloud [1], which pulled in some small additional (mostly comment) changes. [1] gravitational/cloud#7060 * Small bot flag plumbing fixes * Convert bot flag to BotName and UserKind enum This makes a few changes to the bot tagging approach: * The bot name is embedded in the cert rather than just true/false * UserKind is included in events rather than just a bot flag, to allow for an unspecified value for older client nodes. * Add a quick unit test for bot cert extensions * Fix outdated grpc * Include bot flag on initial certs * Log a warning and override user kind for usage records if they differ * Fix several unit tests; add a bot metadata test case * Fix unit tests with UserKind zero value * Rename SSH cert extension to use standard format Renames the `teleport-bot` extension to `bot-name@goteleport.com`, to better follow SSH cert extension naming conventions. * Attempt to improve unspecified userkind aggregating logic

timothyb89 added 2 commits December 18, 2023 19:03

Merge remote-tracking branch 'origin/master' into timothyb89/bot-flag…

4409436

…-usage-events

strideynet self-requested a review December 19, 2023 20:54

timothyb89 added 2 commits December 22, 2023 15:59

Small bot flag plumbing fixes

8889b64

Merge remote-tracking branch 'origin/master' into timothyb89/bot-flag…

ded03c0

…-usage-events

timothyb89 commented Dec 27, 2023

View reviewed changes

api/proto/teleport/legacy/types/events/events.proto Outdated Show resolved Hide resolved

strideynet reviewed Dec 27, 2023

View reviewed changes

lib/auth/keygen/keygen.go Outdated Show resolved Hide resolved

timothyb89 added 2 commits December 27, 2023 18:11

Convert bot flag to BotName and UserKind enum

7cb5a0e

This makes a few changes to the bot tagging approach: * The bot name is embedded in the cert rather than just true/false * UserKind is included in events rather than just a bot flag, to allow for an unspecified value for older client nodes.

Add a quick unit test for bot cert extensions

113a6fb

timothyb89 marked this pull request as ready for review December 28, 2023 02:39

github-actions bot added the size/md label Dec 28, 2023

github-actions bot requested review from flyinghermit and ryanclark December 28, 2023 02:39

timothyb89 added backport/branch/v13 backport/branch/v14 and removed size/md labels Dec 28, 2023

Fix outdated grpc

5738078

strideynet reviewed Dec 28, 2023

View reviewed changes

timothyb89 added 2 commits December 29, 2023 16:42

Include bot flag on initial certs

cc783fe

Log a warning and override user kind for usage records if they differ

de0cc17

timothyb89 added 4 commits December 29, 2023 16:56

Fix several unit tests; add a bot metadata test case

c32f158

Merge remote-tracking branch 'origin/master' into timothyb89/bot-flag…

3558b50

…-usage-events

Merge remote-tracking branch 'origin/master' into timothyb89/bot-flag…

4012947

…-usage-events

Fix unit tests with UserKind zero value

ee77d9c

timothyb89 requested a review from espadolini December 30, 2023 00:48

strideynet approved these changes Jan 2, 2024

View reviewed changes

espadolini reviewed Jan 2, 2024

View reviewed changes

timothyb89 added 2 commits January 2, 2024 15:04

Rename SSH cert extension to use standard format

ba60a89

Renames the `teleport-bot` extension to `bot-name@goteleport.com`, to better follow SSH cert extension naming conventions.

Attempt to improve unspecified userkind aggregating logic

73dc21e

timothyb89 mentioned this pull request Jan 2, 2024

Report resource usage counts by handling heartbeat events #35968

Merged

timothyb89 requested a review from espadolini January 3, 2024 20:34

espadolini approved these changes Jan 4, 2024

View reviewed changes

public-teleport-github-review-bot bot removed request for ryanclark and flyinghermit January 4, 2024 22:17

Merge remote-tracking branch 'origin/master' into timothyb89/bot-flag…

9d7b12d

…-usage-events

timothyb89 force-pushed the timothyb89/bot-flag-usage-events branch from 17a3729 to 9d7b12d Compare January 4, 2024 22:28

timothyb89 enabled auto-merge January 4, 2024 22:35

timothyb89 added 2 commits January 4, 2024 15:40

Merge branch 'master' into timothyb89/bot-flag-usage-events

764c8c4

Merge branch 'master' into timothyb89/bot-flag-usage-events

4e33a32

timothyb89 added this pull request to the merge queue Jan 4, 2024

Merged via the queue into master with commit f7e97d0 Jan 4, 2024
36 checks passed

timothyb89 deleted the timothyb89/bot-flag-usage-events branch January 4, 2024 23:26

timothyb89 mentioned this pull request Jan 5, 2024

[v14] Add bot field to certificates and various usage events (#35881) #36313

Merged

timothyb89 mentioned this pull request Jan 6, 2024

[v13] Add bot field to certificates and various usage events (#35881) #36366

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add bot field to certificates and various usage events #35881

Add bot field to certificates and various usage events #35881

timothyb89 commented Dec 19, 2023 •

edited

timothyb89 commented Dec 19, 2023 •

edited

github-actions bot commented Dec 28, 2023

strideynet left a comment

strideynet Dec 28, 2023

timothyb89 Dec 29, 2023

espadolini Jan 2, 2024

timothyb89 Jan 2, 2024 •

edited

timothyb89 Jan 2, 2024

timothyb89 commented Dec 29, 2023

espadolini Jan 2, 2024

public-teleport-github-review-bot bot commented Jan 4, 2024

Add bot field to certificates and various usage events #35881

Add bot field to certificates and various usage events #35881

Conversation

timothyb89 commented Dec 19, 2023 • edited

timothyb89 commented Dec 19, 2023 • edited

github-actions bot commented Dec 28, 2023

strideynet left a comment

Choose a reason for hiding this comment

strideynet Dec 28, 2023

Choose a reason for hiding this comment

timothyb89 Dec 29, 2023

Choose a reason for hiding this comment

espadolini Jan 2, 2024

Choose a reason for hiding this comment

timothyb89 Jan 2, 2024 • edited

Choose a reason for hiding this comment

timothyb89 Jan 2, 2024

Choose a reason for hiding this comment

timothyb89 commented Dec 29, 2023

espadolini Jan 2, 2024

Choose a reason for hiding this comment

public-teleport-github-review-bot bot commented Jan 4, 2024

timothyb89 commented Dec 19, 2023 •

edited

timothyb89 commented Dec 19, 2023 •

edited

timothyb89 Jan 2, 2024 •

edited