Open afeld opened 5 years ago
@afeld since that was mostly my doing, here’s the deets. At the time, there was not an easy way to install git-secrets without a bunch of dependency hell (wasn’t installable from brew for example). Also, the output is less than ideal for non-developers, giving you no real clue what you were violating unless you could read/understand the regex, and this was something we were instituting across the org for all staff. We also thought we were going to get more support from the original developer, since was fairly responsive to PRs at first, and then he just stopped. We still have PRs out there that went unmerged.
As for output, compare git-secrets
:
/tmp/example:3:password=******
[ERROR] Matched prohibited pattern
Possible mitigations:
- Mark false positives as allowed using: git config --add secrets.allowed ...
- List your configured patterns: git config --get-all secrets.patterns
- List your configured allowed patterns: git config --get-all secrets.allowed
- Use --no-verify if this is a one-time false positive
VS. what is output from git-seekret
Found Secrets: 1
/tmp/example:3
- Metadata:
status: test
- Rule:
password.password
- Content:
password = 'this is super secret'
Why is that subtle difference important you may be asking?
I can give each rule a name with git-seekret
, which can add additional context on the issue, and I can group the exceptions with the rule definition. With git-secrets
the rules are just a line separated file with a bunch of regexes.
Let's take searching for New Relic license keys.
git-seekret
I can define the rule in it's own file, and when it's matched, I see the Rule
comes up as newrelic.license_key
when it's found:
license_key:
match: (\"|')?(NEW|New|new)?_?(RELIC|Relic|relic)_?(LICENSE|License|license)?_?(KEY|Key|key)?\s*(:|=>|=)?\s*(\"|')?[0-9A-Fa-f]{40}(\"|')?
unmatch:
- (\"|')?github.com.*/#[A-Za-z0-9]{40}(\"|')?
- (\"|')?sha(\"|')?\s*(:|=>|=)\s*(\"|')?[A-Za-z0-9]{40}(\"|')?
- (\"|')?revision(\"|')?\s*(:|=>|=)\s*(\"|')?[A-Za-z0-9]{40}(\"|')?
- (\"|')?version(\"|')?\s*(:|=>|=)\s*(\"|')?[A-Za-z0-9]{40}(\"|')?
- (\"|')?hash(\"|')?\s*(:|=>|=)\s*(\"|')?[A-Za-z0-9]{40}(\"|')?
Could we re-prioritize this? cloud.gov has had two incidents recently with credentials being committed and we'd love to have a working tool to minimize the chance of more of these. Thanks!
Is git seekrets
actually broken for folks, or just not tested/trusted? I think the UI argument raised on the issue matters not a whit when it’s just not working.
@hillaryj I'm curious if updating the git-seekrets rules could prevent a repeat of those.
I suspect the rules are going to be an issue regardless of the tool, though maybe the other tools have built-in rules that already cover our needs. I'd still be interested in seeing how they perform against issues like the ones Hillary mentioned.
I second @pburkholder's question. If the tool actually works and it's an issue of outdated/untested rules, then changing tools only helps if the new tool also has the rules we need and those are maintained and tested. If we still have to write and maintain our own rules, then a different tool isn't going to be any better.
Sorry, realized I forgot to include a link to our current rules: https://github.com/18F/laptop/tree/master/seekret-rules
@afeld -- I know we looked through this the other day when we were doing grooming together, but I'm looking back through this and I'm wondering if we could be more explicit on what a 'path forward' would look like? Is that a document, mvp code, consensus from folks asking for it...?
My earlier comment was based on the assumption that git-seekrets
isn't
working. But so far, my testing with git-seekrets shows it works as
advertised.
I'd like to know more about what issues folks are having with git-seekrets itself (regardless of the rules), if any.
Given the stability of Git, I don't think git-seekrets itself is going to need much by way of maintenance. And the UI is indeed better than awslab's git-secrets.
The patterns need updates, and there's been an idea floated by @hillaryj
that maybe we should have a laptop-git
repo that takes care of
installing, updating git-related stuff that's not tied to the cruft in
laptop
.
P.
On Tue, Feb 11, 2020 at 12:57 AM Alyssa Feola notifications@github.com wrote:
@afeld https://github.com/afeld -- I know we looked through this the other day when we were doing grooming together, but I'm looking back through this and I'm wondering if we could be more explicit on what a 'path forward' would look like? Is that a document, mvp code, consensus from folks asking for it...?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/18F/laptop/issues/185?email_source=notifications&email_token=AAJHWCXI2DQ46WUSUVIIBXLRCI43ZA5CNFSM4IIJIBH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELLJKNA#issuecomment-584488244, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJHWCXUKSQBQ2MGOB4Y35DRCI43ZANCNFSM4IIJIBHQ .
Peter Burkholder cloud.gov compliance & security | co-lead DevOps Community of Practice https://digital.gov/communities/devops/
Solutions https://www.gsa.gov/about-us/organization/federal-acquisition-service/technology-transformation-services/tts-solutions | Technology Transformation Services https://join.tts.gsa.gov/tts-offices/
|
GSA http://www.gsa.gov/portal/category/100000
202-709-2028 <(202)%20209-2028> | peter.burkholder@gsa.gov peter.burkholder@gsa.gov
| pronouns he-him https://www.mypronouns.org/he-him Free/Busy Calendar https://calendar.google.com/calendar/embed?src=peter.burkholder@gsa.gov
we could be more explicit on what a 'path forward' would look like
That help?
Yes, and is the acceptance criteria is that a new issue is developed based on the outcome of what combination of doing it will be?
Poirot is another tool for doing this. I happened across it when I noticed it was used in SBA's CKAN repository (example patterns).
That said, the GitHub repo is archived, and I'm not sure why... It's still listed in PyPI. From this release history it appears that it reached a 1.0.x status in 2016, and then stopped. I can imagine that a tool like this can be considered "done", so maybe the repo being archived is more about that than 🕸 ?
@its-a-lisa Can be part of this issue or split into separate ones - whatever.
From @JJediny in https://github.com/18F/laptop/issues/189#issuecomment-589091756:
These might be good replacements for git-seekrets, moving the configuration burden to the repos being worked on using a yaml configuration file in version control would allow custom pre-commit checks specific to whatever project is being worked on with: https://pre-commit.com/#intro
a secret pre-commit check could use:
https://github.com/Yelp/detect-secrets https://github.com/zricethezav/gitleaks
@afeld @its-a-lisa
Based on a thorough review of solutions, this is the proposed plan for managing repos with secrets compatible with transparent git workflows:
To protect against committing credentials or other secrets the TTS organization uses:
To set this up locally Mac users can use homebrew (for any other OS, please consult in the installation instructs for each using the links provided ^)
$ brew install sops gitleaks pre-commit
.github/hooks/precommit
:
#!/bin/sh
# GITLEAKS_PRECOMMIT_HOOK BEGIN
precommit_latest_url=$(curl --silent --location --head --output /dev/null --write-out '%{url_effective}' -- https://github.com/GSA/odp-code-repository-commit-rules/releases/latest)
precommit_version=${precommit_latest_url##*/}
precommit_url="https://raw.githubusercontent.com/GSA/odp-code-repository-commit-rules/${precommit_version}/gitleaks/precommit.sh"
precommit_path="$(git rev-parse --show-toplevel)/.git/hooks/precommit.sh"
curl --silent -o "${precommit_path}" "${precommiturl}" os=$(uname -s | cut -d'' -f 1) if [ "$os" = "Linux" ] || [ "$os" = "Darwin" ]; then chmod +x "${precommit_path}" fi
"${precommit_path}" "${precommit_version}"
Users should review the shared GSA rules and make a Pull Request for any additional regex rules needed, the rules can be forked if needed: https://github.com/GSA/odp-code-repository-commit-rules
1. Determine the approach to managing secret injection per repo, based on the following:
**_Is the secret ONLY or INFREQUENTLY required for initial set up or one-off tasks AND DOES NOT need to be shared with multiple team members or CI/CD?_**
- If the secret is not needed for every deployment and there is no need for team members to share access to the secret in version control. Then use Environmental variables and provide documentation on how and when to export them in developer setup/deployment/installation instructions.
**_Does the secret NEED to be shared with multiple team members or CI/CD?_**
- Storing the encrypted secret in git is often a forced requirement, but while it can be hard to setup, it can be more secure if done correctly. As it provides auditability, implements zero-trust, and can be easier to remediate incidents like a secret compromise by on-demand key rotation. When it is appropriate to manage such shared secret in git we use [SOPS](https://github.com/mozilla/sops). Please consult their documentation; but also reach out in the #infrastructure or # dev slack channels early.
{{ DRAFT 3/2/2020 }}
The proposed model is better explained in this blog post: https://oteemo.com/2019/06/20/hashicorp-vault-is-overhyped-and-mozilla-sops-with-kms-and-git-is-massively-underrated/
I like this. One thing I'd suggest though is rolling installation of gitleaks
into the laptop script (along with removing git-seekret
on machines where that's already installed) and installing it as a system-wide pre-commit hook, like we've done with git-seekret
. I worry that asking people to do it per-repo will lead to people not doing it at all.
Also agree with using the GSA-provided rules. That's a great resource and it makes sense to use it. We'd probably want to get a couple of PRs in to cover New Relic and Mandril (if we're still using both), which are in the rules we have for git-seekret
but are not in the GSA rules.
This is really great, thanks for putting this together @JJediny!
Sent to GSA secops for review/comment, awaiting feedback
Clarified the title that let's keep this issue scoped to the local secret committing prevention; monitoring and revoking would be a whole other thing that we should split into its own issue.
I've started working with pre-commit
and gitleaks
in the context of getting a better leak prevention in place for the cloud.gov team. pre-commit
seems pretty powerful, but it's explicitly designed not to work with a core.hookspath
setting: https://github.com/pre-commit/pre-commit/issues/1198
So a rollout would mean working with a git's init.templateDir
to set up pre-commit
, and then retrofitting all the repositories on a developer's machine.
I'm working with gitleaks
and core.hooksPath
for our team at: https://github.com/pburkholder/caulking which should be ready for review today.
So I heard back from Bryan Alexander that gitleaks
is not yet on GEAR. They have their own build and hashes. I’m not sure what level of validation to apply here.
Do I use GSA’s gitleaks
and await them for features and bugfixes?
Do I trust homebrew?
Do I make my own homebrew formula with hashes that I’ve verified? Or get the binaries from the signed releases on GitHub…?
For now I’m trusting homebrew, but I wish it were using a signed release (4.0.1) instead of an unsigned one (4.1.0).
User Story:
As someone in TTS, I want to use a maintained tool for secrets.
Problem Statement:
We use a fork of git-seekret which isn't maintained. We should investigate using git-secrets or something like it.
Background information:
Actions to take:
Acceptance criteria:
Supporting Documentation:
git-seekret git-secrets
Related Issues:
https://github.com/18F/laptop/issues/184 https://github.com/18F/laptop/issues/189 https://github.com/18F/tts-tech-portfolio/issues/91