David Woodhouse [Tue, 12 May 2020 21:27:24 +0000 (22:27 +0100)]
Handle ConfRej for anything that needs it.
If get a ConfReq with anything we don't want or understand — and that
includes bloody VJ header compression, since I'm not completely batshit
insane — send a ConfRej.
Do this by building up the options to be rejected in an oc_text_buf as
we go, then rejecting that set if it's non-empty once we get to the end.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Tue, 12 May 2020 21:12:25 +0000 (14:12 -0700)]
Fix un-HDLC corner cases
1) The initial 0x7e is optional, the final 0x7e is not (was reversed).
2) Dangling escape can occur even when we haven't run out of buffer. 0x7d 0x7e is an invalid sequence.
While not breaking…
3) 0x7d can be the “target” of an escape (0x7d 0x7d → 0x5d)
4) 0x5d as the “target” of an escape (0x7d 0x5d → 0x7d) doesn't indicate a new escape
Daniel Lenski [Tue, 12 May 2020 06:16:34 +0000 (23:16 -0700)]
simplify PPP header checking
There's no point whatsover to checking if the server is doing ACCOMP/PFCOMP
as negotiated:
- Even if negotiated, they're optional.
- Even if *not* negotiated, they're unambiguous.
- Either way, it's much easier just to ignore the negotiated options.
“Be liberal in what you accept, and conservative in what you send.”
Some day I will acquire a time machine, travel back to 1993, and ask the
designers of PPP not to add meaningless boilerplate bytes to their protocol
in such a uniquely strange-yet-approachable way that compels future implementers to
reinvent clever ways of dealing with them hundreds of times.
David Woodhouse [Mon, 11 May 2020 12:29:28 +0000 (13:29 +0100)]
Drop hdlc and we_go_first args from openconnect_ppp_new()
We should always go first for *our* outbound ConfReqs, not wait for the
server to go first. And HDLC can be inferred from the encap mode, to
which we can add PPP_ENCAP_F5_HDLC.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Mon, 4 May 2020 10:56:36 +0000 (11:56 +0100)]
Use shorter pathname for COPR RPM build
If the path of SOCKET_WRAPPER_DIR is too long, it doesn't fit in the
sun_path field of the sockaddr_un, and libsocket_wrapper gets very
unhappy, reporting 'Too many unix sockets'. Despite actually only ever
trying *one* path over and over again 1024 times due to truncation.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Thu, 30 Apr 2020 16:41:20 +0000 (17:41 +0100)]
Attempt to fix EPEL8 build
Use --without-gnutls-version-check; as if EPEL8 *does* get the fix for
the zero-client-random bug it will probably come without a version bump.
This also partially reverts commit 68641c0393e which disabled the use of
--with-default-gnutls-priority on *all* EPEL versions, but since I wasn't
building for EPEL8 at that point I don't think it was done for EPEL8
specifically, and can probably be restored.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Wed, 29 Apr 2020 21:34:43 +0000 (14:34 -0700)]
add and fix a few changelog entries
One significant user-facing entries left out of v8.09 changelog:
* modernized Juniper TNCC script
Two were labeled as being in v8.08 when in fact they weren't merged until v8.09:
* GlobalProtect MRs (!90, !93, !95)
* disabling of Nagle's algorithm for TLS sockets
Use OpenSSL X509_check_host() and X509_check_ip() correctly.
These functions return 1 for a successful match, 0 for a failed match,
-1 for an internal error, or -2 if the certificate is malformed.
OpenConnect has been treating any value other than zero as a success,
meaning that an attacker who could get a trusted CA to issue an invalid
certificate (on which the ASN.1 decoder fails, for example), could use
that to assume *any* identity.
Daniel Lenski [Thu, 23 Apr 2020 17:30:40 +0000 (10:30 -0700)]
fix IPv4 split-{in,ex}clude routes with misspecified host bits
Some VPN platforms (GlobalProtect, apparently) allow administrators to input
such non-canonical IPv4 routes, and some routing configuration utilities
(apparently *not* iproute2) simply do not accept such non-canonical IPv4
routes.
An example of the confusion this can cause:
https://lists.infradead.org/pipermail/openconnect-devel/2020-April/005665.html
The robustness principle suggests that the best thing to do here is to fix
these routes, but complain about them while we're at it.
David Woodhouse [Sat, 25 Apr 2020 08:54:28 +0000 (09:54 +0100)]
Fix dependencies and tests/configs/server-cert.prm to dist
Strictly, *break* the dependencies. We don't want server-cert.pem being
gratuitously rebuilt. It's breaking the CI because the file isn't pristine
when 'make tmp-distdir' runs.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Wed, 22 Apr 2020 14:30:11 +0000 (15:30 +0100)]
Update SoftHSM token import scripting and reimport
The slot numbers get reassigned now.
The RSA key modulus had been imported with a leading zero bytes,
confusing SoftHSM when it tried to perform CKM_RSA_PKCS or
CKM_RSA_PKCS_PSS signatures.
Daniel Lenski [Tue, 21 Apr 2020 20:03:42 +0000 (13:03 -0700)]
set TCP_NODELAY unconditionally on TCP/TLS sockets
This replaces 67162301, where I tried to only set `TCP_NODELAY` when using
TLS for the tunnel transport.
See https://gitlab.com/openconnect/openconnect/-/merge_requests/89#note_328398311
for why setting it unconditionally is probably the best choice for openconnect.
Daniel Lenski [Sun, 19 Apr 2020 01:13:39 +0000 (18:13 -0700)]
URL-decode GlobalProtect login response fields
The usage of URL encoding in the fields sent by GP servers here is
inconsistent, but in particular the value "%28empty_domain%29" keeps popping up
in places where the server expects "(empty_domain)" (like the stupidly redundant
logout operation). So we do this to be safe and to ensure logout succeeds.
Daniel Lenski [Tue, 21 Apr 2020 01:51:41 +0000 (18:51 -0700)]
periodic HIP fix: ping /ssl-vpn/hipreportcheck.esp at specified interval no matter what
This is a fix for the very subtle regression between v8.05 and v8.08 described in this thread: https://lists.infradead.org/pipermail/openconnect-devel/2020-April/005609.html
- Server never asks for HIP report submission, and no HIP script specified
with `--csd-wrapper`
- v8.05 successfully rekeys 1 minute before server-specified rekey interval,
and continues successfully
- v8.08 appears to successfully rekey, but then gets forced off one minute
later
- Only apparent difference between the two is that v8.05 re-pings
/ssl-vpn/hipreportcheck.esp every time it gets the config
(/ssl-vpn/getconfig.esp), whereas v8.08 only pings it exactly once.
The bottom line is that _even if_ we have no mechanism to actually submit a
HIP report (no `--csd-wrapper`) and _even if_ we think the server will say
"no HIP report needed" every time we check, in order to keep the server from
kicking the client off, we should still…
* ping /ssl-vpn/hipreportcheck.esp at the interval (specified by the portal or `--force-trojan` or 1 hour default)
* ping /ssl-vpn/hipreportcheck.esp every time we re-fetch the config (/ssl-vpn/getconfig.esp) in order to rekey or reconnect
Daniel Lenski [Mon, 20 Apr 2020 17:14:50 +0000 (10:14 -0700)]
Fix print_supported_protocols and print_supported_protocols_usage
These were broken in 7cb8996e21b442c4ec60ce25c87e8a69516fac17, when the
empty sentinel value at the end of the array was removed, without changing
the way these functions iterate over that array.
For some reason, this continues to work on Linux (probably due to `calloc`
allocating more zeroed bytes than we request, in
`openconnect_get_supported_protocols`), but is causing the expected SIGSEGV on
Solaris:
https://lists.infradead.org/pipermail/openconnect-devel/2020-April/005640.html
Fix:
- Modify `print_supported_protocols` and `print_supported_protocols_usage` to
rely on the length returned by `openconnect_get_supported_protocols`.
- Restore the sentinel value at the end of the array returned by
`openconnect_get_supported_protocols`, to preserve ABI compatibility for
other users who may depend on this sentinel.