move common/crypto impl to extensions for openssl

[bdecoste]

Signed-off-by: William DeCoste bdecoste@gmail.com

For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md

Description: Move common/crypto impl (i.e. utility.cc) to extensions to make it clear that the impl is ssl-impl specific (e.g. boringgsl vs openssl) and easier to plug in an openssl impl.
Risk Level: Low
Testing: Passes all standard tests
Docs Changes: None
Release Notes: None
[Optional Fixes #Issue]
[Optional Deprecated:]

[stale]

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

[bdecoste]

Working on an update per the review

[bdecoste]

@lizan @PiotrSikora would you mind doing a re-review? Thanks

[venilnoronha]

Thanks for this!

[lizan]

LGTM, Thanks for the patience.

[bdecoste]

@lizan Thanks!

[moderation]

@bdecoste I think the answer is yes, but am I able to comment out the line https://github.com/envoyproxy/envoy/blob/master/source/extensions/extensions_build_config.bzl#L107 following the instructions at https://github.com/envoyproxy/envoy/tree/master/bazel#disabling-extensions when using the default boringssl configuration and have the build work? Put another way, is this line /extension only relevant when trying to build with openssl?

[venilnoronha]

@moderation that was added in #8745. It's needed for both BoringSSL and non-BoringSSL builds. See https://github.com/envoyproxy/envoy-openssl also.

[mattklein123]

@venilnoronha @moderation @lizan FWIW I'm not thrilled that "extension" was added. IMO it should be the default that utility is enabled for the normal build and openssl should have some other way of turning off.

[moderation]

Still confused too. If it is needed for both BoringSSL and non-BoringSSL it sounds mandatory and shouldn't have an option to compile it out.

[alyssawilk]

coming back to this w.r.t #9953 I think we either need to make this a proper extension, and link it in and out, or move it back to code and just have openssl folks overwrite it.
I tried just removing deps and we have core code (source/common/config/remote_data_fetcher.cc) which depends on a utility singleton (UtilitySingleton) which is in extensions.

@bdecoste are you folks up for doing the work to make it a regular extension, or if I move the code back (but keep it a separate library) is that OK by you?

[rojkov]

It seems we don't have resources to maintain boringssl optional, so it's ok to move the code back to core.

/cc @dmitri-d

[dmitri-d]

if I move the code back (but keep it a separate library) is that OK by you?

Yeah, I think that's the way to go -- There are a lot of changes in the core to support OpenSSL anyway, I don't think it makes sense to maintain a few utility calls as an extension...