Miscellaneous LibreSSL portability fixes
. 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.0 +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.0 +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.0 +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.0 +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.0 +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.c2014-07-16 05:25:50.0 +0200 +++ libssl/apps/s_time.c2014-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.0 +0200 +++ libssl/configure.ac 2014-07-16 05:25:48.0 +0200 @@ -25,7 +25,9 @@ *openbsd*) AC_DEFINE([HAVE_ATTRIBUTE__BOUNDED__], [1], [OpenBSD gcc has bounded]) ;; - *) ;; + *) + CFLAGS=$CFLAGS
Re: Miscellaneous LibreSSL portability fixes
Thanks for the quick responses and convincing feedback. Mind my goal is not to become a supported platform - at all - but rather to lower the portability barrier where reasonable. I can easily patch around any other concerns locally. Brent: I'll email you the changes that there's agreement on so you can review and integrate them: * Fix link order of libcrypto and libssl (with Jan's improvement). * sys/select.h inclusions for good measure (see below). * O_NONBLOCK instead of FIONBIO. * Use fprintf and exit instead of err() in the core library (only used once). Perhaps the regression tests too. * Check for explicit_bzero in configure.ac so the standard library can provide a good implementation. * The header inclusion and non-standard types patches (for good measure, you said you are already are on the task). * A consistency fix for the guard macro in include/machine/endian.h. * Add a const keyword to a variable in crypto/err/err.c (though that doesn't really matter). I haven't received any response on these and I might not have the autoconf skills to properly deal with them: * --program-transform-name defaulting to the host triplet when cross- compiling (it shouldn't do this). * Man-page hardlinks not honoring --program-transform-name leading to ln errors during make install. * The include/ wrapper headers should not duplicate function prototypes if they are already in the standard library. The rest of my miscellaneous thoughts aren't important and wasn't agreed on, so I'll drop those. Here's some responses: On 07/17/2014 05:05 AM, Philip Guenther wrote: Check again. To quote SUSv4 XBD lines 13376-13381: Sorry. Looks like I misread POSIX. I would still prefer if you also include sys/select.h, but I'll fix my libc so sys/time.h does that. On 07/17/2014 05:05 AM, Philip Guenther wrote: 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. I don't get that impression when looking at the Austin Group bug tracker proposal and discussion about the err(3) functions - but that might change. I personally prefer the GNU error(3) extension as it handles all the {err,warn}{,x} cases by having an error number parameter and not exiting the process on exit code zero. It doesn't seem to have been submitted for standardization though. On 07/17/2014 05:05 AM, Philip Guenther wrote: 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! Good point. I mostly wanted the memset semantics just because it could be a safe counterpart to the traditional memset. I'll adopt the explicit_bzero function and remove explicit_memset. This is possible assuming configure is patched so it checks whether the function is already provided by the standard library. On 07/17/2014 05:02 AM, Brent Cook wrote: What sort of OS is this? It's a Unix-like system written from scratch that implements large parts of POSIX-2008 with useful extensions from GNU and BSD as well as my own. I have the fortune that older and non-standard code break when I port it, which gives me a chance to upstream such issues. There is no need or desire to upstream anything specific to my OS. OpenBSD has interesting extensions that libressl takes advantage of - I'm adding some of those to my project's libc (strl*, reallocarray, timingsafe_memcmp, and getentropy are already done and the rest pending). On 07/17/2014 05:02 AM, Brent Cook wrote: This didn’t cause a const-ripple through the code? Color me surprised! It did and doing so was a mistake (but it didn't trigger for most ports as they don't need the provide-all-extensions feature macro). It doesn't matter - that bullet point was just a suggestion to make a particular variable a const string pointer. Jonas
uuid encode breakage
Hi, I was looking at the recently imported uuid interface in libc and noticed the encode functions are broken. In particular, this pattern: uint8_t *p = buf; p[0] = htobe32(uuid-time_low); p[4] = htobe16(uuid-time_mid); p[6] = htobe16(uuid-time_hi_and_version); The intention is obviously that the 32-bit and 16-bit values gets copied to the buffer, but instead the value gets truncated. The resulting encoded uuid will then contain unmodified bytes (likely uninitialized). This appears to be the case in the kernel uuid code as well. The FreeBSD and NetBSD counterparts for this code is instead using be32enc and be16enc macros to put the bytes into the buffer and doesn't suffer from this truncation issue. Grepping through the tree reveals these encode functions are fortunately not currently used. The decode functions are used though, but they don't have this problem. Jonas
Replace LibreSSL times() call
Hi, I noticed libressl's apps.c is using times(3), which is among the functions I am aggressively deprecating in my personal system. This patch switches it to use the clock_gettime and getrusage instead. I pondered using CLOCK_VIRTUAL rather than getrusage, but it turned out to be not be implemented and not portable. Unfortunately, OS X doesn't have clock_gettime, so the portable version will have to add back a times call as a fallback, or perhaps use gettimeofday (but this doesn't have the proper time-doesn't-go-backwards semantics). I didn't use the useful, but non-standard timespecsub and TIMEVAL_TO_TIMESPEC macros from sys/time.h to make things easier for the portable version. Alternatively they could be used and a fallback implementation can be added to the libressl time.h wrapper header. Jonas --- libressl-2.1.0/apps/apps.c 2014-10-11 18:58:11.0 +0200 +++ libssl/apps/apps.c 2014-10-14 21:31:44.827167386 +0200 @@ -126,7 +126,6 @@ #include sys/types.h #include sys/stat.h -#include sys/times.h #include ctype.h #include errno.h @@ -135,6 +134,7 @@ #include limits.h #include string.h #include strings.h +#include time.h #include unistd.h #include apps.h @@ -2203,25 +2203,40 @@ #endif /* !OPENSSL_NO_TLSEXT !OPENSSL_NO_NEXTPROTONEG */ +static struct timespec ts_elapsed(struct timespec a, struct timespec b) +{ + a.tv_sec -= b.tv_sec; + a.tv_nsec -= b.tv_nsec; + if ( a.tv_nsec 0 ) + { + a.tv_nsec += 10L; + a.tv_sec -= 1; + } + return a; +} + double app_tminterval(int stop, int usertime) { - double ret = 0; - struct tms rus; - clock_t now = times(rus); - static clock_t tmstart; + static struct timespec start_ts; + struct timespec now_ts; - if (usertime) - now = rus.tms_utime; + if (usertime) { + struct rusage ru; + getrusage(RUSAGE_SELF, ru); + now_ts.tv_sec = ru.ru_utime.tv_sec; + now_ts.tv_nsec = ru.ru_utime.tv_usec * 1000L; + } else { + clock_gettime(CLOCK_MONOTONIC, now_ts); + } - if (stop == TM_START) - tmstart = now; - else { - long int tck = sysconf(_SC_CLK_TCK); - ret = (now - tmstart) / (double) tck; + if (stop == TM_START) { + start_ts = now_ts; + return 0.0; } - return (ret); + struct timespec elapsed_ts = ts_elapsed(now_ts, start_ts); + return (double) elapsed_ts.tv_sec + (double) elapsed_ts.tv_nsec / 1E9; } int
app_tminterval callers use redundant constants
Hi, The app_tminterval utility function in apps.c has a first parameter that is either TM_START or TM_STOP as defined in apps.h. The two files that use this function disregard those constants and define their own along with their own wrapper utility wrapper function. This patch keeps the wrapper functions as they provide a little convenience, but removes the duplicate constants and fixes calls to use the apps.h constants. There also appears to be some left over compatibility for platforms without timens. Since the libressl build system doesn't define TIMES, this code thinks the feature isn't available doesn't when reporting usage. Jonas diff -Nur libressl-2.1.0/apps/speed.c libssl/apps/speed.c --- libressl-2.1.0/apps/speed.c 2014-10-11 18:58:11.0 +0200 +++ libssl/apps/speed.c 2014-10-14 17:45:02.778749560 +0200 @@ -203,9 +203,6 @@ run = 0; } -#define START 0 -#define STOP 1 - static double Time_F(int s) @@ -481,10 +478,6 @@ int multi = 0; const char *errstr = NULL; -#ifndef TIMES - usertime = -1; -#endif - memset(results, 0, sizeof(results)); memset(dsa_key, 0, sizeof(dsa_key)); for (i = 0; i EC_NUM; i++) @@ -959,9 +952,7 @@ BIO_printf(bio_err, \n); BIO_printf(bio_err, Available options:\n); -#if defined(TIMES) || defined(USE_TOD) BIO_printf(bio_err, -elapsedmeasure time in real time instead of CPU user time.\n); -#endif #ifndef OPENSSL_NO_ENGINE BIO_printf(bio_err, -engine e use engine e, possibly a hardware device.\n); #endif @@ -1066,10 +1057,10 @@ if (doit[D_MDC2]) { for (j = 0; j SIZE_NUM; j++) { print_message(names[D_MDC2], c[D_MDC2][j], lengths[j]); - Time_F(START); + Time_F(TM_START); for (count = 0, run = 1; COND(c[D_MDC2][j]); count++) EVP_Digest(buf, (unsigned long) lengths[j], (mdc2[0]), NULL, EVP_mdc2(), NULL); - d = Time_F(STOP); + d = Time_F(TM_STOP); print_result(D_MDC2, j, count, d); } } @@ -1079,10 +1070,10 @@ if (doit[D_MD4]) { for (j = 0; j SIZE_NUM; j++) { print_message(names[D_MD4], c[D_MD4][j], lengths[j]); - Time_F(START); + Time_F(TM_START); for (count = 0, run = 1; COND(c[D_MD4][j]); count++) EVP_Digest((buf[0]), (unsigned long) lengths[j], (md4[0]), NULL, EVP_md4(), NULL); - d = Time_F(STOP); + d = Time_F(TM_STOP); print_result(D_MD4, j, count, d); } } @@ -1092,10 +1083,10 @@ if (doit[D_MD5]) { for (j = 0; j SIZE_NUM; j++) { print_message(names[D_MD5], c[D_MD5][j], lengths[j]); - Time_F(START); + Time_F(TM_START); for (count = 0, run = 1; COND(c[D_MD5][j]); count++) EVP_Digest((buf[0]), (unsigned long) lengths[j], (md5[0]), NULL, EVP_get_digestbyname(md5), NULL); - d = Time_F(STOP); + d = Time_F(TM_STOP); print_result(D_MD5, j, count, d); } } @@ -,13 +1102,13 @@ for (j = 0; j SIZE_NUM; j++) { print_message(names[D_HMAC], c[D_HMAC][j], lengths[j]); - Time_F(START); + Time_F(TM_START); for (count = 0, run = 1; COND(c[D_HMAC][j]); count++) { HMAC_Init_ex(hctx, NULL, 0, NULL, NULL); HMAC_Update(hctx, buf, lengths[j]); HMAC_Final(hctx, (hmac[0]), NULL); } - d = Time_F(STOP); + d = Time_F(TM_STOP); print_result(D_HMAC, j, count, d); } HMAC_CTX_cleanup(hctx); @@ -1127,10 +1118,10 @@ if (doit[D_SHA1]) { for (j = 0; j SIZE_NUM; j++) { print_message(names[D_SHA1], c[D_SHA1][j], lengths[j]); - Time_F(START); + Time_F(TM_START); for (count = 0, run = 1; COND(c[D_SHA1][j]); count++) EVP_Digest(buf, (unsigned long) lengths[j], (sha[0]), NULL, EVP_sha1(), NULL); - d = Time_F(STOP); + d = Time_F(TM_STOP); print_result(D_SHA1, j, count, d); } } @@ -1138,10 +1129,10 @@ if (doit[D_SHA256]) { for (j = 0;
CLOCK_VIRTUAL
Hi, I noticed clock_gettime(2) describes a CLOCK_VIRTUAL clock that measures how long a process has run in user-space. However, it is not implemented in sys/kern/kern_time.c where it fails with EINVAL in the default switch case. It does seem to be implemented in FreeBSD and NetBSD along with a CLOCK_PROF counterpart. It is not part of POSIX, but a quick online search suggests it might have been part of ancient POSIX revisions (that I don't have access to). I see these options: 1) Remove CLOCK_VIRTUAL fully as it doesn't work and is non-standard. This might break some ports but that code wasn't working in the first place. 2) Don't mention it at all in clock_gettime(2) and keep it for compatibility purposes. 3) Document it is unimplemented in the BUGS section. 4) Implement it. The kernel already has the information to implement getrusage(2) and the CLOCK_PROCESS_CPUTIME_ID case is similar. If it remains documented, it should also be marked as an extension under STANDARDS. Additionally, perhaps further clocks should be added to get the individual user-time and system-time components of CLOCK_PROCESS_CPUTIME_ID and CLOCK_THREAD_CPUTIME_ID? These can be determined in the process case with getrusage(2), but I know of no way in the thread case. Jonas
crypt_newhash
Hi, The flak blog just had an interesting post about why the old crypt() interface should be replaced, and on the new crypt_newhash() and crypt_checkpass() that were added to OpenBSD. I would like to see this API become portable and perhaps standardized, but crypt_newhash is currently tied to login_cap_t, which is not a portable abstraction. The current synopsis is: #include login_cap.h int crypt_newhash(const char *pass, login_cap_t *lc, char *hash, size_t hashlen); The purpose of the lc parameter is to determine which algorithm to use: The implementation merely does a login_getcapstr(lc, localcipher, NULL, NULL) call to convert that into a string telling what algorithm to use. If lc is NULL, then it defaults to a reasonable algorithm. It would be superior to move the login_getcapstr call to the caller and instead have a string parameter. This removes the association with login_cap and it can be moved to unistd.h or pwd.h alongside the other functions. It also becomes more like crypt (where various algorithms can be requested) and thus more reusable in other situations than local-user authentication (like a web-server). Jonas
gomoku debug regression
Hi, I noticed revision 1.25 of games/gomoku/main.c was faulty: if (!debug) -#ifdef SVR4 - srand(time(0)); -#endif if (interactive) cursinit(); /* initialize curses */ The if (!debug) line should've been removed as well. Now curses isn't initialized in debug mode. Jonas
Re: gomoku debug regression
I just realized SVR4 wasn't defined in the first place, so this isn't a regression. It was always broken.
Re: Violating randomization standards
On 12/08/2014 09:55 PM, Theo de Raadt wrote: Index: lib/libc/stdlib/mrand48.c === RCS file: /cvs/src/lib/libc/stdlib/mrand48.c,v retrieving revision 1.3 diff -u -p -u -r1.3 mrand48.c --- lib/libc/stdlib/mrand48.c 8 Aug 2005 08:05:37 - 1.3 +++ lib/libc/stdlib/mrand48.c 8 Dec 2014 03:13:07 - @@ -19,6 +19,8 @@ extern unsigned short __rand48_seed[3]; long mrand48(void) { + if (__rand48_deterministic == 0) + return arc4random(); __dorand48(__rand48_seed); return ((long) __rand48_seed[2] 16) + (long) __rand48_seed[1]; } POSIX says mrand48 is meant to return signed integers in the interval [-2^31,2^31), but this code returns an unsigned 32-bit integer value.
Re: Violating randomization standards
On 12/09/2014 12:06 AM, Todd C. Miller wrote: Which gets cast to a signed integer in that same range... Except long is larger than 32 bits on 64-bit platforms.
Re: [PATCH] update zlib to 1.2.8
Hi, There's some value in having a newer zlib release. There have been bugfixes, documentation fixes, various improvements, and API additions. I'm not sure how much is backported to the OpenBSD zlib, but a quick glance through your attached patch suggests there's stuff that would be nice to have. Do note your patch does revert some OpenBSD-local changes to zlib. You should diff the older zlib release with the OpenBSD zlib to see what they are and try to preserve them when rebasing on the latest zlib release. But if you read the patch to 1.2.8 carefully, you would also notice a good deal of added portability code. Nothing too exciting has happened in the zlib world the past decade, except the addition of more portability to support even more awful systems. It's basically the OpenSSL approach to portability gone worse. Not all of it is worth having, some of it is undesirable. I think it's a better idea to go through the patch manually and pick out the desirable changes and apply them. Or, as Joerg suggests, just patch the port to be compatible with older zlib releases if possible. Recently I got fed up with the awful code quality of upsteam zlib, so I fetched 1.2.8 and removed support for awful systems line by line, converting it to nice standard C, not zlib-C. The result was a great deal simpler and the process revealed a great deal of horrors. You just need to open a random file to find them. Jonas
Re: smtpd domain append fix
On 01/06/2015 12:11 PM, Gilles Chehade wrote: Index: smtp_session.c === RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v retrieving revision 1.221 diff -u -p -r1.221 smtp_session.c --- smtp_session.c17 Dec 2014 15:49:23 - 1.221 +++ smtp_session.c5 Jan 2015 22:36:55 - @@ -315,7 +315,9 @@ header_append_domain_buffer(char *buffer has_domain = 1; if (buffer[i] == ':' !escape !comment !quote) has_group = 1; - if (! isspace(buffer[i])) + + /* update insert point if not in comment and not on a whitespace */ + if (!comment buffer[i] != ')' !isspace((int)buffer[i])) This isspace call looks wrong, and looking at the source, so does nearby isspace calls. The argument to isspace() must be EOF or representable as an unsigned char; otherwise, the result is undefined. However, char is signed on some platforms, and buffer is a char pointer here, meaning out-of-range values might be passed. Casting to an int just sign extends the potential negative values, rather than mapping them to the high unsigned char values. The callers should be changed to the pattern isspace((unsigned char)buffer[i]) instead. pos_component = i; }
Re: pthread question
On 10/07/2015 12:53 PM, Stuart Henderson wrote: > if (pthread_kill(stat_thread, 0)) { pthread_kill sends the specified signal to the thread, but signal 0 just checks whether a signal can be sent and sends no signal.
ocspcheck size_t printing
Hi, When upgrading to libressl-2.5.4 I noticed a couple -Wformat errors due to this code assuming size_t is of type long when it was actually int on this 32-bit system. Here's a patch against cvs that fixes the issue and also prints the variableas unsigned type. Jonas Index: ocspcheck.c === RCS file: /cvs/src/usr.sbin/ocspcheck/ocspcheck.c,v retrieving revision 1.20 diff -u -r1.20 ocspcheck.c --- ocspcheck.c 27 Mar 2017 23:59:08 - 1.20 +++ ocspcheck.c 8 May 2017 18:15:51 - @@ -564,7 +564,7 @@ if ((request = ocsp_request_new_from_cert(certfile, nonce)) == NULL) exit(1); - dspew("Built an %ld byte ocsp request\n", request->size); + dspew("Built an %zu byte ocsp request\n", request->size); if ((host = url2host(request->url, , )) == NULL) errx(1, "Invalid OCSP url %s from %s", request->url, @@ -601,7 +601,7 @@ dspew("Server at %s returns:\n", host); for (i = 0; i < httphsz; i++) dspew(" [%s]=[%s]\n", httph[i].key, httph[i].val); - dspew(" [Body]=[%ld bytes]\n", hget->bodypartsz); + dspew(" [Body]=[%zu bytes]\n", hget->bodypartsz); if (hget->bodypartsz <= 0) errx(1, "No body in reply from %s", host);