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

Implement validation of prefix in API keys #838

Closed
2 tasks
yaelberger-commits opened this issue Sep 1, 2022 · 18 comments
Closed
2 tasks

Implement validation of prefix in API keys #838

yaelberger-commits opened this issue Sep 1, 2022 · 18 comments
Assignees
Labels
Dev Task for implementation of a technical solution High Priority | Haute priorité Refined l Affiné Stories and tasks that are ready to be worked on Security | Sécurité

Comments

@yaelberger-commits
Copy link
Collaborator

yaelberger-commits commented Sep 1, 2022

Implement validation of prefix in API keys

Description

As a (user), I need to be able to rely on proactive security measure in place in notify so that my data and services can be protected.

WHY are we building?

Enabling this validation will ensure our users are using the latest API key format, which protects them and us from keys being mistakenly committed to public repos.

WHAT are we building?

A validation that ensures any API key used contains our api key prefix.

VALUE created by our solution

Better security: API keys mistakenly stored in github can be automatically identified and we can be notified.

Documentation

https://docs.google.com/document/d/1C8VOHf87Y1m90RYOnOrPwE5RB2atvzTlLM8HDr5IHEc/edit

Acceptance Criteria** (Definition of done)

  • Old api keys (without the prefix) are denied by the API
  • New API keys (with the prefix) are successful when using the API

To be refined through discussion with the team

@yaelberger-commits
Copy link
Collaborator Author

Hey team! Please add your planning poker estimate with Zenhub @andrewleith @jimleroyer @jzbahrai @sastels

@yaelberger-commits yaelberger-commits added the Refined l Affiné Stories and tasks that are ready to be worked on label Sep 7, 2022
@yaelberger-commits
Copy link
Collaborator Author

Blocked because we can not yet enroll in Github secrets

@yaelberger-commits
Copy link
Collaborator Author

Hey @patheard Jimmy and I were wondering what is blocking us from progressing with API prefixes. Are we able to unblock in the New Year?

@yaelberger-commits yaelberger-commits self-assigned this Dec 22, 2022
@patheard
Copy link
Member

Right now the blocker for defining our own secret scanning patterns is that we don't have GitHub Advanced Security, which would give us access to this feature:
https://docs.github.com/en/enterprise-cloud@latest/code-security/secret-scanning/defining-custom-patterns-for-secret-scanning

We do have access to GitHub secret scanning though on all public repositories, which I'd say is worth turning on since it comes with a bunch of predefined secret scanning patterns:
https://docs.github.com/en/code-security/secret-scanning/configuring-secret-scanning-for-your-repositories

All that being said, it would be worth testing out how GitHub secret scanning works with a dummy API key. Scanners are usually pretty quick to pick up high entropy random strings of characters and flagging them as potential secrets.

@jimleroyer
Copy link
Member

There was some breakthrough by SRE team and Pat in the past days. CDS applied as a GitHub partner in order to leverage plaintext key misusage of all orgs/repositories in their product (this is not offered to non-partners).

A legal document was signed by TBS to accept this partnership and it went surprisingly quick (quicker than the actual implementation).

So now we have access to the GitHub tools that will monitor any files submitted in that service to check for detecting the pattern of our keys. GitHub would callback an endpoint hosted and provided by us when it detected a regex fitting our keys. So far this regex has been defined as such:

^gcntfy-(\w+_)*\w+-([a-fA-F\d]{8}-[a-fA-F\d]{4}-[a-fA-F\d]{4}-[a-fA-F\d]{4}-[a-fA-F\d]{12})-([a-fA-F\d]{8}-[a-fA-F\d]{4}-[a-fA-F\d]{4}-[a-fA-F\d]{4}-[a-fA-F\d]{12})$

i.e. the GCNotify prefix + "-" + formatted service name + "-" + service UUID4 + key UUID4.

@yaelberger-commits
Copy link
Collaborator Author

Great news!

@patheard
Copy link
Member

@jimleroyer I've tweaked the regex slightly to handle cases where the API key name has a dash:

^gcntfy-[\w-]*\w-[a-fA-F\d]{8}-[a-fA-F\d]{4}-[a-fA-F\d]{4}-[a-fA-F\d]{4}-[a-fA-F\d]{12}-[a-fA-F\d]{8}-[a-fA-F\d]{4}-[a-fA-F\d]{4}-[a-fA-F\d]{4}-[a-fA-F\d]{12}$

@patheard
Copy link
Member

I've submitted the request to GitHub with the above regex and our secret scanning endpoint:
https://github-secret-scanning.alpha.canada.ca/alert

I also provided them with sample API keys (revoked) from Pat's test service that they can use to test things are working as expected.

@yaelberger-commits
Copy link
Collaborator Author

Brought into in progress since Pat is doing some tasks on this one and it's been unblocked

@smcmurtry
Copy link
Contributor

No updates today as Pat is off, will check back tomorrow.

@patheard
Copy link
Member

Our partnership with GitHub's secret scanning service is now live and we should be notified if any Notify API token's matching the regex we provided to GitHub are detected in a public GitHub repo.

@andrewleith
Copy link
Member

Sorry, very late to the party here, but this is amazing work @patheard, very well done! 🏆

@yaelberger-commits
Copy link
Collaborator Author

Conversation booked for July 20th to discuss the problem space and how we might roll out the prefix to our clients without creating new pain points for developers

@yaelberger-commits
Copy link
Collaborator Author

To be implemented in 6 months, or sooner once all Old API keys have been revoked and all clients are changed over to new API keys

@yaelberger-commits
Copy link
Collaborator Author

To revisit after November 18th, 2023

@yaelberger-commits
Copy link
Collaborator Author

@jzbahrai is this work different than what's being worked on this sprint? We should review these cards and acceptance criteria

@jzbahrai
Copy link
Collaborator

jzbahrai commented Nov 6, 2023

This ticket is still valid. We can shut this ticket after we have revoked all the keys on Nov 18. Given that we will be revoking all keys pre Sept 2022, we do not need to enforce the prefix validation in code (all the existing keys will be revoked on the backend).

@yaelberger-commits
Copy link
Collaborator Author

Decided just to revoke all old keys after the 18th, so this step isn't necessary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dev Task for implementation of a technical solution High Priority | Haute priorité Refined l Affiné Stories and tasks that are ready to be worked on Security | Sécurité
Projects
None yet
Development

No branches or pull requests

7 participants