libressl / portable

LibreSSL Portable itself. This includes the build scaffold and compatibility layer that builds portable LibreSSL from the OpenBSD source code. Pull requests or patches sent to tech@openbsd.org are welcome.
https://www.libressl.org
1.36k stars 267 forks source link

Issues with 'make check', --disable-static and slibtool #754

Closed orbea closed 1 year ago

orbea commented 2 years ago

OS gentoo libressl: https://github.com/libressl-portable/portable/commit/5c7021517a6d7675276d3715f44ebef9f09ca0a7

When doing make check with --disable-static it will fail because of the following lines.

https://github.com/libressl-portable/portable/blob/5c7021517a6d7675276d3715f44ebef9f09ca0a7/tests/Makefile.am#L15-L17

I fixed it with this patch.

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6c88c90..c63ee41 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -12,9 +12,9 @@ AM_CPPFLAGS += -I $(top_srcdir)/apps/openssl
 AM_CPPFLAGS += -I $(top_srcdir)/apps/openssl/compat
 AM_CPPFLAGS += -D_PATH_SSL_CA_FILE=\"$(top_srcdir)/cert.pem\"

-LDADD = $(abs_top_builddir)/tls/.libs/libtls.a
-LDADD += $(abs_top_builddir)/ssl/.libs/libssl.a
-LDADD += $(abs_top_builddir)/crypto/.libs/libcrypto.a
+LDADD = $(abs_top_builddir)/tls/libtls.la
+LDADD += $(abs_top_builddir)/ssl/libssl.la
+LDADD += $(abs_top_builddir)/crypto/libcrypto.la
 LDADD += $(PLATFORM_LDADD) $(PROG_LDADD)
 if HOST_ASM_MACOSX_X86_64
 LDADD += $(abs_top_builddir)/crypto/.libs/libcrypto_la-cpuid-macosx-x86_64.o

However this exposes a new issue for GNU libtool only.

The problem is many of the tests fail with undefined references which are missing from the Symbols.list files from the libcrypto, libssl and libtls directories in the openbsd repo where adding all of them is no trivial task.

For example here is an incomplete patch where I started that.

diff --git a/src/lib/libcrypto/Symbols.list b/src/lib/libcrypto/Symbols.list
index 3881d2775..0c037a1dd 100644
--- a/src/lib/libcrypto/Symbols.list
+++ b/src/lib/libcrypto/Symbols.list
@@ -446,6 +446,7 @@ BN_bn2dec
 BN_bn2hex
 BN_bn2lebinpad
 BN_bn2mpi
+BN_bntest_rand
 BN_clear
 BN_clear_bit
 BN_clear_free
@@ -500,9 +501,13 @@ BN_mod_add
 BN_mod_add_quick
 BN_mod_exp
 BN_mod_exp2_mont
+BN_mod_exp_ct
 BN_mod_exp_mont
 BN_mod_exp_mont_consttime
+BN_mod_exp_mont_ct
+BN_mod_exp_mont_nonct
 BN_mod_exp_mont_word
+BN_mod_exp_nonct
 BN_mod_exp_recp
 BN_mod_exp_simple
 BN_mod_inverse
@@ -536,6 +541,7 @@ BN_print_fp
 BN_pseudo_rand
 BN_pseudo_rand_range
 BN_rand
+bn_rand_interval
 BN_rand_range
 BN_reciprocal
 BN_rshift
@@ -552,6 +558,7 @@ BN_swap
 BN_to_ASN1_ENUMERATED
 BN_to_ASN1_INTEGER
 BN_to_montgomery
+bn_to_string
 BN_uadd
 BN_ucmp
 BN_usub
@@ -575,7 +582,54 @@ CAST_ecb_encrypt
 CAST_encrypt
 CAST_ofb64_encrypt
 CAST_set_key
+CBB_add_asn1
+CBB_add_asn1_uint64
+CBB_add_bytes
+CBB_add_space
+CBB_add_u8
+CBB_add_u16
+CBB_add_u24
+CBB_add_u32
+CBB_add_u64
+CBB_add_u8_length_prefixed
+CBB_add_u16_length_prefixed
+CBB_add_u24_length_prefixed
+CBB_cleanup
+CBS_data
+CBB_discard_child
+CBS_dup
+CBB_finish
+CBB_init
+CBB_init_fixed
 CBIGNUM_it
+CBS_asn1_indefinite_to_definite
+CBS_get_asn1
+CBS_get_asn1_uint64
+CBS_get_last_u8
+CBS_get_optional_asn1
+CBS_get_optional_asn1_bool
+CBS_get_optional_asn1_octet_string
+CBS_get_optional_asn1_uint64
+CBS_get_u8
+CBS_get_u16
+CBS_get_u24
+CBS_get_u32
+CBS_get_u64
+CBS_get_u8_length_prefixed
+CBS_get_u16_length_prefixed
+CBS_get_u24_length_prefixed
+CBS_init
+CBS_len
+CBS_mem_equal
+CBS_offset
+CBS_peek_asn1_tag
+CBS_peek_last_u8
+CBS_peek_u8
+CBS_peek_u16
+CBS_peek_u24
+CBS_peek_u32
+CBS_skip
+CBS_write_bytes
 CERTIFICATEPOLICIES_free
 CERTIFICATEPOLICIES_it
 CERTIFICATEPOLICIES_new
@@ -1194,6 +1248,7 @@ EC_POINT_copy
 EC_POINT_dbl
 EC_POINT_dup
 EC_POINT_free
+EC_POINT_get_Jprojective_coordinates
 EC_POINT_get_Jprojective_coordinates_GFp
 EC_POINT_get_affine_coordinates
 EC_POINT_get_affine_coordinates_GF2m
@@ -3336,6 +3391,13 @@ X509_check_trust
 X509_cmp
 X509_cmp_current_time
 X509_cmp_time
+x509_constraints_domain
+x509_constraints_parse_mailbox
+x509_constraints_valid_domain_constraint
+x509_constraints_valid_host
+x509_constraints_valid_sandns
+x509_constraints_uri
+x509_constraints_uri_host
 X509_delete_ext
 X509_digest
 X509_dup
@@ -3512,6 +3574,7 @@ d2i_ASIdentifierChoice
 d2i_ASIdentifiers
 d2i_ASN1_BIT_STRING
 d2i_ASN1_BMPSTRING
+d2i_ASN1_BOOLEAN
 d2i_ASN1_ENUMERATED
 d2i_ASN1_GENERALIZEDTIME
 d2i_ASN1_GENERALSTRING
@@ -3725,6 +3788,7 @@ i2d_ASIdentifierChoice
 i2d_ASIdentifiers
 i2d_ASN1_BIT_STRING
 i2d_ASN1_BMPSTRING
+i2d_ASN1_BOOLEAN
 i2d_ASN1_ENUMERATED
 i2d_ASN1_GENERALIZEDTIME
 i2d_ASN1_GENERALSTRING
diff --git a/src/lib/libssl/Symbols.list b/src/lib/libssl/Symbols.list
index 3b1063592..695f6d000 100644
--- a/src/lib/libssl/Symbols.list
+++ b/src/lib/libssl/Symbols.list
@@ -43,6 +43,54 @@ i2d_SSL_SESSION
 /* setup */
 ERR_load_SSL_strings

+/* tests */
+handshakes
+handshake_count
+pitem_free
+pitem_new
+pqueue_free
+pqueue_find
+pqueue_insert
+pqueue_iterator
+pqueue_new
+pqueue_next
+pqueue_pop
+ssl_bytes_to_cipher_list
+ssl_cipher_list_to_bytes
+ssl_enabled_tls_version_range
+ssl_max_shared_version
+ssl_parse_ciphersuites
+ssl3_get_cipher
+ssl3_num_ciphers
+tls_buffer_cbs
+tls_buffer_extend
+tls_buffer_finish
+tls_buffer_free
+tls_buffer_new
+tls_legacy_method
+tls12_record_layer_free
+tls12_record_layer_inc_seq_num
+tls12_record_layer_new
+tls12_record_layer_set_initial_epoch
+tls12_record_layer_set_version
+tls13_derive_application_secrets
+tls13_derive_early_secrets
+tls13_derive_handshake_secrets
+tls13_record_content
+tls13_record_content_type
+tls13_record_data
+tls13_record_free
+tls13_record_header
+tls13_record_layer_inc_seq_num
+tls13_record_new
+tls13_record_recv
+tls13_record_send
+tls13_record_set_data
+tls13_secrets_create
+tls13_secrets_destroy
+tls13_update_client_traffic_secret
+tls13_update_server_traffic_secret
+
 /* general API */
 SSL_CIPHER_description
 SSL_CIPHER_find
diff --git a/src/lib/libtls/Symbols.list b/src/lib/libtls/Symbols.list
index 42c039d29..ecdc77c2e 100644
--- a/src/lib/libtls/Symbols.list
+++ b/src/lib/libtls/Symbols.list
@@ -66,6 +66,16 @@ tls_error
 tls_free
 tls_handshake
 tls_init
+tls_keypair_clear_key
+tls_keypair_free
+tls_keypair_load_cert
+tls_keypair_new
+tls_keypair_set_cert_file
+tls_keypair_set_cert_mem
+tls_keypair_set_key_file
+tls_keypair_set_key_mem
+tls_keypair_set_ocsp_staple_file
+tls_keypair_set_ocsp_staple_mem
 tls_load_file
 tls_ocsp_process_response
 tls_peer_cert_chain_pem

Slibtool entirely avoids this issue and correctly passes all the tests by not passing the -version-script and libcrypto.ver, libssl.ver and libtls.ver files to the compiler. I am not yet sure the reason why slibtool does this, but perhaps getting rid of these files altogether is the simple solution?

kinichiro commented 2 years ago

Hello, For testing some regresses call internal functions those are not exported from shared libraries. To cope with this, regresses are linked with static libraries that does not have export functionality. If Makefile.am indicates linking with .la files, regresses are linked with shared libraries. That is why Makefile.am is written to link with .a under the .libs/ dirs.

The internal functions are not public API and explicitly hidden (not exported), and those should not be described in Symbols.list.

I think building static libraries are required to run regresses.

orbea commented 2 years ago

As I was making the patch I realized why they were not exported as you described and stopped before completing it for that reason. However it is common practice to not build or install static libraries in many linux distros and if possible I would really appreciate if make check worked with --disable-static.

orbea commented 2 years ago

The difference with slibtool is that -export-symbols is a no-op in slibtool, this should be fixed there.

The issue with --disable-static could be fixed with PRs https://github.com/libressl-portable/openbsd/pull/132 and https://github.com/libressl-portable/portable/pull/765.

orbea commented 8 months ago

The difference with slibtool is that -export-symbols is a no-op in slibtool, this should be fixed there.

This is fixed in the slibtool main branch which now supports both -export-symbols and -export-symbols-regex.