0
0
mirror of https://github.com/OpenVPN/openvpn.git synced 2024-09-20 12:02:28 +02:00
Commit Graph

516 Commits

Author SHA1 Message Date
Steffan Karger
dd2fbc26eb PolarSSL x509_get_sha1_hash now returns correct SHA1 fingerprint.
509_get_sha1_hash() is supposed to return the certificate fingerprint,
which is the hash of the entire certificate - including the signature -
and not just the 'to be signed' data (cert->tbs in polarssl).

This changes externally visible behavior for polarssl builds: it will
change the value of the tls_digest_N values exported to the environment
for scripts.

v2 Steffan Karger: added commit message and Changes.rst entry.
                   Code unchanged from v1 by James.

Signed-off-by: James Yonan <james@openvpn.net>
Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <CAA1AbxL=QYUy6N+jKgxVVuftmF=75mSEz3rYUbisT245UfB5Dg@mail.gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11396
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-28 13:52:24 +02:00
Lev Stipakov
4e37af92f5 Fix "implicit declaration" compiler warning
Add missing "include" directive.

Signed-off-by: Lev Stipakov <lstipakov@gmail.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1447246849-11602-1-git-send-email-lstipakov@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10485
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-27 20:17:15 +02:00
Daniel Kubec
13a882ae39 Fix buffer size parameter for exported keying material.
Commit 41e4b67a22 broke the exported
keying material functionality while addressing lack of variable-length
arrays in MSVC compilers - turning an array into a gc_malloc()'ed
pointer, but still using "sizeof(ekm)" for buffer size - which is
now "4" (unsigned char *), not the actual buffer length...

Fixed!

Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <49496.109.81.184.65.1461736834.squirrel@mail.actumg2.cz>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11509

Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-27 19:53:00 +02:00
Jens Neuhalfen
7c0ecd1191 Fix buffer overflow by user supplied data
Passing very long usernames/passwords for pam authentication could
possibly lead to a stack based buffer overrun in the auth-pam plugin.

Adds a dependency to C99 (includes stdbool.h)

Signed-off-by: Jens Neuhalfen <jens@neuhalfen.name>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <A4F03DE4-3E70-4815-B4B4-CC185E35CF2C@neuhalfen.name>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11477
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-21 19:44:29 +02:00
Selva Nair
9b0f1df256 Support reading the challenge-response from console
Trying to keep the footrpint small, this patch adds to the
convoluted code-flow in get_user_pass_cr(). Cleanup left for later.
-----8<-----

Currently prompting for a response to static-challenge
gets skipped when the username and passowrd are read
from a file. Further, dynamic challenge gets wrongly handled
as if its a username/password request.

The Fix:
- Add yet another flag in get_user_pass_cr() to
  set when prompting of response from console is needed.
- In receive_auth_failed(), the challenge text received
  from server _always_ copied to  the auth_challenge
  buffer: this is needed to trigger prompting from console
  when required.
- Also show the challenge text instead of an opaque
  "Response:" at the prompt.

While at it, also remove the special treatment of authfile ==
"management" in get_user_pass_cr(). The feature implied by that
test does not exist.

Tested:
  - username and optionally password from file, rest from console
  - the above with a static challenge
  - the above with a dynamic challenge
  - all of the above with systemd in place of console
  - all from management with and without static/dynamic
    challenge.

Thanks to Wayne Davison <wayne@opencoder.net> for pointing out the
issue with challenge-response, and an initial patch.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1450638773-11376-1-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10868
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-18 19:07:44 +02:00
Steffan Karger
a44eac2bf4 Further restrict default cipher list
In the past years, the internet has been moving forward wrt deprecating
older and less secure ciphers.  Let's follow this example in OpenVPN and
further restrict the default list of negotiable TLS ciphers.

Compared to earlier, this disables the following:
 * Ciphers in the LOW and MEDIUM security cipher list of OpenSSL
   The LOW suite will be completely removed from OpenSSL in 1.1.0,
   the MEDIUM suite contains ciphers like RC4 and SEED.
 * Ciphers that do not provide forward secrecy (static DH/ECDH keys)
 * DSA private keys (rarely used, and usually restricted to 1024 bits)

v2: added Changes.rst entry.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1460917927-31645-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11457
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-18 16:42:58 +02:00
Jens Neuhalfen
6be0f0015d Make intent of utun device name validation clear
Make intend of the validation clear when validating utun parameter in
open_darwin_utun.  The program logic remains unchanged.

Fixes the following compiler warning on Mac OS X:

tun.c:2847:19: warning: logical not is only applied to the left hand side
of this comparison [-Wlogical-not-parentheses]
  if (dev_node && !strcmp ("utun", dev_node)==0)
                  ^                         ~~
tun.c:2847:19: note: add parentheses after the '!' to evaluate the
comparison first
  if (dev_node && !strcmp ("utun", dev_node)==0)
                  ^
                   (                           )
tun.c:2847:19: note: add parentheses around left hand side expression to
silence this warning
  if (dev_node && !strcmp ("utun", dev_node)==0)
                  ^
                  (                         )
tun.c:2849:11: warning: logical not is only applied to the left hand side
of this comparison [-Wlogical-not-parentheses]
      if (!sscanf (dev_node, "utun%d", &utunnum)==1)
          ^                                     ~~
tun.c:2849:11: note: add parentheses after the '!' to evaluate the
comparison first
      if (!sscanf (dev_node, "utun%d", &utunnum)==1)
          ^
           (                                       )
tun.c:2849:11: note: add parentheses around left hand side expression to
silence this warning
      if (!sscanf (dev_node, "utun%d", &utunnum)==1)
          ^
          (                                     )

Signed-off-by: Jens Neuhalfen <jens@neuhalfen.name>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <3365AB24-33FD-4D9D-A57C-BF9240DC3D69@neuhalfen.name>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11440
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-17 19:05:26 +02:00
Steffan Karger
0f99269711 fixup: change init_key_type() param name in declaration too
Commit 66407e11 changed the name of the cfb_ofb_allowed parameter of the
init_key_type() implementation to 'tls_mode', but forgot to do the same in
the function declaration.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1460886980-12925-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11445
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-17 12:08:00 +02:00
Selva Nair
d09fbf958f Ensure input read using systemd-ask-password is null terminated
Also properly check the return value of read() and leave room
for termination.
Fixes junk data occasionally seen in strings read through systemd.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1460606013-4983-1-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11437
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-14 22:07:26 +02:00
Arne Schwabe
7a7a79f62e Implement inlining of crl files
While crl files can change regulary and it is usually not a good idea to
statically include them into config files, handling multiple files and
updating files on mobile devices is tiresome/problematic. Inlining a static
version of the crl file is better in these use cases than to use no crl at
all.

OpenVPN 3 already supports inlining crl-verify, so <crl-verify> is already
used in config files.

V2: Fixed PolarSSL and made formatting respect the 80 column limit
V3: Accidentally reverted one change too much in V2
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1457293149-10526-1-git-send-email-arne@rfc2549.org>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11337

Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-04 21:33:09 +02:00
James Yonan
f6608a15ef Extended x509-track for OpenSSL to report SHA1 fingerprint.
For example:

  x509-track "+SHA1"

will extract the SHA1 fingerprints for all certs in the
peer chain.

This patch is ported from OpenVPN 2.1.

Signed-off-by: James Yonan <james@openvpn.net>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1456993146-63968-5-git-send-email-james@openvpn.net>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11281
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-04 21:25:07 +02:00
James Yonan
0a6a80156e Added flags parameter to format_hex_ex.
We add the flags parameter without changing the signature of
the function by repurposing the space_break parameter into
space_break_flags where the lower 8 bits are used for the
previous space_break parameter and the higher bits are used
for flag values.

Added new flag FHE_CAPS that formats the generated hex string
in upper case.

Signed-off-by: James Yonan <james@openvpn.net>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1456993146-63968-4-git-send-email-james@openvpn.net>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11275
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-04 21:20:03 +02:00
Steffan Karger
bbde0a766c Replace MSG_TEST() macro for static inline msg_test()
Using a static inline function instead of a macro has the advantages that
(1) 'flags' is not evaluated twice and (2) coverity will stop complaining
that 'Macro compares unsigned to 0 (NO_EFFECT)' each time we use flags
with loglevel 0 (e.g. M_FATAL or M_WARN).

This has a performance impact when compiler optimizations are fully
disabled ('-O0'), but should otherwise be as fast as using a macro.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1459088296-5046-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11368
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-04 20:28:41 +02:00
Steffan Karger
be16d5f6b0 Fix memory leak in argv_extract_cmd_name()
Reported by coverity (in 2009!):

1648 static char *
1649 argv_extract_cmd_name (const char *path)
1650 {
     1. Condition path, taking true branch
1651   if (path)
1652     {
1653       char *path_cp = string_alloc(path, NULL); /* POSIX basename()
implementaions may modify its arguments */
1654       const char *bn = basename (path_cp);
     2. Condition bn, taking true branch
1655       if (bn)
1656         {
     3. alloc_fn: Storage is returned from allocation function
string_alloc. [show details]
     4. var_assign: Assigning: ret = storage returned from
string_alloc(bn, NULL).
1657           char *ret = string_alloc (bn, NULL);
     5. noescape: Resource ret is not freed or pointed-to in strrchr.
1658           char *dot = strrchr (ret, '.');
     6. Condition dot, taking false branch
1659           if (dot)
1660             *dot = '\0';
1661           free(path_cp);
     7. Condition ret[0] != 0, taking false branch
1662           if (ret[0] != '\0')
1663             return ret;
     CID 27023 (#2-1 of 2): Resource leak (RESOURCE_LEAK)8.
leaked_storage: Variable ret going out of scope leaks the storage it
points to.
1664         }
1665     }
1666   return NULL;
1667 }

This function is only used by argv_printf_arglist(), and in a very specific
case, so it might be that this leak can not even occur.  But coverity is
clearly right that this is a bug, so let's just fix it.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1459092130-19905-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11369
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-04 20:09:30 +02:00
Steffan Karger
b064b8111c Fix potential null-pointer dereference
Commit a070f75b (master branch only) changed the openvpn_encrypt logic and
now prepends the contents of the work buffer to buf if no encryption is
used (which is the case for tls-auth packets).  In that case, the code
would potentially dereference a null-pointer in a memcpy(some-dest, 0, 0)
call.  Fortunately, memcpy() inplementations usually do not actually
derefence the src (or dst) pointer for zero-length copies.

And since I'm touching this code now anyway, remove a slightly confusing
jump back to a cleanup label in openvpn_encrypt_aead().

Issue spotted by Daniel Hirche.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1459528980-8304-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11372
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-04-01 20:35:14 +02:00
Selva Nair
2282b1be79 Add support for block-outside-dns through the interactive service
- Add a new message type in openvpn-msg.h
- Pass msg_channel HANDLE to win_wfp_block_dns and win_wfp_uninit
- Add a handler in interactive.c for block_dns request

The service build now depends on block_dns.[ch] in src/openvpn

v2 changes:
- Make CmpEngine non-nested (be nice with non-gcc compilers)
- Print error code in hex

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1456457091-3872-2-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11265
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-03-06 19:28:25 +01:00
Selva Nair
6a33a34dee Refactor and move the block-outside-dns code to a new file (block_dns.[ch])
- Move the core of win_wfp_block_dns() to a new function
- Remove globals and make it independent of the rest of the code

This facilitates implementing support for block-outside-dns through
the interactive service. Should not change any functionality.

v2 changes:
- In comments, correct DeleteBlockDNS() to delete_block_dns_filters

v2a: added <winsock2.h> and <ws2ipdef.h> (Gert Doering)

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1456457091-3872-1-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11264
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-03-06 19:27:01 +01:00
Steffan Karger
71d89065ad Only include aead encrypt/decrypt functions if AEAD modes are supported
This fixes the build for OpenSSL < 1.0.1 (broken by commit 3654d953),
which has no AEAD support.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1457266190-27228-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11325
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-03-06 13:17:58 +01:00
Steffan Karger
e0b3fd49e2 hardening: add safe FD_SET() wrapper openvpn_fd_set()
On many platforms (not Windows, for once), FD_SET() can write outside the
given fd_set if an fd >= FD_SETSIZE is given.  To make sure we don't do
that, add an ASSERT() to error out with a clear error message when this
does happen.

This patch was inspired by remarks about FD_SET() from Sebastian Krahmer
of the SuSE Security Team.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1456996968-29472-1-git-send-email-steffan.karger@fox-it.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11285
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-03-06 11:14:44 +01:00
Steffan Karger
13de0103ea Make AEAD modes work with OpenSSL 1.0.1-1.0.1c
The 'nobody uses OpenSSL 1.0.1-1.0.1c'-gamble in commit 66407e11 (add AEAD
support) did not turn out well; apparently Ubuntu 12.04 LTS ships with a
broken OpenSSL 1.0.1.  Since this is still a popular platform, re-add the
fixup code, now with a clear version check so it's easy to remove once we
drop support for OpenSSL 1.0.1.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1457256715-4467-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11322
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-03-06 10:39:36 +01:00
Selva Nair
3654d953eb Use appropriate buffer size for WideCharToMultiByte output in interactive.c
A widechar can potentially take more than 2 bytes in UTF-8.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1457241722-23433-1-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11318
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-03-06 10:24:50 +01:00
Selva Nair
239d09938b Fix interactive service ignoring stop command if openvpn is running
Make the exit event not auto-reset so that the signal propagates to
all worker threads and finally to the main thread.

Fixes Trac #666

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1457241559-23374-1-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11317
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-03-06 09:54:51 +01:00
Selva Nair
6370f70357 Handle localized Administrators group name in windows
Interactive service allows all configs and options if the user
is in "Administrators" group. This patch makes it work even if the
admin group is renamed or localized.

While at it, also remove two unused variables in validate.c.

Thanks to Leonardo Basilio <leobasilio@gmail.com> for testing
the patch on a localized version of windows and Samuli Seppänen
<samuli@openvpn.net> for pointing out this issue.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1457206796-11863-1-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11316
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-03-06 09:33:57 +01:00
Fish
6a4edc7fc0 Add lz4 support to MSVC.
- Include lz4 code and header in VC project files.
- Fix an issue in comp-lz4.h that prevents it from compiling under MSVC.

Signed-off-by: Fish <fish.thss@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1456434882-6009-1-git-send-email-fish.thss@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11262
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-26 08:36:24 +01:00
Gert Doering
9b9187031b Fix openserv/validate.o linking issues on mingw.
MinGW fails linking after f3c8a04d60 if the right header files
(<lm.h> and <shlwapi.h>) are not included.

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <1456412312-21936-1-git-send-email-gert@greenie.muc.de>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11255
2016-02-25 17:32:16 +01:00
Selva Nair
852f1e49c7 Send stdout and stderr of OpenVPN started by interactive service to NUL
Currently the service directs stdout/stderr of openvpn process to a pipe.
The
service never reads from it unless the process exits with an error. This
causes
the process to hang when large amount of log is written to stdout.

- Direct stdout/stderr to NUL
- Write the exit code (if nonzero) to the event log

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1455470881-32341-1-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11161
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-25 14:29:23 +01:00
ValdikSS
236769150f Update --block-outside-dns to work on Windows Vista
Windows Vista doesn't support non-equal matching of application name, it
is available only since Windows 7.

This commit splits 2 filtering conditions with non-equal matching to 2
filters each with 1 filtering condition: permit IPv4 (first filter)
and IPv6 (second filter) port 53 traffic from openvpn.exe instead
of blocking all non-openvpn.exe traffic on port 53 for both protocols.

Trac #648

Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <1452900938-3636-1-git-send-email-iam@valdikss.org.ru>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10998

Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-25 13:43:29 +01:00
Selva Nair
f3c8a04d60 Restrict options/configs for startup through interactive service
Windows only:

- Allow only a set of whitelisted options in the command line options
passed by
  interactive service clients unless
   (i) user is the local Adminsitrator group
      AND/OR
   (ii) in a predefined group (see below)
  Only the group membership is checked, the client process need not be
running with
  any elevated privileges available to those groups.

- Restrict config files to config_dir or it sub directories unless (i)
and/or (ii) above
  is true (config_dir is as defined in HKLM\Software\OpenVPN\config_dir)

- The predefined group may be set in the registry
HKLM\Software\OpenVPN\ovpn_admin_group
  (default: "OpenVPN Administrators")

- The white-list of options is a simple flat array of option strings
(without leading --)
  defined in validate.c

- Further options may be added to the whitelist without breaking the GUI
-- the startup
  data is passed from the GUI to the service the same way as before.

Notes to GUI developers:
(i) If the user is an administrator, the service will grant all privileges
even if
the GUI is not running elevated. This is practically equivalent to
'highestAvailable' without the risks of running the GUI elevated.

(ii) If the option checks fail, openvpn is not started, but an error
message
is passed back to the service pipe and written to event log. Currently the
GUI does
not read from the service pipe -- this needs fixing.

v2 changes:
  - checked non-unicode build and fixed an error -- in case anyone builds
non-unicode
  - added an info message to event log when user auth succeeds

Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1455937988-12414-1-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11225
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-25 12:59:59 +01:00
Steffan Karger
463ea2bc90 Clean up get_tls_handhake_key()
This function has *much* more code than required.  This commit cleans up
the function:
 * Merge the handling of inline and non-inline code.
 * Don't double-check key.2, since must_have_n_keys() already does that
   (but keep the message about dropped passphrase support in 2.4).
 * Remove stale references to 'passphrase' - we no longer support those

This commit should not change any behaviour except for log messages.

v2: Leave message about dropped passphrase support in place - this option
    was dropped in 2.4, so it is indeed better to be clear about it.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1456151046-16047-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11238
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-22 16:20:22 +01:00
Steffan Karger
b3560c98d7 Minor AEAD patch cleanup
* Remove stale function declaration.
   This slipped into the AEAD cipher modes patch, but the function is
   now implemented as a static function is ssl.c.
 * Add ASSERT() to ensure frame is not NULL.
 * Fix "ENCRYPT TO" log message in openvpn_encrypt_aead().

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1456016892-8671-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11233
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-21 13:25:52 +01:00
Steffan Karger
3a5a46cf2b Add preliminary server-side support for negotiable crypto parameters
Add preliminary support for Negotiable Crypto Parameters 'level 2'
(IV_NCP=2), as proposed by James Yonan on the openvpn-devel mailinglist:
http://comments.gmane.org/gmane.network.openvpn.devel/9385

This patch makes a server push a 'cipher XXX' directive to the client,
if the client advertises "IV_NCP=2", where XXX is the cipher set in the
server config file.

This enables clients that have support for IV_NCP to connect to a
server, even when the client does not have the correct cipher specified
in it's config file.

Since pushing the cipher directive is quite similar to pushing peer-id,
I moved peer-id pushing to the same prepare_push_reply() function I
created for pushing cipher.  Adding these directives as regular push
options allows us to use the existing 'push-continuation'
infrastructure.  Note that we should not reduce safe_cap in
send_push_reply, because it was never increased to account for peer-id.

This is a preliminary patch, which will be followed by more patches to
add client support, and configurability.

v2:
 * Reword doxygen of push_options_fmt()
 * No longer push IV_NCP as a server

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <CAA1Abx+gSgFH3=+xO6QN4NDAYwf8jctYhe8VyRxD8e1L=D6LWg@mail.gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11170
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-15 21:20:24 +01:00
Steffan Karger
44dc5d309c Add cipher name translation for OpenSSL.
This keeps naming consistent. For example, instead of id-aes128-GCM use
AES-128-GCM, which is more like AES-128-CBC.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1454874438-5081-10-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11081
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-15 20:23:01 +01:00
Steffan Karger
66407e11c4 Add AEAD cipher support (GCM)
Add Authenticated Encryption with Additional Data (AEAD) support for
ciphers, which removes the need for a separate HMAC step.  The MAC is
integrated into the cipher and the MAC tag is prepended to the payload.

This patch is inspired by the patch originally submitted by Kenny Root
on the openvpn-devel mailinglist, but does a number things differently:
 * Don't support XTS (makes no sense for VPN)
 * Don't support CCM (needs extra code to make it actually work)
 * Don't force the user to specify "auth none" (that would break
   tls-auth)
 * Add support for PolarSSL (and change internal API for this)
 * Update openvpn frame size ('link mtu') calculation for AEAD modes
 * Use the HMAC key as an implicit part of the IV to save 8 bytes per
   data channel network packet.
 * Also authenticate the opcode/peer-id as AD in P_DATA_V2 packets.

By using the negotiated HMAC key as an implicit part of the IV for
AEAD-mode ciphers in TLS mode, we can save (at least) 8 bytes on each
packet sent.  This is particularly interesting for connections which
transfer many small packets, such as remote desktop or voip connections.

The current AEAD-mode ciphers (for now GCM) are based on CTR-mode cipher
operation, which requires the IV to be unique (but does not require
unpredictability).

IV uniqueness is guaranteed by using a combination of at least 64-bits
of the HMAC key (unique per TLS session), and a 32-bit packet counter.
The last 32-bit word of the 128-bit cipher block is not part of the IV,
but is used as a block counter.

AEAD cipher mode is not available for static key mode, since IV
uniqueness is harder the guarantee over sessions, and I believe
supporting AEAD in static key mode too is not worth the extra
complexity.  Modern setups should simply use TLS mode.

OpenSSL 1.0.1-1.0.1c will not work with AEAD mode, because those
versions have an unnecessary check that fails to update the cipher if
the tag was not already set.  1.0.1d, which fixes that, was released in
February 2013.  People should have updated, and distros should have
backported the fix by now.

Changes in v2:
 * Remove extra code that was just for making OpenSSL 1.0.1-1.0.1c work
   in AEAD mode.
 * Do not make AEAD support configurable in ./configure.
 * Get rid of '12' magic constant in openvpn_encrypt_aead().
 * Update manpage to explain that --auth is ignored for the data channel
   when using an AEAD cipher.
 * Move setting the IV in AEAD cipher modes to the IV generation code.
   This is a more natural place and now we can pull iv[] into the IV
   generation scope.
 * Read packet ID directly from packet buffer instead of from iv buffer,
   to remove the need for an extra buffer.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <CAA1AbxL_S4umZr5Nd0VTvUvXEHjoWmji18GqM6FgmWqntOKqaA@mail.gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11162
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-15 20:19:19 +01:00
Leonardo Basilio
5f5229e41d Correctly report TCP connection timeout on windows.
On nonblocking TCP connects, we set status = ETIMEOUT on failure.
On windows, depending on which header files are included, ETIMEOUT
is defined differently, and this leads to incomprehensible error
messages - so, always use WSAETIMEDOUT here.

Trac #651

Signed-off-by: Leonardo Basilio <leobasilio@gmail.com>

Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <CACqLfMnBXwSY=MXyc7B1oMKwYE2Z_49G3mpkEPxbSAuG61tgZA@mail.gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11085
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-10 11:19:39 +01:00
Lev Stipakov
15f78acfae Report Windows bitness
Trac #599

Signed-off-by: Lev Stipakov <lstipakov@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1454876492-6588-1-git-send-email-lstipakov@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11086
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-10 11:15:17 +01:00
Steffan Karger
de4cbd62b7 Create separate function for replay check
In preparation for AEAD cipher modes, which will need the same
functionality.

Should not change any behaviour.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1454874438-5081-8-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11076
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-09 09:47:11 +01:00
Steffan Karger
a070f75b7d Change openvpn_encrypt() to append to work buffer only
Preparation for AEAD cipher modes, which also have to authenticate the
opcode and peer-id of packets.  To supply that information to
openvpn_encrypt(), I want to simply write those to the work buffer
before calling openvpn_encrypt().  That however requires that
openvpn_encrypt() never prepends something to the work buffer.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1454874438-5081-7-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11074
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-09 09:15:07 +01:00
Steffan Karger
3ebc31f959 Move packet_id into crypto_options
Decouples struct key_state and struct crypto_options. No longer updating
self-referential pointers!

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1454874438-5081-6-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11082
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-09 08:58:59 +01:00
Steffan Karger
8b1a00ca4b Move key_ctx_bi into crypto_options
The encrypt and decrypt routines use struct crypto_options as their main
information source.  A struct crypto_options would have a pointer to a
struct key_ctx_bi, which had to be updated at the correct moments to keep
them correct.  Instead of doing this administration, just put the struct
key_ctx_bi inside crypto_options.  Makes the code a little simpler too.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1454874438-5081-5-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11078
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-09 08:39:23 +01:00
Steffan Karger
2d9c6d20e6 Move crypto_options into key_state and stop using context in SSL-mode.
Moving crypto_options into key_state enables us to stop using the global
context for each packet encrypt/decrypt operation. Decoupling the crypto
from the global context removes the need to copy the relevant parts of
crypto_options for each processed packet, but instead enables us to just
pass along a pointer to the related crypto_options.

This paves the way for an efficient GCM cipher mode implementation, but is
probably fruitful too for threading and/or cipher negotiation.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1454874438-5081-4-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11075
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-09 08:02:35 +01:00
Steffan Karger
e7d78e407d Remove reuse of key_type during init of data channel auth and tls-auth
Prepare for using AEAD cipher modes + tls-auth, as tls-auth might want to
use an HMAC, while the data channel uses e.g. GCM tags.  This separates
the two initialisations.  Also, error out (and give a clear error message)
if a user specifies tls-auth but no valid auth algorithm, which makes no
sense at all.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1454874438-5081-3-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11073
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-09 07:58:40 +01:00
Steffan Karger
70fbc5be20 Allow NULL argument in cipher_ctx_get_cipher_kt()
Since otherwise we'll have to perform the check before each call.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1454874438-5081-2-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11079
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-09 07:51:14 +01:00
Heiko Hund
a24dd2e31f interactive service v3
v1: Heiko Hund
 - Message-ID: <2215306.x9ci9DhAZ9@de-gn-40970>
 - extend openvpn service to provide "automatic service" and "interactive
   service" (which is used by GUI and OpenVPN to run openvpn non-privileged
   and still be able to install routes and configure IPv6 addresses)
 - add --msg-channel <n> option to openvpn to tell it which pipe to use
   to talk to the interactive service (used in tun.c for ifconfig + ARP
   flush, and route.c for routing)
 - add openvpn-msg.h with message definitions for talking to interactive
   service
 - routing in openvpn uses message-pipe automatically if --msg-channel <n>
   is configured, no other option needed
 - today, the integration in route.c and tun.c is windows-only, but could
   be adapted to other platforms

v2: Steffan Karger
 - Message-ID: <548D9046.5000600@karger.me>
 - include "openvpn-msg.h" not "include/openvpn-msg.h"
 - add $(top_srcdir)/include to openvpnsrv build for out-of-tree builds

v3: Gert Doering, rebasing and integrating review feedback
 - rebased to 417fe4a72c
 - r->metric_defined is now r->flags & RT_METRIC_DEFINED (c3ef2d2333)
 - move "openvpn-msg.h" include inside #ifdef WIN32 (windows-only right now)
 - hide "msg_channel" extra option inside tt->tuntap_options, so we do not
   need an extra argument to all the add/del_route...() functions
 - do_route_ipv6_service(): use r->adapter index (if set) for RGI6 routes

Signed-off-by: Heiko Hund <heiko.hund@sophos.com>
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Acked-by: Selva Nair <selva.nair@gmail.com>        (Service changes)
Acked-by: Arne Schwabe <arne@rfc2549.org>          (OpenVPN changes)
Message-Id: <1453835508-26119-1-git-send-email-gert@greenie.muc.de>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11027
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-05 09:07:54 +01:00
Michael McConville
d4d5d9259a Fix undefined signed shift overflow
Originally discussed here:

https://github.com/OpenVPN/openvpn/pull/42

Thanks for your time,
Michael

Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20160202191122.GE1675@thinkpad.swarthmore.edu>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11050

Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-02-05 08:29:23 +01:00
Niels Ole Salscheider
9dfc2309c6 Fix build with libressl
Signed-off-by: Niels Ole Salscheider <niels_ole@salscheider-online.de>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1452433475-16779-1-git-send-email-niels_ole@salscheider-online.de>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10975
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-01-15 20:04:52 +01:00
Steffan Karger
982ab2364a socks.c: fix check on get_user_pass() return value(s)
My compiler rightfully complains that the checks on creds.username and
creds.password always evaluate to true, so remove those checks.

Judging from the code, they were meant to check the returned values by
get_user_pass().  So instead of these non-functioning checks, just check
the return value of get_user_pass().

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1452701348-9577-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10993
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-01-15 10:04:18 +01:00
Steffan Karger
52012d6516 polarssl: remove now redundant 128-bit blowfish key override
As of 1.3.0, polarssl/mbedtls now by default uses a 128 bit key for the
blowfish cipher (as opposed to the 32-bit (!) default they had previously).
Since we require polar 1.3+, we no longer need this fixup code.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1452198273-26493-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10956
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-01-08 08:53:29 +01:00
Steffan Karger
072fdcd006 polarssl: use wrappers to access md_info_t member functions
The md_info_t will become an opaque struct in mbed TLS 2.x, start using
the wrapper function in preparation to a future upgrade to 2.x.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1452198132-25560-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10955
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-01-08 08:50:11 +01:00
Steffan Karger
3a39bf7dfe polarssl: optimize polar_ok() for non-errors
Adding polar_ok() was a good plan for improving error reporting, but also
added two function calls (one to polar_log_func_line() and one to
polar_log_err()) for each function call wrapped with polar_ok().
Especially in the critical path, this is a waste of time.

To avoid this overhead, add a simple static inline wrapper to reduce it to
a single branch.

v2 - use a static inline wrapper to prevent evaluating 'errval' twice in
     the macro.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1452158116-17363-1-git-send-email-steffan.karger@fox-it.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10949
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-01-07 11:07:23 +01:00
Steffan Karger
aa416be950 polarssl: actually use polarssl debug logging
We had the machinery in place, but did not actually use it because nothing
will be logged untill the debug threshold is increased.

This commit makes --verb 8 result is level 2 polar logging (which is
verbose, and --verb 9 result in level 3 polar logging (which is very
verbose).  There are higher levels, but those are extremely verbose.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1452113943-30684-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10945
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2016-01-07 09:46:17 +01:00