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>

Reply via email to