add openssl support

[Stebalien]

Also adds conformance tests to make sure we're generating the same thing each
time.

Also removes:

1. RSA Encrypt. We can't support this with openssl, at least not easily. We also don't really support it generally as the security of reusing the same RSA key for encrypting and signing is questionable.
2. MarshalRsa{Private,Public}Key. These didn't return errors but openssl may.

fixes libp2p/go-libp2p-secio#42

[Stebalien]

Unfortunately, RSA is the only place where we use the PKCS1v15 signature format, as far as I can tell and I can't find an easy way to get something else from this openssl library.

[marten-seemann]

The OpenSSL library uses cgo, which afaik will cause the whole binary to be built on cgo mode, which prevents a lot of optimizations usually performed by the Go compiler.

Do we really need to fix this issue, considering that we’re in the procreas of phasing our secio right at this moment?

[Stebalien]

This is only enabled when -tags openssl is passed to the compiler.

The OpenSSL library uses cgo, which afaik will cause the whole binary to be built on cgo mode, which prevents a lot of optimizations usually performed by the Go compiler.

I know performance from go to C will suck but I've never heard of it preventing other optimizations.

[vyzo]

I take it the key binary format is the same, right? It's pretty crucial to be able to load existing keys.

[vyzo]

shouldn't we have a build in the matrix to test the go implementation as well?

[Stebalien]

(oops)

[marten-seemann]

Are there any other hosts than mars that are experiencing that problem and that are expected to compile their own IPFS? If not, this might be better left in a separate branch, instead of adding it to the master branch just to become technical debt as soon as the transition to TLS is completed.

I've never heard of it preventing other optimizations.

I think I’ve read an article about that some time ago, but I could remember it incorrectly.

[vyzo]

@marten-seemann the relays are pretty hammered with connections and spend a lot of time in RSA handshakes.

[marten-seemann]

@vyzo My main point is that the benefits of this workaround will be pretty short-lives, as we’re already deploying TLS: The last IPFS release added TLS support (but defaults to secio), and the plan is to switch to TLS by default (with secio as a fallback) with the next release.

[vyzo]

We'll have secio with us for a while, not everyone in the network updates promptly.
Besides, the debt is pretty minimal.

[raulk]

Note that all of this will be moved to go-libp2p-core.

Besides https://github.com/libp2p/go-libp2p-crypto/pull/61/files#r280682324, this LGTM.

Benchmarks, consistent with my observations in libp2p/go-libp2p-secio#42 (comment) 🚀

❯ openssl version
LibreSSL 2.6.5
❯ go test -test.bench=.*Benchmark.*RSA.* -test.benchmem | tee 1.txt
goos: darwin
goarch: amd64
pkg: github.com/libp2p/go-libp2p-crypto
BenchmarkSignRSA1B-8          	    5000	    341062 ns/op	   16137 B/op	     103 allocs/op
BenchmarkSignRSA10B-8         	    5000	    337009 ns/op	   16135 B/op	     103 allocs/op
BenchmarkSignRSA100B-8        	    5000	    342037 ns/op	   16142 B/op	     103 allocs/op
BenchmarkSignRSA1000B-8       	    5000	    347712 ns/op	   16139 B/op	     103 allocs/op
BenchmarkSignRSA10000B-8      	    5000	    362775 ns/op	   16140 B/op	     103 allocs/op
BenchmarkSignRSA100000B-8     	    3000	    556653 ns/op	   16137 B/op	     103 allocs/op
BenchmarkVerifyRSA1B-8        	    5000	    340684 ns/op	   16140 B/op	     103 allocs/op
BenchmarkVerifyRSA10B-8       	    5000	    337609 ns/op	   16137 B/op	     103 allocs/op
BenchmarkVerifyRSA100B-8      	    5000	    340003 ns/op	   16138 B/op	     103 allocs/op
BenchmarkVerifyRSA1000B-8     	    5000	    340086 ns/op	   16138 B/op	     103 allocs/op
BenchmarkVerifyRSA10000B-8    	    5000	    366710 ns/op	   16142 B/op	     103 allocs/op
BenchmarkVerifyRSA100000B-8   	    3000	    562177 ns/op	   16140 B/op	     103 allocs/op
PASS
ok  	github.com/libp2p/go-libp2p-crypto	21.866s
❯ go test -tags=openssl -test.bench=.*Benchmark.*RSA.* -test.benchmem | tee 2.txt
goos: darwin
goarch: amd64
pkg: github.com/libp2p/go-libp2p-crypto
BenchmarkSignRSA1B-8          	   20000	     97005 ns/op	     196 B/op	       5 allocs/op
BenchmarkSignRSA10B-8         	   20000	     95826 ns/op	     196 B/op	       5 allocs/op
BenchmarkSignRSA100B-8        	   20000	     98084 ns/op	     196 B/op	       5 allocs/op
BenchmarkSignRSA1000B-8       	   10000	    100483 ns/op	     196 B/op	       5 allocs/op
BenchmarkSignRSA10000B-8      	   10000	    118962 ns/op	     196 B/op	       5 allocs/op
BenchmarkSignRSA100000B-8     	    5000	    315869 ns/op	     196 B/op	       5 allocs/op
BenchmarkVerifyRSA1B-8        	   20000	     95454 ns/op	     196 B/op	       5 allocs/op
BenchmarkVerifyRSA10B-8       	   20000	    102122 ns/op	     196 B/op	       5 allocs/op
BenchmarkVerifyRSA100B-8      	   20000	    103856 ns/op	     196 B/op	       5 allocs/op
BenchmarkVerifyRSA1000B-8     	   10000	    103769 ns/op	     196 B/op	       5 allocs/op
BenchmarkVerifyRSA10000B-8    	   10000	    124242 ns/op	     196 B/op	       5 allocs/op
BenchmarkVerifyRSA100000B-8   	    5000	    335315 ns/op	     196 B/op	       5 allocs/op
PASS
ok  	github.com/libp2p/go-libp2p-crypto	26.499s
❯ benchcmp {1,2}.txt
benchmark                       old ns/op     new ns/op     delta
BenchmarkSignRSA1B-8            341062        97005         -71.56%
BenchmarkSignRSA10B-8           337009        95826         -71.57%
BenchmarkSignRSA100B-8          342037        98084         -71.32%
BenchmarkSignRSA1000B-8         347712        100483        -71.10%
BenchmarkSignRSA10000B-8        362775        118962        -67.21%
BenchmarkSignRSA100000B-8       556653        315869        -43.26%
BenchmarkVerifyRSA1B-8          340684        95454         -71.98%
BenchmarkVerifyRSA10B-8         337609        102122        -69.75%
BenchmarkVerifyRSA100B-8        340003        103856        -69.45%
BenchmarkVerifyRSA1000B-8       340086        103769        -69.49%
BenchmarkVerifyRSA10000B-8      366710        124242        -66.12%
BenchmarkVerifyRSA100000B-8     562177        335315        -40.35%

benchmark                       old allocs     new allocs     delta
BenchmarkSignRSA1B-8            103            5              -95.15%
BenchmarkSignRSA10B-8           103            5              -95.15%
BenchmarkSignRSA100B-8          103            5              -95.15%
BenchmarkSignRSA1000B-8         103            5              -95.15%
BenchmarkSignRSA10000B-8        103            5              -95.15%
BenchmarkSignRSA100000B-8       103            5              -95.15%
BenchmarkVerifyRSA1B-8          103            5              -95.15%
BenchmarkVerifyRSA10B-8         103            5              -95.15%
BenchmarkVerifyRSA100B-8        103            5              -95.15%
BenchmarkVerifyRSA1000B-8       103            5              -95.15%
BenchmarkVerifyRSA10000B-8      103            5              -95.15%
BenchmarkVerifyRSA100000B-8     103            5              -95.15%

benchmark                       old bytes     new bytes     delta
BenchmarkSignRSA1B-8            16137         196           -98.79%
BenchmarkSignRSA10B-8           16135         196           -98.79%
BenchmarkSignRSA100B-8          16142         196           -98.79%
BenchmarkSignRSA1000B-8         16139         196           -98.79%
BenchmarkSignRSA10000B-8        16140         196           -98.79%
BenchmarkSignRSA100000B-8       16137         196           -98.79%
BenchmarkVerifyRSA1B-8          16140         196           -98.79%
BenchmarkVerifyRSA10B-8         16137         196           -98.79%
BenchmarkVerifyRSA100B-8        16138         196           -98.79%
BenchmarkVerifyRSA1000B-8       16138         196           -98.79%
BenchmarkVerifyRSA10000B-8      16142         196           -98.79%
BenchmarkVerifyRSA100000B-8     16140         196           -98.79%

[raulk]

@marten-seemann yeah, we'll need to support SecIO with RSA keys for a while...

[raulk]

Sidenote: so happy we don't have to gx new imports any longer.

[vyzo]

I am dying to get this bubbled up and deployed to the relays!

[raulk]

@vyzo while this is definitely worth a release, in general we're abusing releases by making one per PR merged (almost). Just do this for now, and you're set:

~/Workbench/protocol/workspace-go-libp2p/go-libp2p-daemon master
❯ go get github.com/libp2p/go-libp2p-crypto@feat/openssl
go: finding github.com/libp2p/go-libp2p v0.0.20
go: finding github.com/libp2p/go-libp2p-kad-dht v0.0.10
go: finding github.com/libp2p/go-libp2p-peerstore v0.0.5
go: finding github.com/libp2p/go-libp2p-autonat-svc v0.0.5
go: finding github.com/libp2p/go-libp2p-kbucket v0.1.1
go: finding github.com/libp2p/go-conn-security-multistream v0.0.2
go: finding github.com/libp2p/go-libp2p-transport-upgrader v0.0.2
go: finding github.com/libp2p/go-libp2p-swarm v0.0.3
go: finding github.com/libp2p/go-libp2p-secio v0.0.3
go: finding github.com/libp2p/go-msgio v0.0.2
go: finding github.com/libp2p/go-libp2p-crypto feat/openssl
go: downloading github.com/libp2p/go-libp2p-crypto v0.0.2-0.20190503032304-848064968c37
go: extracting github.com/libp2p/go-libp2p-crypto v0.0.2-0.20190503032304-848064968c37

[vyzo]

What's wrong with one release per PR when merging in major new functionality? These are patch releases, not minor increments.

[vyzo]

In the meantime I'll deploy relays from a branch, using this branch, to get some early testing.

[raulk]

What's wrong with one release per PR when merging in major new functionality? These are patch releases, not minor increments.

Non-comprehensive list of reasons: too much noise, unpredictable for downstream users, haphazard, difficult to keep tabs on, high risk of making bad/incomplete releases, lack of planning...

Basically: we are abusing patch releases as a way to compensate for lack of continuous integration and nightly builds.

You should have an easy way to run the latest daemon commit pulling in all transitive masters -- that's where https://github.com/libp2p/workspace-go-libp2p is headed, but incomplete.

[vyzo]

I am not interested in running the daemon locally, but rather deploying it to the relay fleet.
Regardless, I am doing that from a branch.

Basically: we are abusing patch releases as a way to compensate for lack of continuous integration and nightly builds.

That's heavy. I don't think we are abusing anything by making a patch release for each new major PR merged in. And it makes fleet maintenance much easier.

[raulk]

@vyzo you are only thinking about your needs, we should think about users too ;-)

Offloading the conversation to libp2p/go-libp2p#621. The lack of CI creates many deficiencies as pointed out in that issue, and patch releases ain't gonna solve those (it's still a manual process based on developer judgement).

[vyzo]

I would argue that our users do care about having the latest patch release be up-to-date with master :)
Anyway, end of discussion here, we have an issue for it now.

[Stebalien]

If not, this might be better left in a separate branch, instead of adding it to the master branch just to become technical debt as soon as the transition to TLS is completed.

WRT technical debt, this package isn't changed all that much so I don't expect too much maintenance burden. If that changes, this change is also mostly invisible so we should be able to just remove it without downstream noticing (we're not extending any interfaces).