fix(test): Use (legacy) deterministic ECDSA key generation #38
Conversation
Fix #37 Signed-off-by: Thomas Fossati <[email protected]>
Signed-off-by: Thomas Fossati <[email protected]>
Signed-off-by: Thomas Fossati <[email protected]>
| @@ -19,7 +20,7 @@ func TestAppraisalExtensions_SetGetKeyAttestation_ok(t *testing.T) { | |||
| }, | |||
| } | |||
|
|
|||
| kp, err := ecdsa.GenerateKey(elliptic.P256(), new(zeroSource)) | |||
| kp, err := keygen.ECDSALegacy(elliptic.P256(), new(zeroSource)) | |||
There was a problem hiding this comment.
I am not against the fix as such:
However, is it really needed ? Should we not seek feedback on the issue with ECDSA Library maintainers! Then we should not rely on third party library as a workaround?
There was a problem hiding this comment.
If you want to make the case for reverting a change in the Go standard library I am certainly not stopping you :-D
From a personal/pragmatic point of view, I am content with this minimal-impact fix.
There was a problem hiding this comment.
Yes, raised an issue:
golang/go#69248
Please review, if you want to add anything on it!
There was a problem hiding this comment.
Also if the new change is by design should we not use a Non-Zero Reader in our code to avoid crash, instead of fundamentally by-pass the core golang module ??
We will soon become stale using the library, which we are un-aware of?
I mean filippo.io/keygen here:
There was a problem hiding this comment.
The test needs to construct a key deterministically.
There was a problem hiding this comment.
The test needs to construct a key deterministically.
yes, what I meant was we can supply a constant deterministic byte array to the input rather than always zero
There was a problem hiding this comment.
We will soon become stale using the library, which we are un-aware of?
I mean
filippo.io/keygenhere:
No, filippo.io/keygen is forever stable - that's the promise.
Instead, if we use GenerateKey(), golang makes no promises about the stability of the output. From the ecdsa package documentation:
"Note that the returned key does not depend deterministically on the bytes read from rand, and may change between calls and/or between versions."
|
As discussed f2f, the simpler and most effective change is to hardcode a valid ECDSA key in the test. |
Fix #37