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

[v16] Backport Secrets scanner #44799

Merged
merged 26 commits into from
Aug 13, 2024
Merged

[v16] Backport Secrets scanner #44799

merged 26 commits into from
Aug 13, 2024

Conversation

tigrato
Copy link
Contributor

@tigrato tigrato commented Jul 30, 2024

Backport #43804 to branch/v16
Backport #43890 to branch/v16
Backport #44036 to branch/v16
Backport #43462 to branch/v16
Backport #43467 to branch/v16
Backport #43468 to branch/v16
Backport #43486 to branch/v16
Backport #43543 to branch/v16
Backport #43905 to branch/v16
Backport #43906 to branch/v16
Backport #44010 to branch/v16
Backport #44011 to branch/v16
Backport #44014 to branch/v16
Backport #44016 to branch/v16
Backport #44021 to branch/v16
Backport #44032 to branch/v16
Backport #44055 to branch/v16
Backport #44081 to branch/v16
Backport #44159 to branch/v16
Backport #44324 to branch/v16
Backport #44220 to branch/v16
Backport #44523 to branch/v16
Backport #44643 to branch/v16
Backport #45109 to branch/v16

codingllama and others added 23 commits July 30, 2024 11:21
* Add the device assertion protos

* Update generated protos
* Add a client-side API to assert devices

* Add a godoc to authnStreamAdapter
* Define server-side device assertion interfaces

* Update proto comments

* Update generated protos
…43462)

This PR introduces the `teleport.access_graph.v1.SecretsScannerService`that will be used by Teleport SSH nodes to report `authorized_keys` and user's laptops to report secrets found on them.

The `ReportAuthorizedKeys` uses node's TLS certs signed by HostCA for authentication while `ReportSecrets` leverages the device trust credentials (requires that the device is enrolled) to report secrets without requiring valid user credentials.

handle Alan's feedback
…se (#43467)

This PR exposes the configuration for nodes to be aware that they should report SSH Authorized keys to Teleport.

Part of gravitational/access-graph#637

Signed-off-by: Tiago Silva <[email protected]>
…s Graph resources (#43468)

This PR extends the Access Graph resources to be able include the newly added `teleport.access_graph.v1.PrivateKey`,
`teleport.access_graph.v1.AuthorizedKey` and existing device trust information `teleport.devicetrust.v1.Device`.

Part of gravitational/access-graph#637

Signed-off-by: Tiago Silva <[email protected]>
This PR makes `generic.Service` correctly implementing `List*` functions when multiple key prefixes are defined

Signed-off-by: Tiago Silva <[email protected]>
* [sec_scan][5] add secrets backend service

This PR implements the backend service to support storing `authorized_keys` and `private_keys` into Teleport backend.

Part of gravitational/access-graph#637

Signed-off-by: Tiago Silva <[email protected]>

* handle feedback

* handle nits

---------

Signed-off-by: Tiago Silva <[email protected]>
This PR adds the ability to watch for events for `*devicepb.Device` objects.

Backend storage representation of  `devicepb.Device` is achieved using an internal representation that lives in `e/lib/devicetrust/storage` and whose logic is internal to the package.

To be able to expose the unmarshal logic necessary for events to work, this PR exposes a registration hook that `e/lib/devicetrust/storage` function must call during initialization to register the unmarshal function.

Part of gravitational/access-graph#637

Signed-off-by: Tiago Silva <[email protected]>
This PR introduces the ability to watch for events related to `accessgraphsecretsv1pb.AuthorizedKey` and
`accessgraphsecretsv1pb.PrivateKey` objects.

This PR is part of gravitational/access-graph#637.

Signed-off-by: Tiago Silva <[email protected]>
This PR adds the `clusterconfigpbv1.AccessGraphSettings` resource that will be used to control the secrets scanning definition of Teleport.

This resource will be a singleton and the only goal is to carry some settings related to access graph because on the cloud, users don't have access to fileconf.

This PR is part of gravitational/access-graph#637.

Signed-off-by: Tiago Silva <[email protected]>
This PR adds the boilerplate code and proto definition for `AccessGraphSettingsUpdate` audit event.

This PR is part of gravitational/access-graph#637.

Signed-off-by: Tiago Silva <[email protected]>
This PR adds the backend service to be able to create, update and retrieve access graph configurations from Teleport backend.

This PR is part of gravitational/access-graph#637.
…#44016)

* [sec_scan][12] add cache and events support for `AccessGraphSettings`

This PR adds the cache and events support for the new resource `AccessGraphSettings`.

This PR is part of gravitational/access-graph#637.

Signed-off-by: Tiago Silva <[email protected]>

* add tests

---------

Signed-off-by: Tiago Silva <[email protected]>
This PR introduces the gRPC implementation for the CRUD operations related to `AccessGraphSettings`.

This PR is part of gravitational/access-graph#637.

Signed-off-by: Tiago Silva <[email protected]>
* [sec_scan][14] create `AccessGraphSettings` on first auth init

This PR adds a init script that sets `AccessGraphSettings` into Teleport backend when auth first inits and there is no `AccessGraphSettings`.

This PR is part of gravitational/access-graph#637.

Signed-off-by: Tiago Silva <[email protected]>

* remove iterations

---------

Signed-off-by: Tiago Silva <[email protected]>
…tl` (#44055)

This PR allows any cluster admin to edit `access_graph_settings` objects via `tctl`.

This PR is part of gravitational/access-graph#637.

Signed-off-by: Tiago Silva <[email protected]>
…ns (#44081)

This PR adds methods to store/retrieve functions defined by different teleport services.

This PR is part of gravitational/access-graph#637.

Signed-off-by: Tiago Silva <[email protected]>
* [sec_scan][17] add `AssertDevice` to `FakeDeviceService`

This PR introduces a `AssertDevice` logic into `FakeDeviceService` to authenticate devices during unit tests using device trust credentials.

This PR is part of gravitational/access-graph#637.

Signed-off-by: Tiago Silva <[email protected]>

* simplify assert tests

* Update lib/devicetrust/assert/assert_test.go

Co-authored-by: Alan Parra <[email protected]>

---------

Signed-off-by: Tiago Silva <[email protected]>
Co-authored-by: Alan Parra <[email protected]>
… server (#44324)

* [sec_scan][20] add `ReportSecrets` forwarder to proxy's gRPC insecure server

This PR implements a `ReportSecrets` forwarder from Proxy server to Auth server.
The goal is to allow clients to hit the proxy insecure gRPC server (credentialless)
and proxy will forward requests to the AuthServer on behalf of the client. This is required
because the client doesn't have valid credentials and it wasn't possible for it to reach auth server
via reversetunnel when the cluster uses `separate` mode.

This PR is part of gravitational/access-graph#637.

Signed-off-by: Tiago Silva <[email protected]>

* add comments

* move dial to lib/client/proxy/insecure

---------

Signed-off-by: Tiago Silva <[email protected]>
* [sec_scan][19] add `tsh scan keys` implementation

This PR introduces the required code to transverse a directory(es), finding all the SSH private keys and report them back to the cluster using the device security enclave as authentication mechanism.

This PR is part of gravitational/access-graph#637.

Signed-off-by: Tiago Silva <[email protected]>

* handle code review

* fix message

* handle code review

* fork ssh private keys

* add skip dirs support

* handle code review

---------

Signed-off-by: Tiago Silva <[email protected]>
* [sec_scan][22] add authorized keys reporter

This PR introduces a SSH authorized keys reporter that monitors `/etc/passwd` file and all users' authorized_keys files and reports the findings back to teleport.

Part of gravitational/access-graph#637

Signed-off-by: Tiago Silva <[email protected]>

* handle comments

* handle comments

---------

Signed-off-by: Tiago Silva <[email protected]>
This PR adds ability to extract the comment and key type from AuthorizedKeys files.

Signed-off-by: Tiago Silva <[email protected]>
@tigrato tigrato added the no-changelog Indicates that a PR does not require a changelog entry label Jul 30, 2024
Copy link

@orca-security-us orca-security-us bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Failed Failed Secrets high 19   medium 0   low 0   info 0 View in Orca
🔑 The following Secrets have been detected in your pull request across all commits

⚠️ Please take action to mitigate the risk of the identified secrets by revoking them, and if already in use, updating all dependent systems

NAME FILE LINE NUM COMMIT
high Private Key ...testdata/ssh_keys.go 243 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 168 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 115 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 150 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 42 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 35 1ea66fe View in code
high Private Key ...data/invalid_keys.go 44 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 162 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 50 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 29 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 23 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 280 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 265 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 207 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 131 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 107 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 140 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 78 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 10 1ea66fe View in code

Copy link

@orca-security-us orca-security-us bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Failed Failed Secrets high 19   medium 0   low 0   info 0 View in Orca
🔑 The following Secrets have been detected in your pull request across all commits

⚠️ Please take action to mitigate the risk of the identified secrets by revoking them, and if already in use, updating all dependent systems

NAME FILE LINE NUM COMMIT
high Private Key ...testdata/ssh_keys.go 29 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 23 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 265 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 140 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 50 1ea66fe View in code
high Private Key ...data/invalid_keys.go 44 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 115 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 78 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 42 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 35 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 10 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 107 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 280 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 243 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 207 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 168 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 162 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 150 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 131 1ea66fe View in code

Copy link

@orca-security-us orca-security-us bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Failed Failed Secrets high 19   medium 0   low 0   info 0 View in Orca
🔑 The following Secrets have been detected in your pull request across all commits

⚠️ Please take action to mitigate the risk of the identified secrets by revoking them, and if already in use, updating all dependent systems

NAME FILE LINE NUM COMMIT
high Private Key ...testdata/ssh_keys.go 23 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 10 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 115 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 78 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 29 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 150 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 107 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 42 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 35 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 280 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 207 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 131 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 50 1ea66fe View in code
high Private Key ...data/invalid_keys.go 44 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 265 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 243 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 168 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 162 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 140 1ea66fe View in code

@tigrato tigrato force-pushed the tigrato/sec-scan-v16 branch from fd33101 to 1a246f5 Compare July 30, 2024 11:43
Copy link

@orca-security-us orca-security-us bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Failed Failed Secrets high 19   medium 0   low 0   info 0 View in Orca
🔑 The following Secrets have been detected in your pull request across all commits

⚠️ Please take action to mitigate the risk of the identified secrets by revoking them, and if already in use, updating all dependent systems

NAME FILE LINE NUM COMMIT
high Private Key ...testdata/ssh_keys.go 280 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 207 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 168 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 115 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 23 1ea66fe View in code
high Private Key ...data/invalid_keys.go 44 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 243 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 150 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 107 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 78 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 265 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 162 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 42 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 29 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 140 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 131 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 50 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 35 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 10 1ea66fe View in code

@tigrato tigrato force-pushed the tigrato/sec-scan-v16 branch from 1a246f5 to 25a6b98 Compare July 30, 2024 11:54
Copy link

@orca-security-us orca-security-us bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Failed Failed Secrets high 19   medium 0   low 0   info 0 View in Orca
🔑 The following Secrets have been detected in your pull request across all commits

⚠️ Please take action to mitigate the risk of the identified secrets by revoking them, and if already in use, updating all dependent systems

NAME FILE LINE NUM COMMIT
high Private Key ...testdata/ssh_keys.go 243 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 168 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 150 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 78 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 265 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 162 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 140 1ea66fe View in code
high Private Key ...data/invalid_keys.go 44 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 115 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 107 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 50 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 42 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 35 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 23 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 10 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 280 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 207 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 131 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 29 1ea66fe View in code

@tigrato tigrato force-pushed the tigrato/sec-scan-v16 branch from 25a6b98 to 16e8f68 Compare July 30, 2024 12:01
Copy link

@orca-security-us orca-security-us bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Failed Failed Secrets high 19   medium 0   low 0   info 0 View in Orca
🔑 The following Secrets have been detected in your pull request across all commits

⚠️ Please take action to mitigate the risk of the identified secrets by revoking them, and if already in use, updating all dependent systems

NAME FILE LINE NUM COMMIT
high Private Key ...testdata/ssh_keys.go 207 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 140 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 78 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 280 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 243 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 162 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 23 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 42 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 35 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 29 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 10 1ea66fe View in code
high Private Key ...data/invalid_keys.go 44 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 265 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 150 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 168 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 131 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 115 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 107 1ea66fe View in code
high Private Key ...testdata/ssh_keys.go 50 1ea66fe View in code

@tigrato tigrato marked this pull request as ready for review July 30, 2024 15:03
@github-actions github-actions bot added audit-log Issues related to Teleports Audit Log backport size/xl tctl tctl - Teleport admin tool tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Jul 30, 2024
@github-actions github-actions bot requested review from justinas and ryanclark July 30, 2024 15:03
@public-teleport-github-review-bot

@tigrato - this PR will require admin approval to merge due to its size. Consider breaking it up into a series smaller changes.

Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

Bot.

tigrato added 2 commits August 8, 2024 12:02
* [sec_scan][27] add support for LDAP users and macOS

This PR extends support for authorized keys report for users managed by LDAP system and macOS targets.

It leverages `getpwent` to read the system database files and retrieve the user properties.
It doesn't use the `getpwent_r` because it's not available in macOS and because it's not (yet) standerdized

>   PLEASE NOTE: the `getpwent_r' function is not (yet) standardized.
>   The interface may change in later versions of this library.  But
>   the interface is designed following the principals used for the
>   other reentrant functions so the chances are good this is what the
>   POSIX people would choose.

Part of gravitational/access-graph#637

* handle comments

* handle comments 2

* add comment
@tigrato tigrato removed the no-changelog Indicates that a PR does not require a changelog entry label Aug 13, 2024
Copy link

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.

@tigrato tigrato added no-changelog Indicates that a PR does not require a changelog entry and removed do-not-merge labels Aug 13, 2024
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from justinas August 13, 2024 15:38
@tigrato tigrato enabled auto-merge August 13, 2024 15:49
@tigrato tigrato added this pull request to the merge queue Aug 13, 2024
Merged via the queue into branch/v16 with commit e0eb172 Aug 13, 2024
42 of 43 checks passed
@tigrato tigrato deleted the tigrato/sec-scan-v16 branch August 13, 2024 16:15
tigrato added a commit that referenced this pull request Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit-log Issues related to Teleports Audit Log backport no-changelog Indicates that a PR does not require a changelog entry size/xl tctl tctl - Teleport admin tool tsh tsh - Teleport's command line tool for logging into nodes running Teleport.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants