David Woodhouse [Mon, 11 Mar 2013 14:02:01 +0000 (14:02 +0000)]
Enable shared libopenconnect for Android build
We'll definitely want to use it from Java code for the authentication stage.
Not entirely sure yet how we'll invoke the main loop — perhaps by executing
the openconnect executable, but we *could* also invoke the main loop directly
from a Java process too. That might simplify the issue of protecting the
network sockets.
This ends up pulling libxml into libopenconnect.so, so the openconnect
executable gets it from there. Which isn't an ideal setup for the general
case but it's fairly convenient on Android.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Sun, 10 Mar 2013 21:01:53 +0000 (21:01 +0000)]
Clean up CSD invocation for XML POST
We don't use a CSD trojan to download; we *require* a local wrapper.
Theoretically we ought to be able to invoke a 'real' Cisco hostscan
tool. We ought to fix the command line arguments for that but let's keep
it simple for now. Just keep the command line exactly the same as for
wrapping the trojan, with an empty string where the name of the
downloaded file would be.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Sun, 10 Mar 2013 19:00:04 +0000 (19:00 +0000)]
CSD stub URL is optional
The recent change in commit b6ef1c86b6d29684e5a24b62e19827afafec13ed ('Fix
CSD trojan download') was wrong; for the XML POST case we don't necessarily
get handed a trojan to download. We're expected to have a local 'wrapper'
script which will act like a locally-installed 'hostscan'.
The wait URL *is* required though.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Mon, 4 Mar 2013 00:45:21 +0000 (00:45 +0000)]
Destroy vpninfo->https_cred on failing to create it
If something like certificate setup went wrong, we'd return failure but
*not* destroy the gnutls_certificate_credentials_t that we were
attempting to set up. So a subsequent retry would see that it already
exists, assume it's *fine* and just go ahead and use it. Don't do that.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Mon, 4 Mar 2013 00:28:08 +0000 (00:28 +0000)]
Handle redirects in attempting simple auth GET too
If the XML POST fails and we try a GET, we need to handle redirects for
that too. So re-use the same loop. Except the bit about not allowing local
redirects. Why do we do that for the XML POST case anyway?
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Fri, 22 Feb 2013 12:42:07 +0000 (12:42 +0000)]
Fix hostname canonicalisation to stop breaking certifcate checks
Commit b0b4b34f ('Canonicalise hostname during authentication if necessary')
replaces the hostname with a bare IP address if necessary, so that
reconnecting is guaranteed to get the *same* host from a round-robin and
comparing the SSL cert with its previous SHA1 fingerprint (which is how we
do it for two-stage connection for example from NetworkManager) is
guaranteed to work.
However, this breaks certificate auth when invoked in one-stage mode from
the command line to authenticate *and* actually make the connection. When
vpninfo->hostname is replaced with a bare IP address, that might not
actually be what's listed in the certificate's Subject or Altname fields.
So users have reported a certificate validation failure on *reconnecting*
to the server which was acceptable the first time round when we looked it
up by name.
So, don't actually replace vpninfo->hostname at all. Introduce a new field
vpninfo->unique_hostname which is returned by openconnect_get_hostname(),
and leave vpninfo->hostname as it was.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Kevin Cernekee [Sun, 17 Feb 2013 00:18:07 +0000 (16:18 -0800)]
auth: stoken: Fix handling of "Next TOKENCODE" prompt
This needs to allow for input elements named "answer" instead of
"password", and it needs to check form->message instead of the label
attribute for the "Next TOKENCODE" prompt.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Kevin Cernekee [Sun, 17 Feb 2013 00:18:06 +0000 (16:18 -0800)]
http: Fix redirect handling in auth form loop
The gateway may ask the user to fill out different forms that live at
different URLs, e.g.
GET /+webvpn+/index.html
(returns <form method="post" action="/+webvpn+/index.html"> and
username/password form elements)
POST /+webvpn+/index.html
(returns <form method="post" action="/+webvpn+/login/challenge.html">
and challenge/response form elements)
POST /+webvpn+/login/challenge.html
(returns <auth> node with valid cookie)
The refactored openconnect_obtain_cookie() loop tried to post the
challenge/response data to index.html, preventing successful login. This
patch changes the logic so that it will honor the new "action" attribute
if present.
This probably does not affect XML POST mode, because XML POST <form> tags
do not seem to use attributes.
Reported-by: Fabian Jäger <fabian.jaeger@chungwasoft.com> Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Kevin Cernekee [Sun, 17 Feb 2013 00:18:05 +0000 (16:18 -0800)]
auth: Implement special handling of password fields on XML POST
The Cisco AnyConnect client exhibits some quirky behavior on fields
with certain names:
For "answer", "whichpin", and "new_password", the field is renamed to
"password" in the submission.
For "verify_pin" and "verify_password", the field is omitted entirely.
One might expect the client to perform a comparison to see if the first
password/PIN field matches the verify_* field, but in my testing, I didn't
actually see it doing so.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
2) If the name of the <select> node happens to be "group_list", put the
response in a special <group-select> node right under the <config-auth>
node, instead of putting it under the <auth> node. (These strings are
hardcoded into the Cisco client.)
Reported-by: Fabian Jäger <fabian.jaeger@chungwasoft.com> Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Mon, 4 Feb 2013 15:57:35 +0000 (15:57 +0000)]
Canonicalise hostname during authentication if necessary
Some people have round-robin servers, all addressed by the same hostname
but with different SSL certificates. Where we do the authentication (and
user-interactive approval of certificates) from a GUI via libopenconnect,
or with 'openconnect --authenticate', we end up being given the SHA1 on
the server's certificate and the non-interactive connection is going to
expect to see exactly that certificate. So if there is more than one
result in the original DNS lookup, *change* vpninfo->hostname to hold
the IP address that we actually connected to.
This means that the Host: header in what we send will be the numeric IP
address instead of the hostname, but that doesn't seem to hurt. It could
potentially, theoretically, break virtual hosts but I don't think that
kind of setup could ever existing in practice.
This also works only in the case where we're *not* connecting via a proxy.
We currently let the proxy do the DNS lookups *for* us, and we'd have to
do them locally and then ask the proxy for a connection by IP address
even for the *first* connection.
Kevin Cernekee [Sat, 27 Oct 2012 19:25:50 +0000 (12:25 -0700)]
http: Fix overflow on HTTP request buffers (CVE-2012-6128)
A malicious VPN gateway can send a very long hostname/path (for redirects)
or cookie list (in general), which OpenConnect will attempt to sprintf()
into a fixed length buffer. Each HTTP server response line can add
roughly MAX_BUF_LEN (131072) bytes to the next OpenConnect HTTP request,
but the request buffer (buf) is capped at MAX_BUF_LEN bytes and is
allocated on the stack.
The result of passing a long "Location:" header looks like:
Attempting to connect to server 127.0.0.1:443
SSL negotiation with localhost
Server certificate verify failed: self signed certificate in certificate chain
Connected to HTTPS on localhost
GET https://localhost/
Got HTTP response: HTTP/1.0 301 Moved
Ignoring unknown HTTP response line 'aaaaaaaaaaaaaaaaaa'
SSL negotiation with localhost
Server certificate verify failed: self signed certificate in certificate chain
Connected to HTTPS on localhost
*** buffer overflow detected ***: /scr/openconnect2/.libs/lt-openconnect terminated
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7fd62729b82c]
/lib/x86_64-linux-gnu/libc.so.6(+0x109700)[0x7fd62729a700]
/lib/x86_64-linux-gnu/libc.so.6(+0x108b69)[0x7fd627299b69]
/lib/x86_64-linux-gnu/libc.so.6(_IO_default_xsputn+0xdd)[0x7fd62720d13d]
/lib/x86_64-linux-gnu/libc.so.6(_IO_vfprintf+0x1ae7)[0x7fd6271db4a7]
/lib/x86_64-linux-gnu/libc.so.6(__vsprintf_chk+0x94)[0x7fd627299c04]
/lib/x86_64-linux-gnu/libc.so.6(__sprintf_chk+0x7d)[0x7fd627299b4d]
/scr/openconnect2/.libs/libopenconnect.so.2(openconnect_obtain_cookie+0xc0)[0x7fd62832d210]
/scr/openconnect2/.libs/lt-openconnect[0x40413f]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x7fd6271b276d]
/scr/openconnect2/.libs/lt-openconnect[0x404579]
The proposed fix is to use dynamically allocated buffers with overflow
checking.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
(cherry picked from commit 26f752c3dbf69227679fc6bebb4ae071aecec491)
David Woodhouse [Tue, 12 Feb 2013 23:59:34 +0000 (23:59 +0000)]
Use gnutls_pubkey_verify_data2() where possible
Unfortunately, gnutls_pubkey_verify_data() is deprecated. Which is a pain;
the 'threat model' that led to that deprecation doesn't apply here, and it
just means we have to jump through hoops to find the 'intended' algorithm
instead of letting it be inferred from the signature.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Mon, 4 Feb 2013 15:57:35 +0000 (15:57 +0000)]
Canonicalise hostname during authentication if necessary
Some people have round-robin servers, all addressed by the same hostname
but with different SSL certificates. Where we do the authentication (and
user-interactive approval of certificates) from a GUI via libopenconnect,
or with 'openconnect --authenticate', we end up being given the SHA1 on
the server's certificate and the non-interactive connection is going to
expect to see exactly that certificate. So if there is more than one
result in the original DNS lookup, *change* vpninfo->hostname to hold
the IP address that we actually connected to.
This means that the Host: header in what we send will be the numeric IP
address instead of the hostname, but that doesn't seem to hurt. It could
potentially, theoretically, break virtual hosts but I don't think that
kind of setup could ever existing in practice.
This also works only in the case where we're *not* connecting via a proxy.
We currently let the proxy do the DNS lookups *for* us, and we'd have to
do them locally and then ask the proxy for a connection by IP address
even for the *first* connection.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Kevin Cernekee [Wed, 14 Nov 2012 03:00:25 +0000 (19:00 -0800)]
tun: Kill the tunnel script's process group
When invoked with --script-tun, openconnect starts the tunnel script
via "/bin/sh -c 'SCRIPT'", then sends SIGHUP to the shell's PID when
shutting down. However, non-interactive shells are not guaranteed to
send SIGHUP to any running jobs¹; indeed, the observed behavior on
Linux is that only the shell process receives SIGHUP, and the tunnel
script continues running after openconnect exits.
A quick fix is to set the child's pgid == pid, then send SIGHUP to the
entire process group when we want to shut down.
Kevin Cernekee [Wed, 14 Nov 2012 03:00:24 +0000 (19:00 -0800)]
tun: Don't call tunnel script on reconnect events
If --script-tun is used, the vpnc_script is not invoked for pre-init,
connect, or disconnect events. However, it is invoked on reconnect, and
this may confuse the tunnel script.
Add an extra check to script_config_tun() to make the reconnect behavior
consistent with the behavior of other events.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Thu, 8 Nov 2012 13:31:23 +0000 (13:31 +0000)]
Include version.c from build dir in preference to source dir
This should fix out-of-source-tree builds from a tarball, which
otherwise would use the autogenerated $(objdir)/version.c for building
the library, but the pre-packaged $(srcdir)/version.c for the
executable. This is because the latter was included directly from main.c
by #include "version.c". By changing to #include <main.c> instead, we get
to use the new auto-generated one instead if anything's been changed.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Kevin Cernekee [Sun, 28 Oct 2012 07:41:32 +0000 (00:41 -0700)]
stoken: Fix CSD/stoken interaction
When using CSD, the auth form could be parsed more than once per
connection. Change the accounting so that stoken_tries only gets
incremented if a tokencode is actually generated.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Kevin Cernekee [Sun, 28 Oct 2012 04:36:11 +0000 (21:36 -0700)]
csd: Don't return from run_csd_script() in the forked process
If something in the CSD child process fails, we want it to exit. We
do not want it to return to openconnect_obtain_cookie() and cause two
instances of the latter function to run in parallel.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Kevin Cernekee [Sun, 28 Oct 2012 04:14:07 +0000 (21:14 -0700)]
http: Record the last redirection type
The AnyConnect client uses the redirection type (new host, or just a
new URL on the same host) to figure out whether to use XML POST or
the old urlencoded scheme. Preserve this information for future use.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
projects / users / dwmw2 / openconnect.git / log