Kevin Cernekee [Sun, 23 Feb 2014 02:21:39 +0000 (18:21 -0800)]
dtls: Align new-tunnel rekey behavior with Cisco clients
Cisco clients do not appear to send a new DTLS master secret; they just
re-handshake every <n> minutes according to the gateway's rekey interval.
Change openconnect so that it only sets the master secret once upon
creation of the library instance, not in cstp_reconnect(). This also
fixes several other issues:
- Changing the DTLS master secret on sessions that have rekeying disabled
(because (last_rekey + rekey < time(NULL) + 300) is pretty much always
true if rekey == 0)
- Failing to force a DTLS rehandshake after sending a new master secret
through a CSTP reconnection request, if DTLS uses REKEY_SSL
- Incorrectly bumping dtls_times.last_rekey on any successful CSTP
reconnection
Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Kevin Cernekee [Mon, 17 Feb 2014 20:22:52 +0000 (12:22 -0800)]
www: Don't ignore groff errors
Since most shells disable pipefail by default, errors parsing the man
page into HTML can be silently ignored. This results in manual.html
showing a header and a footer, but no content.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
David Woodhouse [Thu, 6 Mar 2014 08:46:39 +0000 (08:46 +0000)]
Fix GnuTLS 2.x build
Commit c7077b96b ("Do not abort when GnuTLS reports unclean shutdown")
introduced a build failure for GnuTLS 2.x which lacks the
GNUTLS_E_PREMATURE_TERMINATION error that we check for. Instead, it
returns GNUTLS_E_UNEXPECTED_PACKET_LENGTH.
Since the offending server turned out not to be an AnyConnect server at
all in the end, gracefully handling the unclean SSL shutdown probably
isn't strictly necessary — although it remains a good idea on general
principles. But it's not *so* important that I'm going to jump through
hoops to do it even for users who are still using GnuTLS 2.x.
So just put the check inside #ifdef GNUTLS_E_PREMATURE_TERMINATION, and
if we later find an *AnyConnect* server which displays this behaviour and
requires us to cope with it, we can find a better solution. Which might
just be to deprecate GnuTLS 2.x entirely... ☺
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Nikos Mavrogiannopoulos [Wed, 5 Feb 2014 21:46:52 +0000 (22:46 +0100)]
Advertise the hostname we connect using SNI.
This would provide the server with more information on the
connection that can be used to distinguish between different
servers (https and vpn), or even different vpn server configurations.
It is conditionally enabled when compiled with gnutls 3.2.9 or later
where the %COMPAT keyword ensures that the client hello size is
outside the F5 firewall black hole range.
[dwmw2: Make string_is_hostname() accept NULL, cosmetic changes]
Signed-off-by: Nikos Mavrogiannopoulos <nmav@gnutls.org> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Antonio Borneo [Sun, 8 Dec 2013 04:29:32 +0000 (12:29 +0800)]
Fix --os=win
Running OpenConnect with flag "--os=win" we get
vpninfo->csd_xmltag == "csd"
In fact, in library.c we have:
85 else if (!strcmp(os, "win"))
86 vpninfo->csd_xmltag = "csd";
In current code, the case (vpninfo->csd_xmltag == "csd") is
grabbed by previous "else if()" condition and not evaluated
anymore to extract "stuburl", "starturl" and "waiturl".
Split the "else if" in independent statement.
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Tue, 18 Feb 2014 16:43:40 +0000 (16:43 +0000)]
Add RFC4226 HOTP token support
This isn't really complete since it doesn't handle the token counter
properly. It relies on being given the token counter along with the
secret key, and there's no way to save the new value when we're done.
We could perhaps add a library function to write the token counter back,
and rely on the library user to manage the file storage containing the
counter.
Or maybe we want to use libpskc and allow the PSKC file to be specified,
then we can update that file directly.
A UI tool might also want to store the PSKC data in something like the
keyring instead of a simple file, so in that case the library should
probably allow for a callback which provides the new PSKC data rather
than unconditionally writing it to a file.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Nikos Mavrogiannopoulos [Mon, 17 Feb 2014 13:02:24 +0000 (14:02 +0100)]
When CSTP rehandshake or reconnection succeeds, reconnect DTLS.
That only occurs when DTLS doesn't have it's owner timer and
mechanism for rehandshake. In that case we replicate the Anyconnect
clients' behavior, by reconnecting DTLS just after the CSTP reconnect.
Kevin Cernekee [Sat, 15 Feb 2014 20:22:11 +0000 (12:22 -0800)]
Sanity-check CSTP and DTLS rekey times
Just set REKEY_NONE if the rekey time is zero or negative, so that we
don't need to test (ka->rekey && ka->rekey_method != REKEY_NONE) in
multiple locations.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Kevin Cernekee [Sat, 15 Feb 2014 23:09:10 +0000 (15:09 -0800)]
cstp: Make sure outbound packets are sent over CSTP if DTLS is down
As of commit 6a3ad9877 (Kill new_dtls_* variables), it is no longer safe
to assume that (dtls_fd != -1) means that the DTLS connection is able
to pass traffic; it might still be handshaking. Instead, we should check
vpninfo->dtls_state.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Kevin Cernekee [Sun, 19 Jan 2014 19:45:01 +0000 (11:45 -0800)]
android: Use make-standalone-toolchain.sh from NDK
This helps avoid having to hardcode internal NDK paths in our build
scripts, making them less fragile. It also lets us use a combined
sysroot for all dependencies, as the local copy of the toolchain is
guaranteed to be writable. Net result is that packages can generally
just invoke plain $(CC) without --sysroot, -I, or -L flags.
For background info, see "the easy way" in
$(NDK)/docs/STANDALONE-TOOLCHAIN.html
Also, after openconnect is built, the user is left with a new toolchain
containing many helpful libraries. It can be used to cross-compile other
programs just by setting $PATH and --host= with appropriate values.
The change was successfully compile-tested with the following NDK releases:
8c, 8d, 8e, 9, 9b, 9c. For each release, ARM/x86/MIPS targets were built
on Linux using both x86 and x86_64 host binaries. OSX was not tested.
NDK releases <= 8b were also tested, but they showed a large number of
unrelated build failures due to factors like:
- Shipping a version of binutils that doesn't recognize the "rrx(s)"
instruction in gmp
- Calling the x86 binary "i686-android-linux-gcc" instead of
"i686-linux-android-gcc"
- Using binutils (bfd) ld instead of gold for x86, so the final link
hits the same zlib dependency bug that we see on MIPS
- Lack of support for the android-14 (4.0/ICS) ABI
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Kevin Cernekee [Sun, 9 Feb 2014 05:44:52 +0000 (21:44 -0800)]
android: Explicitly disable symbol versioning
On current Android builds, autotools fails to pass --sysroot to one of
the link commands, causing the test to fail and symbol versioning to be
disabled:
configure:12875: arm-linux-androideabi-gcc -shared -fPIC -DPIC conftest.o -v -Wl,-soname -Wl,conftest -o conftest -Wl,-M -Wl,conftest.map
[...]
/opt/android-ndk-r9c/toolchains/arm-linux-androideabi-4.8/prebuilt/linux-x86_64/bin/../lib/gcc/arm-linux-androideabi/4.8/../../../../arm-linux-androideabi/bin/ld: error: cannot open crtbegin_so.o: No such file or directory
[...]
collect2: error: ld returned 1 exit status
configure:12878: $? = 1
configure:12885: result: no
This is somewhat fortuitous, because it wouldn't work anyway.
__gmp_binvert_limb_table is accessed from $GMP/mpn/arm/mode1o.asm using
a homebrew PIC implementation. This causes it to generate a relocation
entry when linked into libopenconnect.so. The Android ldso (via
soinfo_elf_lookup() in Bionic) only allows global and weak symbols to
match relocation entries, so if __gmp_binvert_limb_table gets marked as a
local symbol by our version script, the program will fail to execute:
soinfo_link_image(linker.cpp:1673): could not load library
"libopenconnect.so.3" needed by "./openconnect"; caused by
soinfo_relocate(linker.cpp:1013): cannot locate symbol
"__gmp_binvert_limb_table" referenced by "libopenconnect.so.3"...
CANNOT LINK EXECUTABLE
With no linker script, it remains a global symbol and there is no
problem. Manually hexediting the library to switch the binding between
STB_LOCAL and STB_GLOBAL confirms that there is an issue with this one
symbol.
When make-standalone-toolchain.sh is used to build the library, the
--sysroot problem disappears because it is set implicitly, so symbol
versioning is enabled and that reveals the relocation problem. Thus,
symbol versioning needs to be forced off (or libgmp needs to be fixed)
before we can safely use make-standalone-toolchain.sh.
Kevin Cernekee [Sun, 9 Feb 2014 20:02:12 +0000 (12:02 -0800)]
cstp_reconnect: Don't sleep if the user terminated the connection
If openconnect_make_cstp_connection() fails due to user cancellation,
we don't want cstp_reconnect() to sit around waiting for 10+ seconds.
Instead, return immediately so the mainloop can clean up.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Kevin Cernekee [Thu, 30 Jan 2014 04:10:50 +0000 (20:10 -0800)]
http: Don't retry on user cancellation
We aren't currently checking to see if do_https_request() failed due to
an actual network error or a user cancellation. So if the library tries
to connect to an unreachable host, and the user hits Cancel before the
operation times out, we will still try falling back to non-xmlpost mode:
$ ./openconnect 1.2.3.4
POST https://1.2.3.4/
Attempting to connect to server 1.2.3.4:443
^CSocket connect cancelled
Failed to connect to host 1.2.3.4
Failed to open HTTPS connection to 1.2.3.4
GET https://1.2.3.4/
Attempting to connect to server 1.2.3.4:443
^CSocket connect cancelled
Failed to connect to host 1.2.3.4
Failed to open HTTPS connection to 1.2.3.4
Failed to obtain WebVPN cookie
Add a new check and return the appropriate error code for "user cancelled"
to openconnect_obtain_cookie().
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Kevin Cernekee [Tue, 4 Feb 2014 03:14:29 +0000 (19:14 -0800)]
Add liboath version check
Versions prior to 1.12.0 are missing oath_base32_decode(), resulting in an
error:
CC libopenconnect_la-library.lo
library.c: In function 'set_oath_mode':
library.c:475:3: error: implicit declaration of function 'oath_base32_decode' [-Werror=implicit-function-declaration]
library.c:475:3: warning: nested extern declaration of 'oath_base32_decode' [-Wnested-externs]
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Nikos Mavrogiannopoulos [Sun, 16 Feb 2014 08:44:19 +0000 (09:44 +0100)]
Do rehandshake on the DTLS channel as well.
When we receive "X-DTLS-Rekey-Method: ssl" do a rehandshake
on the DTLS channel as well. Currently this header is only
sent by ocserv, and by using that method we rekey without reconnecting
the DTLS channel.
David Woodhouse [Fri, 14 Feb 2014 11:49:34 +0000 (11:49 +0000)]
Do not abort when GnuTLS reports unclean shutdown
Some servers are observed to send an HTTP 1.0 response and then just kill
the socket. This is horrid, but we need to cope. If we get the
GNUTLS_E_PREMATURE_TERMINATION "error" when trying to read an empty
HTTP body, we shouldn't abort the connection attempt. We should just
treat it as EOF.
David Woodhouse [Fri, 14 Feb 2014 01:03:20 +0000 (01:03 +0000)]
Fix crash with -C option
Commit 1df2832e8 ("Fix a few minor memory leaks") turned vpninfo->cookie
into an "owned" script... except for where it comes from argv[]. Freeing
that hurts.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Jeremy Visser [Wed, 15 Jan 2014 13:27:14 +0000 (00:27 +1100)]
Support IPv6 split tunnelling
First step is to send the "X-CSTP-Full-IPv6-Capability: true" header,
which causes the ASA to send the extra headers we need to support split
tunnelling.
Instead of receiving these headers (where a default route is implied):
Because the address and netmask are being specified in the same header,
this is the reason for the slight logic change with the
"X-CSTP-Address-IP6" header parsing.
Signed-off-by: Jeremy Visser <jeremy@visser.name> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Fri, 14 Feb 2014 00:01:02 +0000 (00:01 +0000)]
Move Windows interface name matching into search_taps() function
This simplifies the return value from the callback since there is never
a "ignore this one and try me with the next one" option. It can just return
-1 for failure, or a handle.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Thu, 13 Feb 2014 23:43:54 +0000 (23:43 +0000)]
Move set_tun_mtu() into os_setup_tun()
There's no real reason for this to be in openconnect_setup_tun_device()
after the vpnc-script is run. That's just a historical artifact because
we *used* to attempt to do IP configuration ourselves if there was no
vpnc-script, and we used to do the MTU setup even if there *was*.
In commit ad9c6573c ("Don't attempt to configure Legacy IP address on
tun device.") it was changed to run the script, then set the MTU anyway
just in case the script didn't get it right. But the MTU could actually
have been set much sooner.
In fact it probably doesn't need to be done at all these days, since
nobody should still be using a vpnc-script that old. But it doesn't hurt.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Wed, 12 Feb 2014 15:56:06 +0000 (15:56 +0000)]
Fix overflow warning in dumb_socketpair() on Win64
The SOCKET type is a pointer, although in practice what's returned really
does look like a file descriptor. It's a low-valued integer such as 0x23
under Wine, 0x54 under Windows 7 in my testing.
The INVALID_SOCKET error return from socket() is defined as (SOCKET)(~0),
or 0xFFFFFFFFFFFFFFFF on Win64. Thus we get errors when assigning it to
an int in the socks[] array. So use -1 here instead.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Wed, 12 Feb 2014 15:53:16 +0000 (15:53 +0000)]
Use intptr_t for intermediate cast to gnutls_transport_ptr_t
In Win64, 'long' is still only 32 bits, and isn't enough to avoid the
"cast to pointer from integer of different size" warning. This kind of
braindamage is why intptr_t exists, so use it.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Tue, 11 Feb 2014 12:41:51 +0000 (12:41 +0000)]
Abstract select() and FD_SET handling
This should let us make the mainloop work for Windows, where we can't just
select() on the tun device file descriptor. Or indeed *get* a proper file
descriptor for the tun device, AFAICT.
It might also let us use epoll() etc. if we wanted to.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Tue, 11 Feb 2014 09:28:42 +0000 (09:28 +0000)]
Fix setenv() with value==NULL on Windows
Also comment about putenv() taking a *copy* of the string. That is
Windows-specific behaviour and not POSIX-compliant, so we don't want
anyone thinking it's reasonable to do it elsewhere:
Tested thus, under wine and Windows 7:
int main(void)
{
char foo[]="FOO=bar";
char *bar;
putenv(foo);
printf("FOO is: %s\n", getenv("FOO"));
foo[5] = 'f';
printf("FOO is: %s\n", getenv("FOO"));
putenv("FOO=");
printf("FOO is: %s\n", getenv("FOO"));
}
With the following results:
FOO is: bar
FOO is: bar
FOO is: (null)
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Antonio Borneo [Sun, 9 Feb 2014 06:16:47 +0000 (14:16 +0800)]
fix bug in run_csd_script()
run_csd_script() uses proxy_write() to write the CSD script
to file.
From commit 433132b90473538fa46fb6934ef8f7b7f36447b5
"Use send() and recv() for proxy communication (for MinGW's benefit)"
proxy_write() replaces write() with send(), incompatible with
run_csd_script().
Got error:
Failed to write temporary CSD script file:
Socket operation on non-socket
Replace proxy_write() with write() directly in run_csd_script().
Run only a simple check that write() really writes all the buffer.
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
projects / users / dwmw2 / openconnect.git / log