Conditionally enable OPENSSL_NO_ASM for Visual Studio

[zackgalbreath]

This configuration variable was originally disabled due to a bug
in CMake. If we are using a "new enough" version of CMake (3.13+)
we can now enable this flag for Visual Studio builds.

@karthikravis
@jtattermusch

see #16376

[jtattermusch]

The concern is whether I need to install nasm or some other assembler that's not part of visual studio to be able to build without OPENSSL_NO_ASM - if so, it could be a regression for users that currently build with cmake + VS.

[zackgalbreath]

The concern is whether I need to install nasm or some other assembler that's not part of visual studio to be able to build without OPENSSL_NO_ASM - if so, it could be a regression for users that currently build with cmake + VS.

We could use CheckLanguage from within cmake/ssl.cmake to make sure that the ASM_NASM language can be enabled. Does that sound like a reasonable safeguard to you?

[jtattermusch]

I tried on a machine that doesn't have NASM installed and got this error:

-- Didn't find assembler
CMake Error at third_party/boringssl/crypto/CMakeLists.txt:60 (enable_language):                                                                                                                  
No CMAKE_ASM_NASM_COMPILER could be found.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                    
-- Configuring incomplete, errors occurred!

[jtattermusch]

The concern is whether I need to install nasm or some other assembler that's not part of visual studio to be able to build without OPENSSL_NO_ASM - if so, it could be a regression for users that currently build with cmake + VS.

We could use CheckLanguage from within cmake/ssl.cmake to make sure that the ASM_NASM language can be enabled. Does that sound like a reasonable safeguard to you?

I don't have experience with CheckLanguage, but it could work.
We should only enable asm if we're sure it's not going to break people's builds.
Perhaps we could have some setting to specify that we really want assembly optimizations (and avoid setting OPENSSL_NO_ASM if that setting is used) - the idea is that some users might want to ensure that ASM optimizations are actually being used (as opposed to disabling them silently which can have performance implications)

[davidben]

From the BoringSSL side, consider the assembly all but mandatory. It may be worth breaking builds with an explicit opt-out. Cryptography has more complex correctness requirements than most code. It's performance-sensitive and needs to meet some odd "constant-time" security requirements.

Two of the most important symmetric algorithms, AES and GHASH (the hash in AES-GCM), do not map well to portable C. Meeting constant-time security requirements takes performance tradeoffs. With assembly, we can use dedicated hardware instructions on modern CPUs. Assembly also lets us use SIMD instructions for a middle tier of fallback: given SSSE3 (x86) or NEON (arm), we can build AES and GHASH out of byte shuffle operations.

OPENSSL_NO_ASM limits you to bottom of the fallback tower. Until a month ago, that fallback was not even constant-time, meaning assembly is required for security. It is not just an optimization. This is poor, so I've been replacing those fallbacks. Our fallback GHASH is constant-time as of November, but I still need to fix the AES fallback. However, these fixes come with perf hits. (Really this is undoing a perf win we never earned.) When we evaluate the tradeoffs here, we assume that assembly is enabled.

The TLS library will prioritize ChaCha20-Poly1305 when AES hardware is unavailable (either due to the hardware itself or OPENSSL_NO_ASM), but AES-GCM is still widely-used. (Note since that post was written, ARMv8 added crypto extensions. AES hardware is now common on mobile too.)

[jtattermusch]

@davidben thanks for the input, we will consider this, but I'd like to consider it separately from this PR.
If OPENSSL_NO_ASM if discouraged, we'd need a solution for e.g. older cmake versions, which is not something we are trying to address in this PR.
For purposes of this PR, let's not require users to have NASM installed (= same behavior as before), but let's use it where possible.
Later (in a different PR), we can change the behavior into only using OPENSSL_NO_ASM when explicitly specified by the user, if that's something that will improve the overall security.

[jtattermusch]

LGTM once tests pass.

[jtattermusch]

@veblush need another Googler's LGTM.