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

README rsa key example is flagged by security scanners #1820

Closed
marcm-ml opened this issue Feb 13, 2024 · 9 comments · Fixed by #1823
Closed

README rsa key example is flagged by security scanners #1820

marcm-ml opened this issue Feb 13, 2024 · 9 comments · Fixed by #1823

Comments

@marcm-ml
Copy link
Contributor

Hey,

this line is flagged by security scanners (trivy for example). Its a false positive, but maybe you can replace it by some placeholder.

gpg: using RSA key 27C50E7F590947D7273A741E85194C08421980C9

https://aquasecurity.github.io/trivy/v0.27.1/docs/secret/scanning/

@marcm-ml marcm-ml changed the title Readme rsa key example is flagged by security scanners README rsa key example is flagged by security scanners Feb 13, 2024
@Byron
Copy link
Member

Byron commented Feb 13, 2024

Thanks for bringing this up!

Definitely, let's replace it, and a PR is definitely welcome.

@EliahKagan
Copy link
Contributor

EliahKagan commented Feb 13, 2024

Should the How to verify a release (DEPRECATED) section of the README, of which that RSA key is a part, be kept any longer?

It's been marked as deprecated for a while. Are the instructions in it possible to use? If its inclusion is only historical, then it could probably be replaced with a brief note at the end of the preceding section linking somewhere like gitpython-developers/gitdb#77, which is where the deprecation notice links. However, I notice gitpython-developers/gitdb#77 is still open, and from gitpython-developers/gitdb#77 (comment) I am not sure what, if anything, makes sense to do with it now.

Definitely, let's replace it, and a PR is definitely welcome.

If the fingerprint is to be replaced with a placeholder, then should the suffix shown in the gpg --edit-key command likewise be replaced? If so, and the instructions were possible to follow, then they will cease to be possible to follow unless elaboration is added.

gpg --edit-key 4C08421980C9

@Byron
Copy link
Member

Byron commented Feb 13, 2024

It's true - for some reason I was thinking the offending text must be in a source file, but instead it's in a README that tries to explain how to validate a signature.

The public key is also stored in release-verification-key.asc, and it seem to not be the same key anymore as in the line 248, but I am unsure about all of it since I don't think it's in use anymore.

Probably it's fine to remove the section entirely even as a quick fix if its applicability can indeed be doubted, which is faster than fixing it for dubious value.

@marcm-ml
Copy link
Contributor Author

Would you want me to make a PR removing the problematic/deprecated section?

@Byron
Copy link
Member

Byron commented Feb 14, 2024

Yes, that seems like the right thing todo. Thank you.

@marcm-ml
Copy link
Contributor Author

Just out of curiosity, when would you plan the next release which contains the updated README?

@EliahKagan
Copy link
Contributor

EliahKagan commented Feb 15, 2024

Should the existing release-verification-key.asc, mentioned in #1820 (comment), be removed too?

Also, should gitpython-developers/gitdb#77 in gitdb be considered resolved by the combination of the verification instructions having been deprecated and their removal in #1823? I ask because gitpython-developers/gitdb#77 had been the readme-linked issue documenting the deprecation, and it doesn't look like there's any recent publicly expressed interest in trying to bring back that kind of verification procedure.

@Byron
Copy link
Member

Byron commented Feb 15, 2024

Just out of curiosity, when would you plan the next release which contains the updated README?

I will try to create a new release tonight - a lot has changed since the last release so it seems worth it.

Should the existing release-verification-key.asc, mentioned in #1820 (comment), be removed too?

I would keep it as it probably still can used to verify older release for those who are into it. Those who are interested in verifying signatures will probably know how to use it as well.

With that said, I am sure it will be removed eventually, and I wouldn't even object to a PR that removes it right away either.

Also, should gitpython-developers/gitdb#77 in gitdb be considered resolved by the combination of the verification instructions having been deprecated and their removal in #1823? I ask because gitpython-developers/gitdb#77 had been the readme-linked issue documenting the deprecation, and it doesn't look like there's any recent publicly expressed interest in trying to bring back that kind of verification procedure.

That's a good point! I will close it with reference to this conversation.

@EliahKagan
Copy link
Contributor

I will try to create a new release tonight

In case it comes up and people find this by searching, note that the CI check failure on the release commit 1f37b48 is not related to this or any other changes that went into 3.1.42, it is just an intermittent failure due to #1676 and it does not reflect a problem in the GitPython library.

I would keep it [...] With that said, I am sure it will be removed eventually [...]

This can probably be done at the same time as #1708 -- see #1708 (reply in thread) and #1716 (comment) -- if it doesn't end up being removed before then. That should bring attention back to it even if it has been forgotten.

That's a good point! I will close it with reference to this conversation.

That issue, gitpython-developers/gitdb#77, is still open, which seems like it may be unintentional.

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

Successfully merging a pull request may close this issue.

3 participants