On Thu, 17 Jul 2014, Jonas 'Sortie' Termansen wrote:
> 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:

To respond to selected items...


> * 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.

There are other OSes still using static libs?!?  :-)


> * 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.

Check again.  To quote SUSv4 XBD lines 13376-13381:
   The <sys/time.h> header shall define the following as described in 
   <sys/select.h>:

   FD_CLR()
   FD_ISSET()
   FD_SET()
   FD_ZERO()
   FD_SETSIZE


> * 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.

Agreed.


> * 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>.

This is already on someone's list...


> * 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.

As you may know, there's traction in getting these into a future version 
of POSIX, so get on the bandwagon now while it's still edgy.  ;-)



> * Consider using _DEFAULT_SOURCE or _ALL_SOURCE as feature macros on
>   unknown platforms.

This seems unwise to me, as they might just as well disable access to 
functions that aren't standard yet, like getentropy(), explicit_bzero(), 
etc.


> * 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 *.

While err.c should probably be changed, changing the prototype of a 
standard function deserves all the self-inflicted pain it gets.


> * 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.

explicit_memset()?  Can you describe a reason to use it with a value other 
than zero that wouldn't also work with memset()?  If the whole point is 
that the buffer is unobserved by the flow and there the call needs 
protection from being optimized away, then the contents of the buffer 
cannot matter!


> * The timingsafe_bcmp function could be implemented using
>   timingsafe_memcmp for the same reasons.

Having compared the source for those two functions, I disagree.



> * 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.

Nice catch.


Philip Guenther

Reply via email to