There's quite a bit of our code that fails compilation with
"gcc -pedantic -ansi" and should not be changed - like, LZ4 using
"long long" variables which C90 does not have. Be pragmatic.
trac #616
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1465477232-25826-1-git-send-email-gert@greenie.muc.de>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11882
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Take upstream release from https://github.com/Cyan4973/lz4/releases,
copy lz4-r131/lib/lz4.c to src/compat/compat-lz4.c
copy lz4-r131/lib/lz4.h to src/compat/compat-lz4.h
change #include line in compat-lz4.c to use "compat-lz4.h" not "lz4.h"
add "config.h" block and wrap in #ifdef NEED_COMPAT_LZ4 / #endif
No other changes to upstream code.
This commit is quite huge, but this is because we bundled a fairly old
version and upstream refactored quite a lot of code, changed // comments
to /* */ style, etc - to review, compare compat-lz4.* files to upstream.
v2: add missing #ifdef NEED_COMPAT_LZ4 wrapping
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <1465473089-20754-1-git-send-email-gert@greenie.muc.de>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11879
Signed-off-by: Gert Doering <gert@greenie.muc.de>
multi_top_init() is always called with alloc_buffers=true, so just remove
the argument and alloc unconditionally.
Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1465388443-15484-3-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11851
Signed-off-by: Gert Doering <gert@greenie.muc.de>
"Be conservative in what you send, be liberal in what you accept"
When receiving packets, the real limitation of how much data we can accept
is the size of our internal buffers, not the maximum size we expect
incoming packets to have.
I ran into this while working on cipher negotiation, which will need
separate bookkeeping for the required internal buffer size, and the
link/tun MTU. Basing this code on the buffer size instead of c2->frame
makes that easier. A nice side-effect of this change is that it
simplifies the code.
This should also reduce the impact of using asymmetric tun/link MTU's,
such as in trac ticket #647.
Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1465388443-15484-2-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11850
Signed-off-by: Gert Doering <gert@greenie.muc.de>
This allows exit notification to complete and finally trigger SIGTERM.
The current practice of allowing a restart in this state clears
the exit notification timer data and thus loses the SIGTERM.
Trac #687
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1465274660-11009-2-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11814
Signed-off-by: Gert Doering <gert@greenie.muc.de>
v2 changes:
- Add the flag "ignore" and have "reject" trigger a restart.
- Unlimited number of filters: yes, going against the consensus,
but the code looks simpler and cleaner this way.
- New commit message to reflect the changes.
Usage: --pull-filter accept|ignore|reject "option"
Permit a client to selectively accept, ignore or reject options
pushed by the server. May be used multiple times. The filters
are applied in the order specified to each pushed option received.
The filtering stops as soon as a match is found. The action "ignore"
removes the option and continues processing the next option, while
"reject" flags an error and restarts the connection with SIGUSR1.
Prefix matching is used so that all options starting with the
specified "option" string are filtered.
Example:
pull-filter accept "route 192.168."
pull-filter ignore "route "
pull-filter accept "ifconfig 10.9.0."
pull-filter reject "ifconfig "
will ignore all pushed routes except those starting with "192.168."
and reject the assigned ip unless its in the "10.9.0.0/24"
range. A match of the reject filter will trigger a restart. SIGUSR1
restart is used instead of SIGHUP so as to try the next remote
for reconnection.
Note the space at the end of "route " to not reject "route-gateway",
for example. All options not matched by any filter are accepted.
Acknowledges shameless imitation of --push-remove.
Inspired by Trac #682.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1465162884-32520-1-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11808
Signed-off-by: Gert Doering <gert@greenie.muc.de>
- Remove and recreate WFP filters during restart even when
tun/tap is not re-opened. This is needed for resolving the remote.
Patch same as for v2.3 except for passing 'msg_channel'.
See also: http://article.gmane.org/gmane.network.openvpn.user/36990
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1465055833-13681-1-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11788
Signed-off-by: Gert Doering <gert@greenie.muc.de>
This was missed by commit 6a33a34dee
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1465070945-5426-1-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11789
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Continuing to fix breakage caused by commit 40cb4cfc5d.
It seems it was a conflict in vendor/Makefile.am's distdir target,
confusing autotools so it wouldn't actually parse that directory
properly. The result was that 'make distcheck' would fail and
tarballs created would just ship with an empty vendor/ directory.
Also remove the 'foreign' AUTOMAKE_OPTIONS flag, as we don't use
that many places at all. Things work well without this flag.
The comment had to be moved to a single line, otherwise the
white spaces between the end of the variable assignment and the
hash character got added to the variable.
[v3 - Further improve white space issues, now 'make clean'
should work too]
[v2 - Fix white space issues in path variables]
Signed-off-by: David Sommerseth <dazo@privateinternetaccess.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1464976163-6162-1-git-send-email-openvpn@sf.lists.topphemmelig.net>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11778
Commit 40cb4cfc5d added infrastructure to write unit tests using
cmocka. This was implemented using a git submodule to fetch an
up-to-date cmocka test framework.
The issue which appeared was that 'make check' stopped working if
the cmocka submodule was not initialized and updated. As we do not
want this to be a hard depenency, this patch makes running these
unit tests conditional. If cmocka has not been initialized, skip
them or if it has been initialized all unit tests will be run.
[v2 - Also check if cmake is available, as cmocka depends on that
to be built ]
Signed-off-by: David Sommerseth <dazo@privateinternetaccess.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1464703645-26640-1-git-send-email-openvpn@sf.lists.topphemmelig.net>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11758
When no DOMAIN is received from push/pull, do not add either domain or
search to the resolv.conf. Fix typo in comment resolv.con[f]. Only add
new line when using domain or search.
URL: https://github.com/OpenVPN/openvpn/pull/34
Acked-by: Steffan Karger <steffan@karger.me>
Signed-off-by: Samuli Seppänen <samuli@openvpn.net>
Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <1463736310-17846-1-git-send-email-samuli@openvpn.net>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11682
Signed-off-by: David Sommerseth <dazo@privateinternetaccess.com>
No functional changes.
Utility functions of auth-pam are split into a dedicated file. This allows
the test programs to easily test these functions without adding
dependencies.
Add a minimal test for searchandreplace as a proof of concept.
[ Modified during commit: Enhanced documentation of functions in utils.h
to comply with doxygen standards ]
Signed-off-by: Jens Neuhalfen <jens@neuhalfen.name>
Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20160525175756.56186-3-openvpn-devel@neuhalfen.name>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11724
Signed-off-by: David Sommerseth <dazo@privateinternetaccess.com>
cmocka [1,2] is a testing framework for C. Adding unit test
capabilities to the openvpn repository will greatly ease the
task of writing correct code.
cmocka source code is added as git submodule in ./vendor. A
submodule approach has been chosen over a classical library
dependency because libcmocka is not available, or only
available in very old versions (e.g. on Ubuntu).
cmocka is build during 'make check' and installed in vendor/dist/.
[1] https://cmocka.org/
[2] https://lwn.net/Articles/558106/
Signed-off-by: Jens Neuhalfen <jens@neuhalfen.name>
Acked-by: Steffan Karger <steffan@karger.me>
Message-Id: <20160525175756.56186-2-openvpn-devel@neuhalfen.name>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11725
Signed-off-by: David Sommerseth <dazo@privateinternetaccess.com>
Currently realloc failure in UpdateWaitHandles() is handled by
triggering exit_event and waiting for all active worker threads
to terminate. However, at this point the wait handles array
will contain an invalid value (handle of the latest thread that
is terminated), causing a cycle of WAIT_FAILED <-> continue and
trashing of the eventlog.
Fix:
- Update the wait handles again after removing the last thread:
this should not fail as no extra memory is needed. Do not set
the exit event; existing connections are not terminated.
- In case of WAIT_FAILED, break out of the while loop and exit
instead of continue. This usually happens when one or more
handles are invalid, which is hard to recover from.
Other changes:
- Use minimal initial allocation size so that the realloc code path
gets exercised (2 or more connections will cause realloc).
- Use a temp variable to check the return value of realloc().
- Initialize handles array pointer to NULL.
v2 changes:
- Increased initial allocation to 10 (warn: now 10 or more connections
needed to exercise the realloc code path).
- Moved up the declaration of "LPHANDLE tmp" to please stone-age MSVC.
Tested using a dummy realloc that returns NULL.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1463942372-26958-1-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11708
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Correctly handle CIDR masks when pushing clients addressing from an IPv6
pool. This change ignores the incorrectly used `bits` argument to the
--ifconfig-ipv6-pool option.
The code to save any provided CIDR mask after the pool IP is left in;
this may someday become useful when we move to allow IPv6 pools without
relying on an IPv4 pool assignment.
Signed-off-by: Josh Cepek <josh.cepek@usa.net>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <53F1DA95.7020701@usa.net>
URL: http://article.gmane.org/gmane.network.openvpn.devel/8990
Signed-off-by: Gert Doering <gert@greenie.muc.de>
With this option, the server can remove individual options from the
set pushed to a client (call from --client-config-dir file, or from
--client-connect script or plugin). Options are removed at parse
time, so it is possible to do stuff like:
push-remove route-ipv6
push "route-ipv6 fd00::/8"
to first remove all IPv6 route options set so far, then add something
specific (what "push-reset" does to all the options).
Arguments to push-remove are strncmp()'ed to option string, so partial
matches like
push-remove "route-ipv6 2001:"
are possible ("remove all IPv6 routes starting with 2001:").
Implementation of remove_iroutes_from_push_route_list() had to be changed
slightly to stop it from re-enabling all disabled options again.
v2: documentation (Changes.rst, doc/openvpn.8)
remove surplus gc_arena
implement filtering of "ifconfig-ipv6"
v3: correct quoting in commit message
only handle a single argument per push-remove statement - if multiple
options are to be removed, just use multiple push-remove statements
Trac #29, #614
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1463393584-8318-1-git-send-email-gert@greenie.muc.de>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11665
Signed-off-by: Gert Doering <gert@greenie.muc.de>
The call to the service returns promptly after delegating the job to
a thread, before the task is completed. In the thread, "net stop dnscache",
"net start dnscache", "ipconfig /flushdns" and "ipconfig /register-dns"
are executed in that order.
Parallel execution of these commands is prevented by a lock that is
common to all connections started by the service.
Note: "net stop .." is used instead of "sc stop.." as the latter can
return before the service has fully stopped (in STOP_PENDING state),
causing the subsequent start to fail.
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1457671646-4322-1-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11354
Signed-off-by: Gert Doering <gert@greenie.muc.de>
During windows power events such as sleep or suspend, the TUN/TAP
I/O aborts and openvpn signals SIGHUP so as to automatically reconnect
on resume (since commit ea66a2b5cdb2..). During the SIGHUP processing
operations such as address and route deletion are expected to fail. Such
failures should be treated as non-fatal to allow for this automatic
recovery logic to work. Currently, when the address deletion is handled
by netsh, errors are treated as M_FATAL. This patch changes the error
level to M_WARN.
Resolves Trac #71 (comments 37 to 43)
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1463185884-4355-2-git-send-email-selva.nair@gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11656
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Fixed port-share bug that can cause segfault when the number
of concurrent connections is large.
The issue is that the port-share code calls openvpn_connect()
which in turn calls select(). When there are a high number
of concurrent port-share connections, the fd passed to select
can potentially exceed FD_SETSIZE, causing undefined behavior.
The fix is to use poll() (if available) instead of select().
Signed-off-by: James Yonan <james@openvpn.net>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <CAA1Abx+2E2FZN-y6P=mkKpSuZ7bOV5m6rUMTx3V7UP2qPMjZPg@mail.gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11626
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Integration tests run by t_client.sh use sudo to run openvpn as root.
If the t_client.sh script is configured to use sudo then the user
must enter the password quickly because t_client assumes a startup
failure if openvpn does not start quick enough.
If the user is not quick enough, then the tests fails.
This change will refresh the sudo timestamp at the start of the script.
Tested on MacOS X & Ubuntu Precise
Signed-off-by: Jens Neuhalfen <jens@neuhalfen.name>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <280154CA-9468-429B-BCAC-DB632C0AFB32@neuhalfen.name>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11622
Signed-off-by: Gert Doering <gert@greenie.muc.de>
-lmbedx509 needs to be before -lmbedcrypto, otherwise you end up with
unresolved symbols mbedtls_pk_load_file and mbedtls_pk_parse_subpubkey
on systems with static mbedtls libraries and a linker that only does
one left-to-right resolving pass through these.
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20160505115050.GA81579@greenie.muc.de>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11605
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Instead of just printing the contents of the flags variable, try to
convert it to a human-readable error string and print that instead.
This will for example print "The certificate is signed with an
unacceptable key (eg bad curve, RSA too short).", instead of
"flags=10000".
Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1462306478-21059-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11594
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Implement a long standing todo in the code, now that we use mbed TLS 2.x,
which provides an interface for the check.
v2: bail out immediately on failure.
Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1462438933-14902-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11599
Signed-off-by: Gert Doering <gert@greenie.muc.de>
When for some reason the dependencies of the compiled mbed TLS libaries
(libmbedtls, libmbedcrypto and libmbedx509) are not correct, the configure
script will fail to link against libmbedcrypto and/or libmbedx509. This
is reported to happen after using 'make install' to install mbedtls.
This patch makes sure the configure tests link to all three. The build
process itself already did.
Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1462126986-2686-1-git-send-email-steffan@karger.me>
Signed-off-by: Gert Doering <gert@greenie.muc.de>
t_client.rc is a config file used in integration tests (t_client.sh).
It is a local (developer/machine) specific file not intended to be
verisonized.
A template file can be found at ./tests/t_client.rc-sample
Signed-off-by: Jens Neuhalfen <jens@neuhalfen.name>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <80711231-C760-410A-B8A5-F2B1D46C22CE@neuhalfen.name>
Signed-off-by: Gert Doering <gert@greenie.muc.de>
The patch looks huge, but it's just file renames, and required changes in
includes / Makefiles. Use 'git diff -C' or a tool like gitk to easily
review this patch.
Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1460918143-408-2-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11459
Signed-off-by: Gert Doering <gert@greenie.muc.de>
PolarSSL / mbed TLS 1.3 is going end-of-life by 2016-12-31, so let's move
the master branch on to the 2.x series.
This patch purges all references to polarssl, except for file names and
some comments referring to 1.2 and earlier, which were never released as
'mbed TLS'. A separate patch for the file names follows, so the real
changes are easier to spot without git-fu.
This patch intends to not change any behaviour.
The vast majority of this patch is just renaming functions and structs.
There are some small changes in the implementation:
* In ssl_polarssl.c: the debug callback prototype changed, so our
implementation changed a bit too.
* in ssl_polarssl.c: the old polarssl ssl_context is now split into a
mbedtls_ssl_config and mbedtls_ssl_context. The intention is that
mbedtls_ssl_config is shared among connections, and mbedtls_ssl_context
contains the per-connection state. That doesn't work for us, because
we use per-connection verify callback data, while the verify callback
is registered on mbed_tls_config. Therefore we still need to init a
mbed_tls_config struct for each connection.
* in ssl_polarssl.c: the mbed bio handling changed, so our
implementation changed a bit too.
* in ssl_polarssl.c and ssl_verify_polarssl.c: the mbedtls x509 parse
functions now fail if we don't provide a NUL-terminated string, so use
strlen()+1 as the length argument to include the terminating NUL.
I tested this patch to work with:
* 'make check' (with 2.0.0 and 2.2.1, other tests just with 2.2.1)
* static key mode
* TLS mode with PEM key file
* TLS mode with password protected PEM key file
* TLS mode with management-external-key
* TLS mode with PKCS#11
* TLS mode with inline ca/key/cert/dh
Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1460918143-408-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11458
Signed-off-by: Gert Doering <gert@greenie.muc.de>
This patch is a variant of the patch to implement x509-track for
PolarSSL that was sent to openvpn-devel@ by James Yonan
(<1456993146-63968-7-git-send-email-james@openvpn.net>). It still uses
some of the original code from James, but proposes a different
implementation.
This patch does the following things differently:
* Do not introduce NID_* defines that need to be maintained. Instead,
just use the short name of the attribute for identification. This
has the advantage that we automatically support everything that
PolarSSL supports, it is less code and we do not have maintain the
list. But the disadvantage is that this approach will not error out
when an unknown attribute name is supplied. PolarSSL (at least 1.3,
I didn't check 2.x) does not provide the functions required to do
that. Instead of erroring out, this implementation will just
silently ignore the unknown --x509-track attribute name.
* Remove the ENABLE_X509_TRACK define completely - it depended just on
ENABLE_CRYPTO anyway.
* Move the --x509-track option parsing out of ENABLE_MANAGEMENT, since
it does not depend on management functionality.
Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <CAA1AbxL1w8e_o-GjS2jETZWxYdMbS2iKABPc6OZBA8bOVycjtA@mail.gmail.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11350
Signed-off-by: Gert Doering <gert@greenie.muc.de>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
