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

Add Tailscale detector #1719

Merged
merged 3 commits into from
Sep 8, 2023
Merged

Add Tailscale detector #1719

merged 3 commits into from
Sep 8, 2023

Conversation

marwan-at-work
Copy link
Contributor

@marwan-at-work marwan-at-work commented Aug 29, 2023

Description:

This PR adds a new Tailscale detector for API and Oauth tokens

Fixes #1712

Checklist:

  • Tests passing (make test-community)?
  • Lint passing (make lint this requires golangci-lint)?

@marwan-at-work marwan-at-work requested a review from a team as a code owner August 29, 2023 14:23
@CLAassistant
Copy link

CLAassistant commented Aug 29, 2023

CLA assistant check
All committers have signed the CLA.

@zricethezav
Copy link
Collaborator

@marwan-at-work Thanks for submitting this PR! I love Tailscale :)

One of the engineers will review this PR. In the meantime, would you mind resolving the merge conflicts?

client = common.SaneHttpClient()

// Make sure that your group is surrounded in boundary characters such as below to reduce false positives.
keyPat = regexp.MustCompile(`^tskey-(api|oauth)-[0-9A-Za-z_]+-[0-9A-Za-z_]+$`)

Choose a reason for hiding this comment

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

we probably don't want to just limit to these prefixes. There are quite a few: https://tailscale.com/kb/1277/key-prefixes/. But we also may add more later, so maybe just a regex for the key type as well?

Copy link

@willnorris willnorris Aug 29, 2023

Choose a reason for hiding this comment

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

oh! I guess these are the only two that will work with the oauth/verify endpoint. In that case, we may actually want to rethink how we implement the key checking endpoint on the Tailscale side 😕

Copy link
Contributor Author

@marwan-at-work marwan-at-work Sep 7, 2023

Choose a reason for hiding this comment

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

We ended up with a new endpoint that should verify everything? Updated the regex to reflect it

@marwan-at-work marwan-at-work marked this pull request as ready for review September 7, 2023 21:44
@marwan-at-work
Copy link
Contributor Author

@zricethezav thanks for the kind words. Resolved the conflicts and the PR is ready for review :)

Copy link
Collaborator

@ahrav ahrav left a comment

Choose a reason for hiding this comment

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

Thanks a lot @marwan-at-work, I know you spent a decent amount of time getting this ironed out and we really appreciate the contribution. Similar to Zach, I am also a huge Tailscale fan!

pkg/detectors/tailscale/tailscale.go Outdated Show resolved Hide resolved
@ahrav ahrav merged commit 3aa5369 into trufflesecurity:main Sep 8, 2023
@bugbaba
Copy link

bugbaba commented Sep 9, 2023

Hi @marwan-at-work,

Just wanted to double-check about the /secret-scanning/verify endpoint. I noticed that it isn't documented and it appears to be a unique endpoint created specifically for trufflehog, which seems a bit unusual.

It would be better to have a more general endpoint that could be utilized to verify the validity of the token.

@marwan-at-work
Copy link
Contributor Author

@bugbaba this is going to be a general endpoint for all secret scanning products that don't require a specific implementation besides validating a token (as opposed to GitHub for example, which requires specific public key verification, request/response body shapes, etc).

A more generic endpoint that I saw in other implementations is to hit a /profiles endpoint but being overly cautious from a security prospective, we'd like to not return any profile or user identifiers besides just a boolean about the requested token. Of course, if the token is valid, actors can hit other endpoints that retrieves more information but some implementation might permit us to revoke the token before that can happen and we currently don't have a /profile endpoint anyway without knowing more information like the tailnet that this token belongs to.

As for documentation, this endpoint is end customers would care about, but it's something we'll discuss for sure.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add Tailscale Detector
7 participants