segfault when printing SSL errors

[jmuchemb]

After some debugging, I could find that the crypto_print_openssl_errors function is wrongly compiled: only the lower 32 bits of the value returned by ERR_error_string are passed to x_msg.

   0x000000000040dabf <+79>:    callq  0x405be0 <ERR_error_string@plt>
   0x000000000040dac4 <+84>:    mov    $0x46acfc,%esi
   0x000000000040dac9 <+89>:    mov    %eax,%edx
   0x000000000040dacb <+91>:    mov    %r13d,%edi
   0x000000000040dace <+94>:    xor    %eax,%eax
   0x000000000040dad0 <+96>:    callq  0x40f250 <x_msg>

(the 3rd instruction should be mov %rax,%rdx)
Compare:

(gdb) x/s 0x7f6ff8a1d5c0
0x7f6ff8a1d5c0 <buf.5304>:      "error:14090086:SSL routines:ssl3_get_server_certificate:certificate verify failed"

with:

[9202436.034396] openvpn[28834]: segfault at f8a1d5c0 ip 00007f6ff8067dcc sp 00007ffe6b016b50 error 4 in libc-2.19.so[7f6ff801d000+1a1000]

This is a regression in 2.3.11 and indeed we never had such issues before.

OS: Debian Jessie (gcc 4.9.2)

Here is the full traceback for reference:

#0  0x00007f6ff8067dcc in _IO_vfprintf_internal (s=s@entry=0x7ffe6b017130, format=<optimized out>, format@entry=0x46acfc "OpenSSL: %s", ap=ap@entry=0x7ffe6b0172f8) at vfprintf.c:1642
#1  0x00007f6ff808f409 in _IO_vsnprintf (string=0x211a1e8 "OpenSSL: \241\021\002", maxlen=<optimized out>, format=0x46acfc "OpenSSL: %s", args=0x7ffe6b0172f8) at vsnprintf.c:119
#2  0x000000000040efb2 in x_msg_va (flags=50331681, format=0x46acfc "OpenSSL: %s", arglist=0x7ffe6b0172f8) at error.c:240
#3  0x000000000040f2d7 in x_msg (flags=flags@entry=50331681, format=format@entry=0x46acfc "OpenSSL: %s") at error.c:199
#4  0x000000000040dad5 in crypto_print_openssl_errors (flags=flags@entry=50331681) at crypto_openssl.c:240
#5  0x0000000000460cbb in bio_read (bio=0x20f38d0, buf=0x20f8500, maxlen=<optimized out>, desc=0x488bad "tls_read_plaintext") at ssl_openssl.c:1190
#6  0x000000000045be5a in tls_process (wakeup=<optimized out>, to_link_socket_info=<optimized out>, to_link_addr=<optimized out>, to_link=<optimized out>, session=<optimized out>, multi=<optimized out>) at ssl.c:2444
#7  tls_multi_process (multi=0x20f80d0, to_link=0x46acfc, to_link@entry=0x7ffe6b0180d0, to_link_addr=0x18, to_link_addr@entry=0x7ffe6b017e00, to_link_socket_info=0xffffffffffffffff, wakeup=0xf8a1d5c0, wakeup@entry=0x7ffe6b0175bc) at ssl.c:2668
#8  0x0000000000410ef8 in check_tls_dowork (c=c@entry=0x7ffe6b0175f0) at forward.c:98
#9  0x0000000000413b4e in check_tls (c=0x7ffe6b0175f0) at forward-inline.h:41
#10 pre_select (c=0x7ffe6b0175f0) at forward.c:1403
#11 0x00000000004315f2 in tunnel_point_to_point (c=<optimized out>) at openvpn.c:80
#12 openvpn_main (argc=40, argv=0x7ffe6b018398) at openvpn.c:270
#13 0x00007f6ff803eb45 in __libc_start_main (main=0x407790 <main>, argc=40, argv=0x7ffe6b018398, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffe6b018388) at libc-start.c:287
#14 0x00000000004077be in _start ()

[jmuchemb]

In fact, we also recompile openssl. With crypto, but with typos in the configure command, so HAVE_OPENSSL_ENGINE ends up being unset. In this case, there's no header declaring ERR_error_string and the returned pointer is treated as an int.

I submitted a patch to openvpn-devel@…, to add missing header files (that are implicitly included when HAVE_OPENSSL_ENGINE is set).

[Gert Döring]

Sorry, we overlooked this. Merged today.

commit ab1302d47b0f2b2ac9272b9fbb90ef0bd7b7cc35
Author: Julien Muchembled
Date: Fri Dec 16 17:32:18 2016 +0100

Fix implicit declarations when HAVE_OPENSSL_ENGINE is unset

will be part of 2.3.15 whenever that is released (no time line, but I expect us to do a few more 2.3.x maintenance releases)