crypto: add better support for alternative backends

[Jorropo]

Based on #1683.

[Stebalien]

Please don't remove OpenSSL support.

• OpenSSL is packaged for many systems, boring crypto isn't.
• This is an experiment that may never land.

Adding support for boringcrypto makes sense, just don't drop OpenSSL support.

[Jorropo]

OpenSSL is packaged for many systems, boring crypto isn't.

I don't have boring crypto installed on my system and yet it work.
I think this is because I have an old enough version of openssl that the API still match boring.
I don't know what happen if you try doing this with openssl v3.

I have no problem adding openssl back in, however this discussion started because it seems no one has time to maintain go-openssl.
I'll wait for a definitive answer from libp2p maintainers (as they are suposed to maintain go-openssl ?).

[MarcoPolo]

OpenSSL is packaged for many systems, boring crypto isn't.

My understanding is that boringssl is packaged within applications. It's not dynamically linked from the system libraries.

Programs ship their own copies of BoringSSL when they use it...

From https://boringssl.googlesource.com/boringssl/

This is an experiment that may never land.

Seems like there's appetite for it though. This came from a separate branch to being part of the standard branch and enabled with an environment variable.

I need to think a bit more about the go-openssl bit, maybe not worth removing it just yet. But if we get the same performance benefits and we get rid of a dependency it seems tempting.

[Stebalien]

My understanding is that boringssl is packaged within applications. It's not dynamically linked from the system libraries.

Ah, that makes more sense.

I need to think a bit more about the go-openssl bit, maybe not worth removing it just yet. But if we get the same performance benefits and we get rid of a dependency it seems tempting.

I'm confused. go-openssl works now and doesn't require a fork of the go compiler. BoringSSL support isn't a replacement.

Removing go-openssl in favor of boringcrypto would be a matter of removing a working feature that people are using in production, in favor of an experimental feature that requires a custom go toolchain. That's definitely not how you keep your users happy.

[MarcoPolo]

Removing go-openssl in favor of boringcrypto would be a matter of removing a working feature that people are using in production, in favor of an experimental feature that requires a custom go toolchain.

I misunderstood how boringcrypto works. I thought you had to run go build with the env flag, I didn’t realize you had to build the Go compiler with the env flag. I agree that doesn’t make sense as a replacement.

Maybe in the future it becomes part of the standard toolchain, until then I agree that we should keep the go-openssl library (it’s pretty trivial anyways).

[marten-seemann]

I misunderstood how boringcrypto works. I thought you had to run go build with the env flag, I didn’t realize you had to build the Go compiler with the env flag. I agree that doesn’t make sense as a replacement.

I had the same misunderstanding. @Jorropo, are you aware of any plans that this might make it into the standard Go build chain?

I need to think a bit more about the go-openssl bit, maybe not worth removing it just yet. But if we get the same performance benefits and we get rid of a dependency it seems tempting.

Indeed, that was the plan. It would be super nice if we could drop the OpenSSL dependency, especially since 1. the v3 release complicates things, as we now have to maintain compatibility with two versions of OpenSSL for the foreseeable future and 2. there’s an upcoming hard fork of OpenSSL, and I already see the inevitable ask to make the library work with that fork as well.

It would be a sad outcome, if we now had to maintain 3 different versions (standard library crypto, OpenSSL and Boring) in the long term. I’m ok with doing it for a limited amount of time, to give us time try out Boring (would be good to get some real-world performance data). I’d regard relying on a custom build of the Go toolchain is a dealbreaker, so it would be good to get some clarity on the Go team’s plans here.

[Jorropo]

are you aware of any plans that this might make it into the standard Go build chain?

No

[Jorropo]

Update:
I have reverted the openssl changes and bumped to the v3 compatible version.
I have also updated the tests to not rebuild go (idk why it didn't worked the first time I've tried it, probably a typo in the env varaible name, anyway it does now).

[marten-seemann]

LGTM. This should be part of the v0.22.0 release (it is anyway, because the #1688 also updates the respective dependency, but let's make it explicit).

@Stebalien, can you please re-review this?

[Jorropo]

I've benchmarked the things with the current PR and I see slightly different results than when I tested stuff directly (without going through core/crypto package):

	Go Native	Openssl	Boring
SignRSA1B-12	1.16ms	0.52ms	0.51ms
SignRSA10B-12	1.16ms	0.52ms	0.52ms
SignRSA100B-12	1.18ms	0.52ms	0.52ms
SignRSA1000B-12	1.17ms	0.52ms	0.52ms
SignRSA10000B-12	1.19ms	0.53ms	0.52ms
SignRSA100000B-12	1.24ms	0.57ms	0.57ms
VerifyRSA1B-12	45.5µs	7.5µs	14.5µs
VerifyRSA10B-12	42.7µs	7.6µs	14.4µs
VerifyRSA100B-12	45.3µs	7.5µs	14.5µs
VerifyRSA1000B-12	44.4µs	8.0µs	14.8µs
VerifyRSA10000B-12	48.2µs	12.4µs	19.3µs
VerifyRSA100000B-12	95.2µs	56.6µs	63.3µs
SignEd255191B-12	22.8µs	22.8µs	23.9µs
SignEd2551910B-12	22.9µs	22.8µs	23.8µs
SignEd25519100B-12	22.9µs	22.9µs	24.2µs
SignEd255191000B-12	25.6µs	25.3µs	27.2µs
SignEd2551910000B-12	49.2µs	49.4µs	56.5µs
SignEd25519100000B-12	286µs	283µs	343µs
VerifyEd255191B-12	55.7µs	55.1µs	56.5µs
VerifyEd2551910B-12	56.3µs	55.4µs	56.4µs
VerifyEd25519100B-12	56.2µs	55.8µs	56.0µs
VerifyEd255191000B-12	57.5µs	56.3µs	58.0µs
VerifyEd2551910000B-12	67.6µs	68.5µs	71.5µs
VerifyEd25519100000B-12	190µs	187µs	218µs

I don't know why boring is slower in some cases, so this probably need more investigation before people start using it in production (I still think it's fine to merge as is for now).

[MarcoPolo]

I misunderstood how boringcrypto works. I thought you had to run go build with the env flag, I didn’t realize you had to build the Go compiler with the env flag. I agree that doesn’t make sense as a replacement.

I misunderstood again, GOEXPERIMENT=boringcrypto go build works with the standard go 1.19 compiler/toolchain. Users don't need to build their own toolchain. I originally thought you had to set this env var when building the Go compiler from source, that's not the case.

Anyways it's still marked experimental and we don't need to make the decision to drop openssl now. This PR is good as is. Thanks for adding the boringcrypto version to the test matrix @Jorropo . and sorry for the confusion here.

[Jorropo]

Rebased on master due to #1689 (:tada:)

[marten-seemann]

I reran the RSA part of the benchmark on a Linode machine with 2 virtual CPUs (AMD EPYC 7642 48-Core Processor) and Go 1.19.4, and I'm getting slightly different numbers, that make BoringSSL look a lot more attractive, especially for signature verification.

	Go Standard Library	OpenSSL	BoringSSL
BenchmarkSignRSA1B-2	1982.767	679.99	695.587
BenchmarkSignRSA10B-2	1964.801	663.548	686.936
BenchmarkSignRSA100B-2	2046.494	677.749	697.331
BenchmarkSignRSA1000B-2	1957.661	678.97	679.613
BenchmarkSignRSA10000B-2	2054.141	690.729	687.625
BenchmarkSignRSA100000B-2	2146.577	780.73	722.816
BenchmarkVerifyRSA1B-2	80.53	14.041	19.37
BenchmarkVerifyRSA10B-2	99.67	13.909	18.553
BenchmarkVerifyRSA100B-2	100.073	15.279	18.601
BenchmarkVerifyRSA1000B-2	97.549	17.234	18.93
BenchmarkVerifyRSA10000B-2	86.147	20.472	24.594
BenchmarkVerifyRSA100000B-2	159.112	79.88	79.879

(all values in microseconds)

[Jorropo]

@marten-seemann thoses benchmarks have similar results to my previous ones: #1686 (comment)

I was disappointed by the Ed25519 performance regressions. Given that unlike openssl, we can't use it only for RSA. If you enable boring it's for everything.