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>
Kevin Cernekee [Wed, 24 Oct 2012 04:10:44 +0000 (21:10 -0700)]
library: Add call to change reported OS name
Newer gateways require the client to announce its platform name (win,
mac, linux, linux-64) in the HTTP headers and in the <config-auth>
section of each request. The gateway can be configured to apply different
security policies to different OSes, or even completely block access to
OSes that are not on the "approved" list.
Therefore, it is useful to be able to adjust the OS name that is reported
to the gateway.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Kevin Cernekee [Fri, 26 Oct 2012 04:53:10 +0000 (21:53 -0700)]
auth: Parse the new server response format
Newer AnyConnect installations use a different XML document tree
to pass information to the client. This patch allows OpenConnect
to parse the new format, and attempts to document both the old
format and the new format in the comments.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Kevin Cernekee [Fri, 26 Oct 2012 04:16:57 +0000 (21:16 -0700)]
auth: Remove obsolete trace message from parse_form()
parse_form() used to add hidden form fields to the query string on
the spot, but since it doesn't anymore, it should not display the
"Fixed options" trace message.
After making this change, we can remove the request / request_body
arguments.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Kevin Cernekee [Sat, 27 Oct 2012 19:25:50 +0000 (12:25 -0700)]
http: Fix overflow on HTTP request buffers
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>
Kevin Cernekee [Sat, 13 Oct 2012 20:06:18 +0000 (13:06 -0700)]
stoken: Fill in "password" fields with a generated tokencode
If the gateway prompts for a password and soft token information is
available, generate a tokencode and mark the form field as OPT_STOKEN
so the user is not prompted for a password.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Kevin Cernekee [Sat, 13 Oct 2012 19:31:18 +0000 (12:31 -0700)]
stoken: Implement new auth form to gather soft token information
If the user has asked to use a soft token, libopenconnect will prompt
for devid/pass/pin (as necessary) to unlock the soft token, prior to
the initial server connection. If the user aborts, soft token mode will
be disabled and the user will need to enter his tokencode by hand.
Manual entry could be useful for e.g. activating a new token.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Kevin Cernekee [Sat, 13 Oct 2012 17:46:18 +0000 (10:46 -0700)]
stoken: Add software token functions to library API; bump to v2.1
openconnect_has_stoken_support(): returns 1 if the library was linked
with libstoken.
openconnect_set_stoken_mode(): enables/disables tokencode generation,
and tells the library how to locate the seed. Unless this function is
called, the library will not try to use a soft token.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Kevin Cernekee [Sun, 30 Sep 2012 03:02:35 +0000 (20:02 -0700)]
Delete references to long-removed SecurID code
Commit d707fc524 (Clean up auth form handling) removed securid.c and
the tokencode+PIN prompts, but the docs and headers still refer to these
features.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
projects / users / dwmw2 / openconnect.git / log