David Woodhouse [Fri, 24 May 2013 22:42:14 +0000 (23:42 +0100)]
Close https connection when falling back to non-xmlpost mode
Some servers will continue to respond with a redirect, even redirecting a
request back to the *same* URL that was requested, unless we close the
connection and try again.
Red Hat bug #964650.
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse [Tue, 21 May 2013 07:45:50 +0000 (08:45 +0100)]
Close HTTPS socket after various errors
This avoids leaving the socket in an unknown state. We were attempting to
send a request with a stale or out-of-sync socket, and that would make
the *next* request fail too, when it should have opened a new connection
for itself.
We should also make do_https_request() notice that and actually retry for
itself when it fails to even *send* the request, if it was re-using an
already open socket. But currently it doesn't *know* if it's re-using a
socket so that'll require a little more work.
Kevin Cernekee [Mon, 25 Mar 2013 01:14:02 +0000 (18:14 -0700)]
android: Remove dependency on files outside the openconnect repo
The Android build tries to run "git clone --reference ../../../gnutls".
This only works if the user happens to have another copy of the gnutls
repo in the right place.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Kevin Cernekee [Mon, 25 Mar 2013 01:13:58 +0000 (18:13 -0700)]
Fix token-related command line options
Aliasing --stoken to --token-secret is not effective because token_mode
does not get set. Might as well just drop --stoken.
For --token-mode, change "stoken" to "rsa" to make the UI more intuitive:
liboath provides TOTP token support, and libstoken provides RSA token
support.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Kevin Cernekee [Mon, 25 Mar 2013 01:13:57 +0000 (18:13 -0700)]
Get rid of LIBSTOKEN_HDR and LIBOATH_HDR
Unlike libproxy, these two libraries have well-defined names for their
respective header files. So include the headers by name, and use
HAVE_LIBSTOKEN / HAVE_LIBOATH for compile-time tests.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Kevin Cernekee [Mon, 25 Mar 2013 01:13:56 +0000 (18:13 -0700)]
Tweak liboath ./configure help text for consistency
Was:
--with-gnu-ld assume the C compiler uses GNU ld [default=no]
--with-sysroot=DIR Search for dependent libraries within DIR
(or the compiler's sysroot if not specified).
--without-libproxy Build without libproxy library [default=auto]
--without-stoken Build without libstoken library [default=auto]
--without-liboath Build without liboath library (default: test)
Now:
--with-gnu-ld assume the C compiler uses GNU ld [default=no]
--with-sysroot=DIR Search for dependent libraries within DIR
(or the compiler's sysroot if not specified).
--without-libproxy Build without libproxy library [default=auto]
--without-stoken Build without libstoken library [default=auto]
--without-liboath Build without liboath library [default=auto]
Signed-off-by: Kevin Cernekee <cernekee@gmail.com> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
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>
projects / users / dwmw2 / openconnect.git / log