Controlling visibility of ASM functions

[vszakats]

I'd like to limit visibility of non-exported LibreSSL functions. For C functions this is doable with -fvisibility=hidden. For ASM functions however, llvm/gcc don't offer a built-in command-line option.

For macOS, it can be worked around with CPPFLAGS=-Dglobl=private_extern. Converting each of these lines:

.globl <func>

to

.private_extern <func>

Effect: https://github.com/curl/curl-for-win/actions/runs/7159163777/job/19492134204#step:3:9430

This is a case when LibreSSL is statically linked to a libcurl shared lib intending to expose only the libcurl interface. The same should apply when building libcrypto shared lib and wanting to hide these internal symbols.

For Linux / ELF, this workaround doesn't work because Linux needs a extra line to add the 'hidden' attribute to the declaration:

.hidden <func>
.globl <func>

Effect when tested with AES_cbc_encrypt: https://github.com/curl/curl-for-win/actions/runs/7159857921/job/19493575609#step:3:11893

Without this, ASM symbols are visible from a shared lib: https://github.com/curl/curl-for-win/actions/runs/7159163777/job/19492133628#step:3:11730

Some projects solve this by using this for each declaration:

LIBRESSL_ASM_FUNC_VISIBILITY(<func>)
.globl <func>

then do this in its headers:

#ifdef __ELF__
#define LIBRESSL_ASM_FUNC_VISIBILITY(func) .hidden func
#elif defined(__APPLE__)
#define LIBRESSL_ASM_FUNC_VISIBILITY(func) .private_extern func
#else
#define LIBRESSL_ASM_FUNC_VISIBILITY(func)
#endif

Could this (or something to this effect) be implemented in LibreSSL?

[botovq]

This should be doable with a bit of effort.

The bignum_* symbols already have some machinery for this: if S2N_BN_HIDE_SYMBOLS is defined, s2n_bignum_internal.h should define S2N_BN_SYM_PRIVACY_DIRECTIVE appropriately, and these are used in all the corresponding .S files, for example for bignum_cmul.

For the other assembly files, which are mostly generated from Perl, one probably has to adjust the translation scripts. For example these lines and these would be where I would start playing (for amd64).

[vszakats]

Thanks for the pointers! I could successfully hide the bignum ones with the options above. Then made a "feeling-lucky" patch to make the bignum logic generic for all libcrypto. Then hit a wall with the Perl generator. It seems that OpenSSL 3 also doesn't support this (but noticed support bits for Intel CET in it.)

This also fixes the issue, by patching the ASM sources locally:

find . -name '*-elf-x86_64.S' | sort | while read -r f; do
  awk '/^\.globl\t/ {s=$0; sub("^.globl", ".hidden", s); print s}; {print}' < "${f}" > "${f}.tmp"
  mv "${f}.tmp" "${f}"
done

[vszakats]

We might apply such transformation via update.sh. With a twist to add a macro, that has the default values globl, which can be overridden to hidden, private_extern (or others).

Draft patch:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 605cfde..c508e60 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -379,6 +379,18 @@ if(ENABLE_ASM)
    elseif(MINGW AND CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64")
        set(HOST_ASM_MINGW64_X86_64 true)
    endif()
+
+   option(HIDE_ASM_SYMBOLS "Hide symbols in assembly code" ON)
+   if(HIDE_ASM_SYMBOLS)
+       add_definitions(-DS2N_BN_HIDE_SYMBOLS)
+       if(HOST_ASM_ELF_X86_64)
+           add_definitions(-DLIBRESSL_ASM_VISIBILITY=hidden)
+       elseif(HOST_ASM_MACOSX_X86_64)
+           add_definitions(-DLIBRESSL_ASM_VISIBILITY=private_extern)
+       endif()
+   else()
+       add_definitions(-DLIBRESSL_ASM_VISIBILITY=globl)
+   endif()
 endif()

 if(CMAKE_SYSTEM_NAME MATCHES "Linux")
diff --git a/crypto/Makefile.am b/crypto/Makefile.am
index 0059b59..903f9d2 100644
--- a/crypto/Makefile.am
+++ b/crypto/Makefile.am
@@ -178,6 +178,8 @@ include Makefile.am.arc4random
 libcrypto_la_SOURCES =
 EXTRA_libcrypto_la_SOURCES =

+libcrypto_la_CPPFLAGS += -DLIBRESSL_ASM_VISIBILITY=globl
+
 include Makefile.am.elf-arm
 include Makefile.am.elf-mips
 include Makefile.am.elf-mips64
diff --git a/update.sh b/update.sh
index 1f2d78b..3366a62 100755
--- a/update.sh
+++ b/update.sh
@@ -308,6 +308,12 @@ for abi in elf macosx masm mingw64; do
    gen_asm        $abi x86_64cpuid.pl               cpuid-$abi-x86_64.S
 done

+# add option for visibility directive
+for i in `find . \( -name '*-elf-x86_64.S' -o -name '*-macosx-x86_64.S' \)`; do
+  awk '/^\.globl\t/ {s=$0; sub("^.globl", ".LIBRESSL_ASM_VISIBILITY", s); print s}; {print}' < "${i}" > "${i}.tmp"
+  $MV "${i}.tmp" "${i}"
+done
+
 # copy libtls source
 echo copying libtls source
 rm -f tls/*.c tls/*.h libtls/src/*.c libtls/src/*.h

[botovq]

I'm perfectly fine with adding build logic to the infrastructure to help support this. However, this logic should be picked up during the generation of the .S files. I'd really like to avoid adding more hacks that run sed/awk on these files afterward. That's super fragile. The endbr64 stuff should stop doing this, too (there's some work on this that can hopefully land relatively soon).

[vszakats]

Fair enough, but does that leave any place to implement this, outside of those Perl generator scripts?

[botovq]

On Mon, Dec 11, 2023 at 03:03:26AM -0800, Viktor Szakats wrote:

Fair enough, but does that leave any place to implement this, outside of those Perl generator scripts?

There is another option that should fix this, but I don't know what the status of these plans are and if anyone made actual progress on it: we could wrap the exported assembly routines in C routines and export only the latter. If I understand you right, this would solve your problem.

@4a6f656c, did this go anywhere?

[vszakats]

Wrapping them in C would have a number of other great advantages too.