Hi,
I ported libressl to my custom hobby OS and it has been a pleasant
experience. Nonetheless, I did run into some minor portability problems
that I wish to share:
* apps/Makefile.am.tpl links libcrypto and libssl in the wrong order.
The libssl library depends on libcrypto and libcrypto doesn't depend
on libssl. Linking the openssl program as -lcrypto -lssl breaks on
static linking if the main program didn't pull in parts of libcrypto
that libssl needs.
* The POSIX <sys/select.h> header is not used to get the select
function. While <sys/time.h> is required to also provide it (though
that seems transitional to me), there is no requirement <sys/time.h>
also provide the FD_* macros needed to actually use select. This
inclusion needs to be added in apps/ocsp.c, apps/s_client.c,
apps/s_server.c and apps/s_time.c.
* ioctl(FIONBIO) is used in a few files to make sockets non-blocking
rather than using fcntl to set the the standard O_NONBLOCK bit. The
files apps/s_client.c and apps/s_server.c should use BIO_socket_nbio()
instead of directly using BIO_socket_ioctl(). The bio/b_sock.c file
should implement BIO_shock_nbio() using fcntl, F_GETFL/F_SETFL and
O_NONBLOCK.
* include/machine/endian.h wraps the <endian.h> header if Linux and the
<machine/endian.h> header if non-Linux. However, it may be the case
that a non-Linux system (such as mine) has <endian.h> and not
<machine/endian.h>. The POSIX committee has accepted <endian.h> into
the next revision of POSIX. guenther recently implemented this draft
in OpenBSD and the header is already present on many modern Unix
systems. It would be better if the compatibility layer instead
implement <endian.h> rather than <machine/endian.h>.
* The build system defaults --program-transform-name to the host triplet
when cross-compiling. It shouldn't do that as the library doesn't have
a target and is not a cross-compiler (as far as I know). It certainly
doesn't make sense to do when the library itself is being
cross-compiled. This also caused unexpected trouble during the man
page installation as the real man pages got installed under the
transformed name, but the hard links to the man pages wasn't
transformed and broke the build.
* The non-standard err(3) family is used a few places. In the main
library it is only used in the apps/openssl.c file, while it is used a
few times in the regression tests. The convenience of using these
functions is not worth the portability concerns when the calling code
can easily use fprintf and exit instead.
* The configure.ac file doesn't check if explicit_bzero is provided by
the operating system but unconditionally uses the internal one.
I also have a list of minor portability issues I don't mind patching
locally and other stray suggestions:
* crypto/compat/issetugid_linux.c is used on non-Linux platforms. This
fail on including glibc internal headers which is hardly elegant.
Perhaps another file should be used for unknown operating systems that
assumes the worst and throws a #warning? It could also be handled like
getentropy where it fails with linker errors.
* ssl/d1_lib.c includes the non-standard header <sys/param.h>. It worked
when I removed the inclusion, so it seems redundant.
* The non-standard u_char and u_int types are used a few places in the
random compatibility code. It would work just as well if those were
replaced with explicit unsigned char and unsigned int types.
* Consider using _DEFAULT_SOURCE or _ALL_SOURCE as feature macros on
unknown platforms.
* My strerror() (when certain feature macros are defined) return a const
char* as it would be an error to modify the storage. This triggered on
a strerror() invocation in crypto/err/err.c where it stores the result
in a char * rather than a safe const char *.
* There should be an explicit counterpart of memset called
explicit_memset. My libc has such a function, but no explicit_bzero as
I have no regular bzero. If an explicit_memset is added (or detected
in the host system), it could be used in the compatibility
implementation of explicit_bzero (which is currently implemented by
calling memset). I would much prefer if the library uses a safe
zeroing function from the standard library (rather than its own) as
that would give the standard library a chance to be better: I'm quite
interested in using new compiler extensions to robustly ensure memory
is cleared even in the face of link-time optimization and sufficiently
advanced compilers. I don't have absolute confidence that the weak
symbol trick used in explicit_bzero.c is good enough.
* The timingsafe_bcmp function could be implemented using
timingsafe_memcmp for the same reasons.
* The compatibility wrapper headers doesn't check whether the standard
library already provides prototypes. The #include_next mechanism is a
good solution, but duplicate function prototypes would happen if the
standard library happens to provide the extensions. Though, I think
that should be harmless, I don't know how that interacts with
__attribute__'s used in the system headers, if they are implemented as
macros, or any other imaginable situation.
* The <sys/stat.h> and <sys/time.h> inclusions in include/stdlib.h seems
redundant.
* The <sys/types.h> inclusion in string.h seems redundant, size_t is
always provided by <string.h>.
* The header inclusion guard macro in include/machine/endian.h is
_COMPAT_BYTE_ORDER_H_ (in the reserved namespace) while the other
headers use the macro LIBCRYPTOCOMPAT_FOO_H.
* The <stdint.h> inclusion in include/sys/types.h seems like a
non-standard extension. Files relying on <sys/types.h> pulling in
<stdint.h> should be rewritten to include <stdint.h> explicitly.
Below you'll find the reasonably-finished parts of my local patch that
solves some of the above issues.
Thanks,
Jonas 'Sortie' Termansen
diff -Nur libressl-2.0.2/apps/Makefile.am libssl/apps/Makefile.am
--- libressl-2.0.2/apps/Makefile.am 2014-07-16 05:25:52.000000000 +0200
+++ libssl/apps/Makefile.am 2014-07-16 13:26:55.425448073 +0200
@@ -4,8 +4,8 @@
openssl_CFLAGS = $(USER_CFLAGS)
openssl_LDADD = $(PLATFORM_LDADD)
-openssl_LDADD += $(top_builddir)/crypto/libcrypto.la
openssl_LDADD += $(top_builddir)/ssl/libssl.la
+openssl_LDADD += $(top_builddir)/crypto/libcrypto.la
openssl_SOURCES =
noinst_HEADERS =
diff -Nur libressl-2.0.2/apps/ocsp.c libssl/apps/ocsp.c
--- libressl-2.0.2/apps/ocsp.c 2014-07-16 05:25:50.000000000 +0200
+++ libssl/apps/ocsp.c 2014-07-16 13:26:55.429448073 +0200
@@ -57,6 +57,8 @@
*/
#ifndef OPENSSL_NO_OCSP
+#include <sys/select.h>
+
#include <stdio.h>
#include <stdlib.h>
#include <limits.h>
diff -Nur libressl-2.0.2/apps/openssl.c libssl/apps/openssl.c
--- libressl-2.0.2/apps/openssl.c 2014-07-16 05:25:50.000000000 +0200
+++ libssl/apps/openssl.c 2014-07-16 13:26:55.429448073 +0200
@@ -109,7 +109,6 @@
*
*/
-#include <err.h>
#include <signal.h>
#include <stdio.h>
#include <string.h>
@@ -256,8 +255,10 @@
arg.count = 0;
bio_err = BIO_new_fp(stderr, BIO_NOCLOSE);
- if (bio_err == NULL)
- errx(1, "failed to initialise bio_err");
+ if (bio_err == NULL) {
+ fprintf(stderr, "%s: failed to initialize bio_err\n", argv[0]);
+ exit(1);
+ }
CRYPTO_set_locking_callback(lock_dbg_cb);
diff -Nur libressl-2.0.2/apps/s_client.c libssl/apps/s_client.c
--- libressl-2.0.2/apps/s_client.c 2014-07-16 05:25:50.000000000 +0200
+++ libssl/apps/s_client.c 2014-07-16 13:26:55.429448073 +0200
@@ -137,6 +137,7 @@
#include <sys/types.h>
#include <sys/ioctl.h>
+#include <sys/select.h>
#include <sys/socket.h>
#include <netinet/in.h>
@@ -830,9 +831,8 @@
BIO_printf(bio_c_out, "CONNECTED(%08X)\n", s);
if (c_nbio) {
- unsigned long l = 1;
BIO_printf(bio_c_out, "turning on non blocking io\n");
- if (BIO_socket_ioctl(s, FIONBIO, &l) < 0) {
+ if (BIO_socket_nbio(s, 1) < 0) {
ERR_print_errors(bio_err);
goto end;
}
diff -Nur libressl-2.0.2/apps/s_server.c libssl/apps/s_server.c
--- libressl-2.0.2/apps/s_server.c 2014-07-16 05:25:50.000000000 +0200
+++ libssl/apps/s_server.c 2014-07-16 13:26:55.429448073 +0200
@@ -148,6 +148,7 @@
#include <sys/types.h>
#include <sys/ioctl.h>
+#include <sys/select.h>
#include <sys/socket.h>
#include <assert.h>
@@ -1363,11 +1364,9 @@
goto err;
}
if (s_nbio) {
- unsigned long sl = 1;
-
if (!s_quiet)
BIO_printf(bio_err, "turning on non blocking io\n");
- if (BIO_socket_ioctl(s, FIONBIO, &sl) < 0)
+ if (!BIO_socket_nbio(s, 1))
ERR_print_errors(bio_err);
}
@@ -1798,11 +1797,9 @@
goto err;
if (s_nbio) {
- unsigned long sl = 1;
-
if (!s_quiet)
BIO_printf(bio_err, "turning on non blocking io\n");
- if (BIO_socket_ioctl(s, FIONBIO, &sl) < 0)
+ if (!BIO_socket_nbio(s, 1))
ERR_print_errors(bio_err);
}
diff -Nur libressl-2.0.2/apps/s_time.c libssl/apps/s_time.c
--- libressl-2.0.2/apps/s_time.c 2014-07-16 05:25:50.000000000 +0200
+++ libssl/apps/s_time.c 2014-07-16 13:26:55.433448073 +0200
@@ -64,6 +64,7 @@
-----------------------------------------*/
#include <sys/socket.h>
+#include <sys/select.h>
#include <stdio.h>
#include <stdlib.h>
diff -Nur libressl-2.0.2/configure.ac libssl/configure.ac
--- libressl-2.0.2/configure.ac 2014-07-16 05:25:48.000000000 +0200
+++ libssl/configure.ac 2014-07-16 05:25:48.000000000 +0200
@@ -25,7 +25,9 @@
*openbsd*)
AC_DEFINE([HAVE_ATTRIBUTE__BOUNDED__], [1], [OpenBSD gcc has
bounded])
;;
- *) ;;
+ *)
+ CFLAGS="$CFLAGS -D_ALL_SOURCE"
+ ;;
esac
AM_CONDITIONAL(TARGET_DARWIN, test x$TARGET_OS = xdarwin)
diff -Nur libressl-2.0.2/crypto/bio/b_sock.c libssl/crypto/bio/b_sock.c
--- libressl-2.0.2/crypto/bio/b_sock.c 2014-07-16 05:25:49.000000000
+0200
+++ libssl/crypto/bio/b_sock.c 2014-07-16 13:26:55.437448072 +0200
@@ -65,6 +65,7 @@
#include <netinet/tcp.h>
#include <errno.h>
+#include <fcntl.h>
#include <limits.h>
#include <netdb.h>
#include <stdio.h>
@@ -462,5 +463,10 @@
int
BIO_socket_nbio(int s, int mode)
{
- return (BIO_socket_ioctl(s, FIONBIO, &mode) == 0);
+ int flags = fcntl(s, F_GETFD);
+ if ( mode && !(flags & O_NONBLOCK) )
+ return (fcntl(s, F_SETFL, flags | O_NONBLOCK) == 0);
+ else if ( !mode && (flags & O_NONBLOCK) )
+ return (fcntl(s, F_SETFL, flags & ~O_NONBLOCK) == 0);
+ return 1;
}
diff -Nur libressl-2.0.2/crypto/err/err.c libssl/crypto/err/err.c
--- libressl-2.0.2/crypto/err/err.c 2014-07-16 05:25:49.000000000 +0200
+++ libssl/crypto/err/err.c 2014-07-16 13:26:55.437448072 +0200
@@ -596,7 +596,7 @@
if (str->string == NULL) {
char (*dest)[LEN_SYS_STR_REASON] =
&(strerror_tab[i - 1]);
- char *src = strerror(i);
+ const char *src = strerror(i);
if (src != NULL) {
strlcpy(*dest, src, sizeof *dest);
str->string = *dest;
diff -Nur libressl-2.0.2/ssl/d1_lib.c libssl/ssl/d1_lib.c
--- libressl-2.0.2/ssl/d1_lib.c 2014-07-16 05:25:48.000000000 +0200
+++ libssl/ssl/d1_lib.c 2014-07-16 13:26:55.441448071 +0200
@@ -57,7 +57,6 @@
*
*/
-#include <sys/param.h>
#include <sys/socket.h>
#include <netinet/in.h>
