rm verify_test_dsa.go #35
Conversation
DSA signature verification is no longer supported since Go 1.16. Go <= 1.21 are no longer supported. Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
50dd16e to
1b700ef
Compare
| os.Remove(tmpContentFile.Name()) // clean up | ||
| } | ||
|
|
||
| var dsaPrivateKey = []byte(`-----BEGIN PRIVATE KEY----- |
There was a problem hiding this comment.
This line was causing a confusion to key leakage detection tools
| @@ -1,182 +0,0 @@ | |||
| // +build go1.11 go1.12 go1.13 go1.14 go1.15 | |||
There was a problem hiding this comment.
This line was apparently intended to compile the test only on Go 1.11-1.15.
However, this was actually compiled for every Go (>= 1.11) releases.
The test was not executed by go test because the file does not have _test.go suffix.
|
Thanks for opening this up @AkihiroSuda! |
|
@azazeal could you please take a look when you get a chance? Thanks! |
There was a problem hiding this comment.
I think that renaming the test file from verify_test_dsa.go to verify_dsa_test.go would probably negate the need to remove the test, as the go.mod specifies that the minimum required Go version is 1.14.
Will also bring @hslatman in this.
I assume this would also clear up any confusion caused to security tools. |
Why retain the test? |
I understand that. What I'm trying to articulate is that the project's Renaming the file would allow us to keep the test that satisfies these edge cases, while at the same time clear up any confusion caused to the analysis tools. |
|
Thank you @AkihiroSuda for noting the tests weren't performed. It likely never was an issue before, and when we adopted the package we must've missed it 😅 We've merged #37, which is based on your commit, but kept the test intact for the older Go versions. The test file should not result in scanners alerting anymore now, but do tell if they do. Released in https://github.com/smallstep/pkcs7/releases/tag/v0.1.1. |
|
Thanks! |
|
@hslatman @AkihiroSuda thanks for getting this in! Would you be able to cut a new release tag with this fix please? |
It should already be in https://github.com/smallstep/pkcs7/releases/tag/v0.1.1, but do let me know if it doesn't fix the scanner issues. |
Name of feature:
Not a feature.
Pain or issue this feature alleviates:
The test have been actually skipped by
go testbecause it doesn't have the_test.gosuffix.vendordirectory of projects consuming this package. Thevar dsaPrivateKey = ...line (L128) could cause confusion to some security scanning tools.Why is this important to the project (if not answered above):
Answered above.
Is there documentation on how to use this feature? If so, where?
Not a feature.
In what environments or workflows is this feature supported?
Not a feature.
In what environments or workflows is this feature explicitly NOT supported (if any)?
Not a feature.
Supporting links/other PRs/issues:
N/A
💔Thank you!