On 14/04/14 21:34, Gert Doering wrote: > Hi, > > On Sun, Apr 13, 2014 at 05:26:13PM +0200, Gert Doering wrote: >> OpenVPN does not currently report the version of the SSL library it is >> using - which I'm not sure whether it's by design or just because nobody >> ever added it. Anyway, right now I think we need it, to help future >> cases. > > OK, here's a "continue the discussion" patch. > > This is actually surprisingly messy, as the "first line" OpenVPN prints > is a static string that is referenced all over the place, sometimes sent > to file descriptors, sometimes used with msg(), so extending this to > include the "library version numbers" would be a fairly big change. So > I've decided to just print a second line:
Makes sense!
>
> If --push-peer-info is set (and only then), this is sent as part of the
> peer-info handshake towards the server:
>
> Apr 14 21:23:49 gentoo openvpn[17804]: 2001:608:4:0:222:68ff:fe7f:7420 peer
> info: IV_SSL=OpenSSL_1.0.1g_7_Apr_2014
Looks good.
Regarding your patch - the PolarSSL stuff
+char *
+get_ssl_library_version(void)
+{
+ static char polar_version[30]; /* "at least 18 bytes in size" */
+ version_get_string_full( polar_version );
+ return polar_version;
+}
version_get_string_full() is crappy due to the "at least 18 bytes".
That's ugly. Can we please ensure that we at least adds a NULL
termination in polar_version[29] after version_get_string_full() ...
just to reduce the risk somewhat, in case it suddenly gets longer. Then
it should (hopefully) be somewhat harder to get a buffer information leak.
Or maybe even slightly more hardened. Put version_get_string_full()
into a much bigger temporary buffer, which then uses a strncpy() or
memcpy() into the polar_version[] buffer. The strncpy()/memcpy() should
restrict it to 30 bytes, including NULL terminator.
I presume get_ssl_library_version() will be treated as a normal NULL
terminated string elsewhere in the code, also later on.
--
kind regards,
David Sommerseth
signature.asc
Description: OpenPGP digital signature
