Skip to content

Conversation

atoomic
Copy link
Contributor

@atoomic atoomic commented Dec 30, 2021

When compiling Perl or a binary with different versions
of tomcrypt this is going to have unexpected behaviors
as we cannot guarantee which flavor of the C function is used.

This changeset is isolating all the 'der_' C functions
by adding a prefix 'cryptx_der_
' so they are unique
and would not conflict with another library linked to our binary.

Recipe replacement rules applied:
replace ' der_' ' cryptx_der_'
replace '(der_' '(cryptx_der_'
replace ')der_' ')cryptx_der_'
replace '!der_' '!cryptx_der_'
replace '=der_' '=cryptx_der_'
replace '= der_' '= cryptx_der_'
replace 'cryptx_der_to_pem' 'der_to_pem' lib/**/*..pm

Note: we should consider adding this to src/update-libtom.pl

@atoomic
Copy link
Contributor Author

atoomic commented Dec 30, 2021

I just run into some issues with a compiled binary which was using libtomcrypt from multiple sources
and linked against multiple versions if the 'der_' helpers

In my example these two libraries are providing 'der_*' helpers

  • /lib/x86_64-linux-gnu/libhx509.so.5
  • 532/lib/perl5/cpanel_lib/x86_64-linux-64int/auto/CryptX/CryptX.so

der_length_integer is available from both libraries
but only der_decode_integer is availabe from CryptX.so

At the end the binary is confused by which version to use

@atoomic
Copy link
Contributor Author

atoomic commented Dec 30, 2021

also reported the issue to heimdal as heimdal/heimdal#911

@FGasper
Copy link
Contributor

FGasper commented Dec 31, 2021

Looks like this addresses the same issue as reported in #68.

@karel-m
Copy link
Contributor

karel-m commented Dec 31, 2021

As I mentioned in #68 I do know how to fix this properly. The solution I would like will create CryptX.so in a way that it does not export "unwanted" symbols.

@atoomic
Copy link
Contributor Author

atoomic commented Dec 31, 2021

we should consider something like this by default if possible to do not advertise the symbols

CFLAGS="-Wl,--exclude-libs,ALL -fvisibility=hidden" perl Makefile.PL

@karel-m
Copy link
Contributor

karel-m commented Dec 31, 2021

Well, CFLAGS hack did not work for me, the resulting CryptX.so still exports a tons of functions.

But my experiments with LDDLFLAGS => "$Config{lddlflags} -Wl,--version-script=ldscript.version" seems to be promising.

@karel-m
Copy link
Contributor

karel-m commented Dec 31, 2021

@atoomic could you please try this simple patch

diff --git a/Makefile.PL b/Makefile.PL
index dcc55858..101478e1 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -62,6 +62,9 @@ else {
     MYEXTLIB => "src/liballinone$Config{lib_ext}",
     clean    => { 'FILES' => join(' ', @myobjs, "src/liballinone$Config{lib_ext}") },
   );
+  if ($Config{ld} =~ /gcc|g\+\+/) {
+    push @EUMM_INC_LIB, (LDDLFLAGS => "$Config{lddlflags} -Wl,--exclude-libs,ALL");
+  }
 }

 my %eumm_args = (

Any suggestions how to improve if (..) condition are appreciated.

@karel-m
Copy link
Contributor

karel-m commented Dec 31, 2021

BTW this kind of "protection" should be IMO a feature of perl toolchain.

@atoomic
Copy link
Contributor Author

atoomic commented Dec 31, 2021

@karel-m sure I m going to test but I think the one we want is -fvisibility=hidden
I was working on a similar patch

@atoomic
Copy link
Contributor Author

atoomic commented Dec 31, 2021

my current patch looks like this, testing your now

diff --git a/Makefile.PL b/Makefile.PL
index dcc5585870..541c110121 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -31,6 +31,11 @@ else {
   $mycflags .= " $ENV{CFLAGS} "   if $ENV{CFLAGS};
   $mycflags .= " $ENV{CPPFLAGS} " if $ENV{CPPFLAGS};

+  # avoid der_* functions to conflict with libhx509.so
+  if ( $^O ne 'MSWin32' && $Config{ld} =~ /gcc|g\+\+/) {
+      $mycflags .= "-fvisibility=hidden";
+  }
+
   #### remove all lto hacks - https://github.com/DCIT/perl-CryptX/issues/70
   ## #FIX: gcc with -flto is a trouble maker see https://github.com/DCIT/perl-CryptX/issues/32
   ## #FIX: another issue with "-flto=auto" see https://github.com/DCIT/perl-CryptX/pull/66

@atoomic
Copy link
Contributor Author

atoomic commented Dec 31, 2021

@karel-m I can confirm that your patch is working fine, thank you so much for your quick feedback
maybe add a $^O ne 'MSWin32' ?

@atoomic
Copy link
Contributor Author

atoomic commented Dec 31, 2021

We should not merge this change

@atoomic atoomic closed this Dec 31, 2021
GH DCIT#79

When compiling Perl or a binary with different versions
of tomcrypt this is going to have unexpected behaviors
as we cannot guarantee which flavor of the C function is used.

Original Author: @karel-m
@atoomic atoomic reopened this Dec 31, 2021
@atoomic atoomic force-pushed the isolate-ltc-functions branch from bfcd9bd to e28506b Compare December 31, 2021 18:48
@atoomic
Copy link
Contributor Author

atoomic commented Dec 31, 2021

I reopened the PR with your suggested patch after confirming it was working fine

@atoomic
Copy link
Contributor Author

atoomic commented Dec 31, 2021

Thanks again @karel-m for the patch

@karel-m
Copy link
Contributor

karel-m commented Jan 1, 2022

now on CPAN as CryptX-0.075_001

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants