TestGetKey fails with Go >= 1.11

[anthonyfok]

As seen on https://travis-ci.org/cloudflare/gokey, with https://travis-ci.org/cloudflare/gokey/builds/474000487 from PR #16 in particular, the test TestGetKey fails on Go 1.11.4 as well as master (Go 1.12 in beta), while it passes on Go 1.10.7:

--- FAIL: TestGetKey (26.58s)
    gokey_test.go:161: keys with same invocation options do not match

This was first detected by "autopkgtest" on Debian CI when we migrated the default Go version from 1.10 to 1.11, as reported at https://tracker.debian.org/pkg/golang-defaults:

• autopkgtest for gokey/0.0~git20170602.05f83bb-1: amd64: Regression ♻
• https://ci.debian.net/packages/g/gokey/unstable/amd64/

Thanks in advance! Have a Happy New Year!

Anthony

[anthonyfok]

On closer look, it seems that only RSA2048 and RSA4096 are failing the test on Go >= 1.11; the others (EC256, EC521, X25519 and ED25519) are fine.

[anthonyfok]

Further Google search reveals the answer: RSA key generation, for example, has been intentionally made non-deterministic. From the Go 1.11 release notes:

crypto
Certain crypto operations, including ecdsa.Sign, rsa.EncryptPKCS1v15 and rsa.GenerateKey, now
randomly read an extra byte of randomness to ensure tests don't rely on internal behavior.

The actual upstream CL is here:

• https://go-review.googlesource.com/c/go/+/64451/
  crypto: randomly read an extra byte of randomness in some places.

And two other projects encountering the same issue:

• Go1.11 crypto changes in RSA key generation will break TestVectorsRSA square/go-jose#189
• math/rand (or crypto/rsa): non-deterministic output after go1.11.2 golang/go#29290

[ignatk]

Thanks for the report and investigation. Does #18 look good to you?

[anthonyfok]

Thank you for the fix!

I was trying to fix an apparent regression during Debian's transition from Go 1.10 to 1.11 which affected gokey, see https://ci.debian.net/packages/g/gokey/testing/amd64/

I must admit I didn't know much about gokey personally (it was packaged by another Debian Maintainer), so my temporary fix was to "Disable tests for RSA2048 and RSA4096 in TestGetKey" in the Debian package, see https://tracker.debian.org/news/1016281/accepted-gokey-00git20181023b4e2780-1-source-into-unstable/

But now you have a much more comprehensive fix (I didn't know gokey needs the keys to be deterministic, but yeah, it does make sense), I will package your new version ASAP.

Thanks again!