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

110 Commits

Author SHA1 Message Date
Arne Schwabe
8fa8a17528 Implement '--compress migrate' to migrate to non-compression setup
This option allow migration to a non compression server config while
still retraining compatibility with client that have a compression
setting in their config.

For existing setups that used to have comp-lzo no or another
compression setting in their configs it is a difficult to migrate to
a setup without compression without replacing all client configs at
once especially if OpenVPN 2.3 or earlier clients are in the mix that
do not support pushing stub-v2. Even with OpenVPN 2.4 and later clients
that support pushing this is not a satisfying solution as the clients
log occ mismatches and the "push stub-v2" needs to be in the server
config "forever".

If the new migrate option to compress is set and  a client is detected
that indicates that compression is used (via OCC), the server will
automatically add ``--push compress stub-v2`` to the client specific
configuration if stub-v2 is supported by the client and otherwise
switch to ``comp-lzo no`` and add ``--push comp-lzo`` to the client
specific configuration.

Patch v2: better commit message/man page, add USE_COMP ifdefs, various
          style fixes

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210324220853.31246-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21801.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2021-04-02 14:49:39 +02:00
Arne Schwabe
88664aba69 Refactor extract_var_peer_info into standalone function and add ssl_util.c
Our "natural" place for this function would be ssl.c but ssl.c has a lot of
dependencies on all kinds of other compilation units so including ssl.c
into
unit tests is near impossible currently. Instead create a new file
ssl_util.c
that holds small utility functions like this one.

Patch v2: add newline add the end of sll_util.h and ssl_util.c

Patch v3: Refactor/clean up the function even more as suggested by Gert.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20210226111012.21269-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21585.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2021-03-10 10:40:18 +01:00
Arne Schwabe
06f6cf3ff8 Prefer TLS libraries TLS PRF function, fix OpenVPN in FIPS mode
This moves from using our own copy of the TLS1 PRF function to using
TLS library provided function where possible. This includes currently
OpenSSL 1.1.0+ and mbed TLS 2.18+.

For the libraries where it is not possible to use the library's own
function, we still use our own implementation. mbed TLS will continue
to use our own old PRF function while for OpenSSL we will use a
adapted version from OpenSSL 1.0.2t code. The version allows to be
used in a FIPS enabled environment.

The old OpenSSL and mbed TLS implementation could have shared some
more code but as we will eventually drop support for older TLS
libraries, the separation makes it easier it remove that code
invdidually.

In FIPS mode MD5 is normally forbidden, the TLS1 PRF1 function we
use, makes uses of MD5, which in the past has caused OpenVPN to segfault.
The new implementation for OpenSSL version of our custom implementation
has added the special flags that tell OpenSSL that this specific use
of MD5 is allowed in FIPS mode.

No FIPS conformitiy testing etc has been done, this is only about
allowing OpenVPN on a system where FIPS mode has been enabled system
wide (e.g. on RHEL derivates).

Patch v4: Handle the unlikely case that PRF generation fails. More
formatting
          fixes.
Patch v5: v4 with the formatting fixes actually commited. sigh.

Patch v6: More formatting fixes, make OpenSSL fucntion return bool instead
          of int.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210305141352.21847-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21612.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2021-03-08 10:43:01 +01:00
Arne Schwabe
c018fc00be Allow 'none' cipher being specified in --data-ciphers
Although we want to get rid of none as cipher, we still have not
deprecated it. In order to use it currently you need
--ncp-disable together with --cipher none to use the none cipher
otherwise OpenVPN will spit out an error about an unrecognised
cipher in --data-ciphers.

In our current situation allowing none to be specified in data-ciphers
is the lesser evil.

This commit also fixes that we use '[null-cipher]' instead 'none' when
setting remote_cipher.

Note that negotiating to cipher 'none' can the same the same problems
with frame size calculation as any other non AEAD cipher. If
--cipher none is also specified in the configuration, the workaround
of commit e539c95dc will also apply to cipher none.

Patch V2: Also work correctly if remote_cipher is NULL.
Patch V3: fix unit tests, add note about corner case

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20201008115959.21151-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21181.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-10-08 16:59:56 +02:00
Gert Doering
81f9bb3a2f Replace 'echo -n' with 'printf' in tests/t_lpback.sh
"echo -n" is inherently less portable than printf, so the tests look
ugly on (at least) OpenSolaris/Illumos on AIX.

Add a blank at the end of the tls-crypt-v2 messages, so it has the
same look as the cipher messages ("... OK").

Reported-by: mnowak on Trac
Trac: #1196

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Antonio Quartulli <a@unstable.cc>
Message-Id: <20200909130024.24264-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20930.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-09-11 12:56:59 +02:00
Arne Schwabe
2c1d8c33d9 Rework NCP compability logic and drop BF-CBC support by default
This reworks the NCP logic to be more strict about what is
considered an acceptable result of an NCP negotiation. It also
allows us to finally drop support for BF-CBC as default cipher.

All new behaviour is currently limited to server/client
mode with pull enabled. P2p mode without pull does not change.

New Server behaviour:
- when a client announces its supported ciphers through either
  OCC or IV_CIPHER/IV_NCP we reject the client with a
  AUTH_FAILED message if we have no common cipher.

- When a client does not announce any cipher in either
  OCC or NCP we reject it unless data-ciphers-fallback is
  specified in either ccd/ or config.

New client behaviour:
- When no cipher is pushed (or a cipher we refused to support)
  and we also cannot support the server's cipher announced in
  OCC we fail the connection and log why

- If there is no cipher in OCC but data-ciphers-fallback is
  specified we will use the fallback cipher instead of failing the
  connection

Both client and server behaviour:
- We only announce --cipher xyz in occ if we are willing
  to support that cipher (always announce the cipher if
  NCP is disabled or not in --client mode)

  It means that we only announce the fallback-cipher if
  it is also contained in --data-ciphers

Compatibility behaviour:

In 2.5 both client and server will use a --cipher xyz present
in the config to automatically set --data-ciphers-fallback xyz
and also append this cipher to the end of data-ciphers.

We log a warning about this and point to --data-ciphers and
--data-ciphers-fallback This also happens if the configuration
contains an explicit --cipher BF-CBC.

If --cipher is not set, we only warn that previous versions
allowed BF-CBC and point out how to re-enable BF-CBC. This will
break configs where someone connects a 2.3 client (or older)
to a 2.5 server AND has no explicit --cipher setting in the
server config.  We still do it, because at some point we need
to drop the BF-CBC default - and affected users already had the
scary SWEET32 warning in their logs for a long time.

In short: If --cipher is explicitly set then 2.5 will work the
same as 2.4 did. When --cipher is not set, BF-CBC support is
dropped and we warn about it.

Examples how breaking the default BF-CBC will be logged:

Client side:
 - Client connecting to server that does not push cipher but
   has --cipher in OCC

    OPTIONS ERROR: failed to negotiate cipher with server.  Add the
            server's cipher ('BF-CBC') to --data-ciphers (currently
            'AES-256-GCM:AES-128-CBC') if you want to connect to this server.

 - Client connecting to a server that does not support OCC:

    OPTIONS ERROR: failed to negotiate cipher with server. Configure
            --data-ciphers-fallback if you want connect to this server.

Server Side:
- Server has a client only supporting BF-CBC connecting:

  styx/IP PUSH: No common cipher between server and client. Server
          data-ciphers: 'CHACHA20-POLY1305:AES-128-GCM:AES-256-GCM:AES-256-CBC:AES-128-CBC', client supports cipher 'BF-CBC'.

 - Client without OCC:

   styx/IP PUSH:No NCP or OCC cipher data received from peer.
   styx/IP Use --data-ciphers-fallback with the cipher the client is using
           if you want to allow the client to connect

In all cases the client is rejected with this message:

   AUTH: Received control message: AUTH_FAILED,Data channel cipher
         negotiation failed (no shared cipher)

Signed-off-by: Arne Schwabe <arne@rfc2549.org>

Patch V2: rename fallback-cipher to data-ciphers-fallback
          add all corrections from Steffan
          Ignore occ cipher for clients sending IV_CIPHERS
          move client side ncp in its own function
          do not print INSECURE cipher warning if BF-CBC is not allowed

Patch V3: fix minor style, add null check when client sends no peerinfo at
          all

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200809141922.7853-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20656.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-08-10 13:41:57 +02:00
Antonio Quartulli
e6c86b24db t_net.sh: drop hard dependency on t_client.rc
Right now t_net.sh depends on t_client.rc in order to source the
RUN_SUDO variable only.
However, t_client.rc is something that a few people only have configured
and thus this would result in t_net.sh almost never executed even if it
just could.

Drop dependency on t_client.rc by falling back to RUN_SUDO=sudo when the
file is missing and no RUN_SUDO is passed via env.

While at it, reword the error message to better match the current logic
flow.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200721195518.14358-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20533.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-07-22 08:56:07 +02:00
Gert Doering
ec33bae311 t_client.sh: correctly report all failed instances in summary
t_client.sh reports a summary at the end:

  Test sets succeeded: none.
  Test sets failed: 1 2 3 4 5.

for tests that are skipped due to the pre-test ping check ("vpn target
IP must not ping before VPN ist started") the script forgot to add
the instance number to the summary line.  Fixed.

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20200626082743.15397-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20130.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-07-03 23:50:44 +02:00
Gert Doering
8a168a9ac8 Fix 'engine' unit test on FreeBSD (specifically 'not GNU make')
The rules to generate $(builddir)/openssl.cnf from $(srcdir)/openssl.cnf.in
only worked for GNU Make.  BSD make needs the rules more explicit, and
the target must not have a directory specification (fixes commit
542c69c37).

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Message-Id: <20200629175109.94276-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20159.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-07-01 16:56:06 +02:00
Arne Schwabe
c1ff8f247f Reformat files using uncrustify
Some of the commits, especially engine have not strictly used uncrustify
clean code. Rerun uncrustify to make them compliant again.
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200626125332.15385-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20142.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-06-26 14:58:47 +02:00
James Bottomley
013498ddfe engine-key tests: make check_engine_keys.sh work with --enable-small
--enable-small eliminates one of the openssl errors the test is
looking for, so alter the grep also to account for the message in this
version.  Additionally output log.txt on failure so any test platform
gives an easy clue about what went wrong.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1592953354.2103.3.camel@HansenPartnership.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20102.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-06-24 09:55:50 +02:00
James Bottomley
21e3e9fc34 Fix make distcheck for new engine key unit test
Add config precursor and script to extra dist and make sure
built and test leftover files are cleaned up afterwards.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1592917531.4768.4.camel@HansenPartnership.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20088.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-06-23 20:45:51 +02:00
James Bottomley
542c69c37b Add unit tests for engine keys
Testing engines is problematic, so one of the prerequisites built for
the tests is a simple openssl engine that reads a non-standard PEM
guarded key.  The test is simply can we run a client/server
configuration with the usual sample key replaced by an engine key.
The trivial engine prints out some operations and we check for these
in the log to make sure the engine was used to load the key and that
it correctly got the password.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200622232319.8143-2-James.Bottomley@HansenPartnership.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20075.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-06-23 08:16:35 +02:00
Arne Schwabe
3bc12aefd5 Add unit test for cipher name translations
The unit test duplicates some part of the test for
the ncp-cipher list but that is not a bad thing.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200605112519.22714-3-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19968.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-06-21 10:33:39 +02:00
Arne Schwabe
14a57be460 Do not write extra 0 byte for --gen-key with auth-token/tls-crypt-v2
Change crypto_pem_encode to not put a nul-terminated terminated
string into the buffer. This was  useful for printf but should
not be written into the file.

Instead do not assume that the buffer is null terminated and
print only the number of bytes in the buffer. Also fix a
similar case in printing static key where the 0 byte was
never added to the buffer

Patch V2: make pem_encode behave more like other similar functions in
OpenVPN
          and do not null terminate.

Patch V3: also make the mbed TLS variant of pem_decode behave like other
          similar functions in OpeNVPN and accept a not null-terminated
          buffer.

Patch V4: The newly introduced unit test
          test_tls_crypt_v2_write_client_key_file_metadata
          was added after the V3 version of the patch and now misses the
          strlen with memcmp replacment that were added to
          test_tls_crypt_v2_write_client_key_file. Also add the
          modifictions to this function.

          Unconditionally allocate buffer in mbed TLS path as
          requested by Steffan.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Steffan Karger <steffan.karger@foxcrypto.com>
Message-Id: <20200507132534.6380-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19852.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-05-15 17:56:50 +02:00
Antonio Quartulli
cb2e9218f2 convert *_inline attributes to bool
Carrying around the INLINE_TAG is not really efficient,
because it requires a strcmp() to be performed every
time we want to understand if the data is stored inline
or not.

Convert all the *_inline attributes to bool to make the
logic easier and checks more efficient.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200507135909.21227-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19854.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-05-07 16:58:08 +02:00
Antonio Quartulli
3cb9b156c8 t_net.sh: assign MAC address directly during interface creation
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200428131700.9123-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19832.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-05-07 13:32:06 +02:00
Gert Doering
da1574ef78 Uncrustify the tests/unit_tests/ part of our tree.
Apply uncrustify 0.70.1 (FreeBSD port) with our rules to that part
of the tree, which followed a more compact coding style so far.

Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20200426095402.65047-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19823.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-04-27 10:05:21 +02:00
Arne Schwabe
a17e735314 Add tls-crypt-v2 test writing metadata
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20200420104435.7082-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19798.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-04-26 11:45:06 +02:00
Antonio Quartulli
d8225e98f2 t_net.sh: use dummy interface instead of tun
The tun interface has proved to be a bit fragile for basic netlink tests
as it may introduce delays in switching state, depending on the system
the test is ran on.

For this reason, switch to dummy interface type and at the same type
set its oper-state to up right after creation to avoid hitting the
no-carrier state later. No-carrier has been problematic in pasts tests
as it sometimes persists long enough to create a discrepancy between the
various tests snapshots thus causing a test failure.

Setting a static MAC addressis also re-enabled to avoid it being
different and thus causing a test failure when comparing snapshots.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200416134925.8848-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19751.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-04-19 12:05:59 +02:00
Arne Schwabe
be4531564e Normalise ncp-ciphers option and restrict it to 127 bytes
In scenarios of mbed TLS vs OpenSSL we already normalise the ciphers
that are send via the wire protocol via OCC to not have a mismatch
warning between server and client. This is done by
translate_cipher_name_from_openvpn. The same applies also to the
ncp-ciphers list. Specifying non normalised names in ncp-ciphers will
cause negotation not to succeed if ciphers are not in the same form.
Therefore we will normalise the ciphers in options_postmutate.

The alternative and a lot less user friendly alternative would be to
bail if on of the ciphers in ncp-ciphers is not in its normalised form.

Also restrict the ncp-ciphers list to 127. This is somewhat arbitrary
but should prevent too large IV_CIPHER messages and problems sending
those. The server will accept also large IV_CIPHER values from clients.

Patch V2: Correct comment about normalising ciphers
Patch V3: Correct #ifdef statement
Patch V5: Fix tests with OpenSSL 1.0.2 and libraries missing Chacha
Patch V6: Fix unit tests for mbed tls, which recognises ChaCha20-Poly1305
          only when used with all uppercase, fix missing space in message

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20200312113654.16184-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19546.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-03-27 16:26:29 +01:00
Arne Schwabe
1828f9c199 Move NCP related function into a seperate file and add unit tests
This allows unit test the NCP functions. The ssl.c file has too
many dependencies to make unit testing of it viable.

Patch V2: Removing the include "ssl_ncp.h" from options.c for V2 of
          implement dynamic NCP forces a new version of this patch to
          add the #include in this patch. Merge VS studio file changes
          for ssl_ncp.[ch] into this patch

Patch V3: Regenerate for changes in earlier patches, apply Lev's changes
          to Visual Studio project file

Patch V4: Regenerate to also have the changes of earlier patches.

Patch V5: Fix unit tests for crypto library missing chacha20-poly1305

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Lev Stipakov <lstipakov@gmail.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20200221100746.7065-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19499.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2020-03-09 13:19:50 +01:00
Heiko Hund
e7500875ef
Add gc_arena to struct argv to save allocations
With the private gc_arena we do not have to allocate the strings
found during parsing again, since we know the arena they are
allocated in is valid as long as the argv vector is.

Signed-off-by: Heiko Hund <heiko.hund@sophos.com>
Signed-off-by: David Sommerseth <davids@openvpn.net>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200206132103.15977-4-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19376.html
2020-02-20 16:41:17 +01:00
Heiko Hund
870e2405e2
argv: do fewer memory re-allocations
Prevent the re-allocations of memory when the internal argv grows
beyond 2 and 4 arguments by initially allocating argv to hold up to
7 (+ trailing NULL) pointers.

While at it rename argv_reset to argv_free to actually express
what's going on. Redo the argv_reset functionality so that it can
be used to actually reset the argv without re-allocation.

Signed-off-by: Heiko Hund <heiko.hund@sophos.com>
Signed-off-by: David Sommerseth <davids@openvpn.net>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200206132103.15977-3-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19378.html
2020-02-20 16:40:20 +01:00
Heiko Hund
4ed7bf7f94
re-implement argv_printf_*()
The previous implementation had the problem that it was not fully
compatible with printf() and could only detect % format directives
following a space character (0x20).

It modifies the format string and inserts marks to separate groups
before passing it to the regular printf in libc. The marks are
later used to separate the output string into individual command
line arguments.

The choice of 0x1D as the argument delimiter is based on the
assumption that no "regular" string passed to argv_printf_*() will
ever have to contain that byte (and the fact that it actually is
the ASCII "group separator" control character, which fits its
purpose).

This commit has been updated by David Sommerseth based on Arne
Schwabe and his own feedback on the mailing list.

Signed-off-by: Heiko Hund <heiko.hund@sophos.com>
Signed-off-by: David Sommerseth <davids@openvpn.net>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200206132103.15977-2-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19380.html
2020-02-20 16:38:58 +01:00
Antonio Quartulli
6c8b33f289 get rid of 'broadcast' argument when configuring the tun device
The broadcast argument is actually useless as every platform will figure
it out and configure it on its own. We even realized that on linux, if
you configure it wrong, nothing wrong will happen.

At this point, let's make the code cleaner and let's get rid of this
useless argument at all.

This patch just removed any occurrence of 'broadcast'.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20191110124407.8734-1-a@unstable.cc>
URL: https://www.mail-archive.com/search?l=mid&q=20191110124407.8734-1-a@unstable.cc
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-11-10 14:19:49 +01:00
Arne Schwabe
ef2c7b2fa4 Implement unit tests for auth-gen-token
The unit test is breaking the 80 char limit in some places
but the remaining lines it breaks the limit I feel
forcing the 80 char limit will impair readibility

Patch V2: adapt unit tests to other V2 patches
Patch V4: Resolve rebase conflicts
Patch V5: Add \ lost in rebase that broke compilation
Patch V7: Fix unit test failure, try to stay below 80
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20190917123321.15993-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18821.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-10-01 13:23:22 +02:00
Kyle Evans
7e4a261cc9 tests/t_lpback.sh: Switch sed(1) to POSIX-compatible regex.
A test run with FreeBSD PR 229925 'Disallow escaping ordinary
characters in regex(3)' reveals one sed expression that uses the
GNU-extension "\s".

Given that this is the only occurrence and it's a trivial fix,
update it to be POSIX-compatible.

Signed-off-by: Matthias Andree <matthias.andree@gmx.de>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20190906174458.14975-2-matthias.andree@gmx.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18806.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-09-24 23:48:12 +02:00
Antonio Quartulli
6ccb3b2e3a t_net.sh: execute sleep after checking exit code of previous command
Ensure to check the exit code of the mktun command *before* running
sleep, otherwise '$?' will resolve to the exit code of sleep itself,
thus nullifying the check.

Reported-by: Steffan Karger <steffan@karger.me>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20190919202257.19405-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18845.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-09-19 22:36:14 +02:00
Antonio Quartulli
8aa037161f t_net.sh: wait for NO-CARRIER bit to settle before starting test
Interfaces of type tun are marked as NO-CARRIER when no process is
attached to them. However, this bit gets set with some delay after
creation.

For this reason, it is better to wait for the bit to settle before
starting any test, otherwise any timing influence on the test may lead
to inconsistencies due to the NO-CARRIER bit randomly being or not in
the snapshot output taken by t_net.sh.

This patch add a 'sleep 1' command right after creation of the
interface, to give the NO-CARRIER bit a chance to settle.

This issue has been witnessed on a buildbot that is
apparently slowler than average to run the unit tests.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20190919072820.9913-1-a@unstable.cc>
URL: https://www.mail-archive.com/search?l=mid&q=20190919072820.9913-1-a@unstable.cc
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-09-19 09:44:04 +02:00
Arne Schwabe
0d80b562e4 Implement --genkey type keyfile syntax and migrate tls-crypt-v2
This unifies our key generation and also migrates the generation
of the tls-crypt-v2 keys. Since tls-crypt-v2 is not included in any
released version, we remove the the old syntax without compatibility.

PATCH V4: Introduce warning/error when using --secret with --genkey
          Update non code usages to use new --genkey syntax
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20190613134834.5709-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18524.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-07-05 12:32:49 +02:00
Gert Doering
222e691739 Remove cmocka submodule, rely on system-wide installation instead.
We used to ship git submodule instructions to build a local copy of
cmocka in vendor/cmocka/ and use that (if cmake is installed) to build
unit tests.  With the network test driver this turns out to be a
LD_LIBRARY_PATH vs. SUDO complication which is really outweighing the
benefit of a local build today - so, use the system-wide installation
if available (querying pgk-config).  Do not build unit-tests otherwise.

v2: (inspired by patch from David Sommerseth)
  introduce "configure --disable-unit-test" switch
  simplify configure.ac logic
  use CMOCKA_LIBS and CMOCKA_INCLUDE (set by PKG_CHECK)

v3:
  repair conflict with commit 7473f32636
  CMOCKA_INCLUDE is not correct, must be CMOCKA_CFLAGS (see config.status)

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20190623183210.6005-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18570.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-06-24 20:43:58 +02:00
Antonio Quartulli
06f65a0cbf t_net.sh: fixes for the networking test script
1) Building the networking unit-test when SITNL is not enabled does not
make much sense right now.
Make compilation dependent on having SITNL configured.

2) Remove some no-op mock_msg function calls.

3) Remove obsolete comment and declarations

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <20190618163435.26431-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18556.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-06-20 21:01:14 +02:00
Antonio Quartulli
6f89dac0ed t_net.sh: properly perform sudo check and print test steps
The current script is performing a test on the "kill" command, but this
is not useful to the t_net.sh script as it never really executes it.

Rather test that "sudo <unit-test-binary>" really works.

<unit-test-binary> has to be added to the sudoers file if this test
has to be performend unattanded. The path is:
./unit_tests/openvpn/networking_testdriver

On top of that, print a simple OK for every test that is succesful.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20190615230213.14888-2-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18548.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-06-16 10:28:52 +02:00
Antonio Quartulli
1e894662c6 t_net.sh: make bash dep explicit and run only if SITNL is compiled
The t_net script currently has #!/bin/sh but it implicitly assume to
be using bash.
This is fine on most distros, but some do not have sh pointing to bash
by default, thus breaking the script.
Explicitly use bash to avoid failures.

On the other hand, run this unit-test only if SITNL was enabled at
compile time. This test was designed with SITNL in mind and it is
not yet ready for other backends.

Running only when SITNL is enabled implies running on Linux only
therefore we are guaranteed that bash will always work.

While at it, also add a comment as of why the t_client.rc file is
sourced.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20190615230213.14888-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18547.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-06-16 10:16:49 +02:00
Antonio Quartulli
c4d5bcd7c9 unit tests: implement test for sitnl
This patch introduces a new unit test that is not executed
by the cmocka framework, but rather used by a new t_net.sh
bash script.

The idea behind this test is to ensure that invoking sitnl
functions or running iproute commands leads to the same
networking (interface and routing table) state.

To achieve this, the t_net.sh script first runs a binary
implemented invoking sitnl functions and then takes a
"screenshot" of the state. Subsequently a series of
iproute commands, expected to mimic exactly the same behaviour
as the sitnl functions invoked before, are executed.
The final state is then compared with the screenshot
previously taken.

If no mismatching is found, the test is passed.

The current unit_test, however, does not cover all the
sitnl functionalities and it is expected to be extended
in the future.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20181219050118.6568-7-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18027.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-06-06 08:23:48 +02:00
Steffan Karger
90c61ef3df tests: remove dependency on base64
Triggered by the report from Ilya, that if base64 is missing, the tests
would still report success:

  Testing tls-crypt-v2 key generation (max length
metadata)..../t_lpback.sh: base64: not found
  OK
  PASS: t_lpback.sh

The easiest way to fix that, is to remove the dependency on base64 (which
is it's current form wouldn't work on OSX anyway, because their base64
doesn't understand "-w0").

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <E1hON0G-0007yB-3H@sfs-ml-4.v29.lw.sourceforge.com>
URL: https://www.mail-archive.com/search?l=mid&q=E1hON0G-0007yB-3H@sfs-ml-4.v29.lw.sourceforge.com
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-05-10 14:59:09 +02:00
David Sommerseth
19a22ac5a8 build: Package missing mock_msg.h
The mock_msg.h file was not enlisted in the _SOURCES lists in
Makefile.am for the unit tests.  This caused the mock_msg.h file to not
be present in the .tar.gz file created by 'make dist'.

This was not noticed earlier as we haven't really tried much to run git
clone of the cmocka project manually in vendor/ from an unpacked
tarball.

With this fix the cmocka unit tests can also run from tarballs, with
manually extracting/fetching the cmocka source code in vendor/cmocka.

Signed-off-by: David Sommerseth <davids@openvpn.net>

----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----

How to test:

- Create a tarball: make distcheck (or just 'dist')
- Extract openvpn-2.5_git.tar.gz in a clean directory
- cd openvpn-2.5_git/vendor
- git clone https://git.cryptomilk.org/projects/cmocka.git
- ./configure
- make check
- Observe that the cmocka unit tests ran as expected

Depending on the CMake version, you might want to check out cmocka git
commit b2732b52202ae48f; which is the one we use in the git submodule.
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20190417203015.1903-1-davids@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18380.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-05-03 20:40:28 +02:00
Jonathan Tooker
ccb636c751 Fix various spelling mistakes
New patch, omitted changes to copyrights/licenses & changelog.
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20190123201717.15048-1-jonathan@reliablehosting.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18177.html

Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-02-06 19:07:34 +01:00
Lev Stipakov
a3fd78d486 test_tls_crypt.c: fix global-buffer-overflow found by AddressSanitizer
When writing data to buffer we incorrectly specify source length
 - sizeof for pointer returns 8, but actual buffer length is 1.

Fix by replacing empty global string to local string literal and
specifying the correct length.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1548164060-13144-1-git-send-email-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18140.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-01-22 16:44:48 +01:00
Steffan Karger
ef4b8c972f Extend tls-crypt-v2 unit tests
This commit adds two tests for tls-crypt-v2 to verify the client and
server key generation. These are introduced primarily as a regression
test for the off-by-one bug fixed by Arne in tls_crypt_v2_read_keyfile()
recently (no commit hash availble, patch has not been applied yet).

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <E1gjn4T-0003e9-LN@sfs-ml-1.v29.lw.sourceforge.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18095.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2019-01-16 17:54:10 +01:00
Steffan Karger
ff931c5e99
tls-crypt-v2: add script hook to verify metadata
To allow rejecting incoming connections very early in the handshake,
add a --tls-crypt-v2-verify option that allows administators to
run an external command to verify the metadata from the client key.
See doc/tls-crypt-v2.txt for more details.

Because of the extra dependencies, this requires adding a mock
parse_line() to the tls-crypt unit tests.  Also, this turns tls_wrap_free
into a static inline function, so that we don't need to compile in ssl.c
(and all of it's dependencies) with the unit tests.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <1540208715-14044-6-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17789.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
2018-10-26 19:05:25 +02:00
Steffan Karger
19dffdbde0
tls-crypt-v2: implement tls-crypt-v2 handshake
This makes clients send-and-use, and servers receive-unwrap-and-use
tls-crypt-v2 client keys, which completes the on-the-wire work.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <1540208715-14044-5-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17787.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
2018-10-26 19:04:04 +02:00
Steffan Karger
a98a56768f
tls-crypt-v2: add unwrap_client_key
Add helper functions to unwrap tls-crypt-v2 client keys.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <1540208715-14044-3-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17791.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
2018-10-26 18:53:51 +02:00
Steffan Karger
9d59029a08
tls-crypt-v2: generate tls-crypt-v2 keys
As a first step towards a full tls-crypt-v2 implementation, add
functionality to generate tls-crypt-v2 client and server keys.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <1540208715-14044-2-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17792.html
Signed-off-by: David Sommerseth <davids@openvpn.net>
2018-10-26 18:53:44 +02:00
Steffan Karger
b081038c74 Fix mbedtls unit tests
Commit 674b166 ("Fix build warnings related to get_random()") broke the
unit tests for mbedtls, because <mbedtls/cipher.h> was now included via
platform.c -> crypto.h -> crypto_backend.h, but the crypto cflags were
not included for that unit tests.

Since we got rid of --disable-crypto, we can now fix this by simply always
including the CRYPTO_CFLAGS in the TEST_CFLAGS (and the CRYPTO_LIBS in the
TEST_LDFLAGS). This should not only fix this occurrence, but also prevent
similar problems in the future.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>

Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1539153883-15789-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17687.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2018-10-11 16:50:45 +02:00
Antonio Quartulli
5817b49b4c crypto: always reload tls-auth/crypt key contexts
In preparation to having tls-auth/crypt keys per connection
block, it is important to ensure that such material is always
reloaded upon SIGUSR1, no matter if `persist-key` was specified
or not.

This is required because when moving from one remote to the
other the key may change and thus the key context needs to
be refreshed.

To ensure that the `persist-key` logic will still work
as expected, the tls-auth/crypt key is pre-loaded so that
the keyfile is not required at runtime.

Trac: #720
Cc: Steffan Karger <steffan@karger.me>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20180708024517.27108-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17237.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2018-07-24 14:20:46 +02:00
Steffan Karger
a5d35a01dc Add crypto_pem_{encode,decode}()
Needed for tls-crypt-v2, but isolated enough to be reviewed as a separate
patch.

The encode API allocates memory, because it fits our typical gc-oriented
code pattern and the caller does not have to do multiple calls or
calculations to determine the required destination buffer size.

The decode API does not allocate memory, because the required destination
buffer is always smaller than the input buffer (so is easy to manage by
the caller) and does not force the caller to use the heap.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20180722100645.5813-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17284.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2018-07-22 14:22:31 +02:00
Steffan Karger
b7bea782f3 Move file-related functions from misc.c to platform.c
To avoid having to include misc.c - which is a dependency mess - in the
tls-crypt unit tests, move file-handing related functions to platform.c
(which is where other file-related functions already reside).

Note that platform_create_temp_file() needs random.  To avoid including
misc.c in other tests that use platform.c, add a mock get_random().

(Almost every test includes platform.c, because buffer.c depends on it.
That smells like it needs cleanup too, but not in this patch set.)

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20180704175404.22371-1-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17208.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2018-07-05 22:31:21 +02:00
Gert Doering
4376805d8f Add %d, %u and %lu tests to test_argv unit tests.
Some basic integer tests to verify signed, unsigned and
long unsigned (1L) printing.

Signed-off-by: Gert Doering <gert@greenie.muc.de>

Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20180623191538.29317-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17131.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
2018-06-23 21:24:49 +02:00