Daniel Lenski [Fri, 23 Apr 2021 02:49:53 +0000 (19:49 -0700)]
GlobalProtect IPv6 ESP support
ESP for IPv6 is be initiated in the same way as for Legacy IP: send a
ping (ICMPv6 echo request) to the "magic address" from <gw-address-v6>,
with the same "magic payload."
With a hop limit of 128 and random ICMPv6 ID (I think), this exactly
matches what the Windows client sends out. (Good job by me with the PAN
GlobalProtect mind-meld. 🧠😝)
Based on my testing, it appears that the resulting tunnel works for
*both* IPv4-over-ESP and IPv6-over-ESP. (Whew! No need to do 2 separate
sets of "magic pings.") It appears that when `<gw-address-v6/>` is
present in the config, we *must* send the magic pings via ICMPv6 to that
address (rather than ICMPv4 to the `<gw-address/>`) in order for the
IPv6-over-ESP tunnel to work.
Given the pre-existing weirdness of GP ESP, it could be worse. ¯\_(ツ)_/¯
[ dwmw2: Fix type aliasing abuse, update for new ip_info handling ]
From the original separate commit fixing the aliasing issues:
I believe we get away with it for the address fields and the icmph
because all those structures have uint16_t members, and the compiler
realises that in valid C code, those *might* alias to the uint16_t*
argument to csum_partial().
But the pseudo header, being an array of bytes in Dan's original
version, couldn't *possibly* have been what csum_partial() was going to
look at, so the compiler was free to reorder the writes into it so they
happen *after* the call to csum_partial(). With predictably unhappy
results.
Setting it up as a type which some uint16_t members would make it work,
but really there's not a lot of point. Just add the length and the
next_header field directly to the checksum.
Signed-off-by: Daniel Lenski <dlenski@gmail.com> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Tue, 27 Apr 2021 21:48:23 +0000 (14:48 -0700)]
GP config: hush warning about unknown <quarantine>no</quarantine>
Commit 958da82445d ("Warn if <quarantine> is set in GlobalProtect XML
config") attempted to avoid emitting the warning for the value "no".
But the way the condition was constructed, it left the "quarantine"
node unhandled and triggered the later 'else' condition reporting it
as an 'Unknown GlobalProtect config tag'.
Fix it so that it really does silently ignore the "no" case.
Daniel Lenski [Fri, 23 Apr 2021 01:58:03 +0000 (18:58 -0700)]
Split ESP checksum functions into csum_partial and csum_finish
csum_partial() feeds more uint16_t words into the checksummer;
csum_finish() does the final condensation down to uint16_t, and
conversion to network order.
[dwmw2: Note this still doesn't cope with odd-sized packets but
we don't care for now.]
David Woodhouse [Tue, 27 Apr 2021 23:14:52 +0000 (00:14 +0100)]
Attempt to allow Fortinet reconnect over TCP
We can't just reconnect; we have to fetch the XML config again. Even that
isn't always sufficient; it seems the server only allows *one* reconnect
before it starts to fail.
The *first* reconnection shows this in server logs:
[2326:root:0]ipcp: down ppp:0x7f1e1d218000 caller:0x7f1e1d15f900 tun:37
[2326:root:78]sslvpn_ppp_deassociate_fd_to_ipaddr:318 deassociate 10.212.134.200 to tun (ssl.root:37)
[2326:root:78]tunnel is down, wait for next connection.
[2326:root:78]sslvpn_release_dynip:1309 free app session, idx[0]
...
[2326:root:79]req: /remote/fortisslvpn_xml
[2326:root:79]deconstruct_session_id:426 decode session id ok, user=…
[2326:root:79]deconstruct_session_id:426 decode session id ok, user=…
[2326:root:79]sslvpn_reserve_dynip:1275 tunnel vd[root] ip[10.212.134.200] app session idx[0]
[2326:root:79]req: /remote/sslvpn-tunnel
[2326:root:79]sslvpn_tunnel_handler,52, Calling rmt_conn_access_ex.
[2326:root:79]deconstruct_session_id:426 decode session id ok, user=…
[2326:root:79]sslvpn_tunnel_handler,153, Calling tunnel.
[2326:root:79]tunnelEnter:498 0x7f1e1d15f900:0x7f1e1d1e9000 sslvpn user[dwmw2],type 1,logintime 0 vd 0
[2326:root:79]sconn 0x7f1e1d15f900 (0:root) vfid=0 local=[178.238.156.110] remote=[90.155.92.213] dynamicip=[10.212.134.200]
[2326:root:79]Prepare to launch ppp service...
The *second* reconnection doesn't say anything about waiting for next
connection:
David Woodhouse [Tue, 27 Apr 2021 23:10:31 +0000 (00:10 +0100)]
Abort if PPP transport is closed in PPPS_ESTABLISH
The Fortinet server just closes the connection on us when it screws up
reconnection and thinks it can't allocate an IP address. Don't retry
ad infinitum; give up.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Tue, 27 Apr 2021 23:08:45 +0000 (00:08 +0100)]
Only set ip_info addresses from PPP if they aren't already set
Fortinet reconnection, in particular, didn't like the addr6 being set
because it *isn't* set in the new ip_info we receive in XML; only the
netmask6 is set there.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Tue, 27 Apr 2021 18:20:42 +0000 (11:20 -0700)]
Fix Fortinet IPv6 config and add tests for it
1. Fortinet IPv6 config uses 'prefix-len' attributes where the Legacy IPv4
config uses 'mask' attributes.
2. Also, we need to set ip_info.netmask6 instead of ip_info.addr6 if it
includes a prefix-len (for compatibility with the vpnc-script, which was
originally written based on Cisco's dichotomy here).
3. Align the cstp_option name, "ipaddr6", with other protocols (cf. 792647ee1b9a1eca9cffc79dab746f8273d2279a).
4. Add IPv6 config to Fortinet test server:
David Woodhouse [Tue, 27 Apr 2021 12:05:01 +0000 (13:05 +0100)]
Add IPv6 support for Fortinet
The Fortinet SSL VPN does not support dual stack tunnels. If you connect
over IPv6 you get only IPv6 in the tunnel. And if you connect over Legacy
IP you only get Legacy IP in the tunnel. This is even more insane than
the Pulse abomination which only lets you use the same protocol for ESP
but at *least* let you send the other protocol over the TCP connection.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Fri, 23 Apr 2021 23:39:04 +0000 (16:39 -0700)]
F5, Fortinet: ignore errors in landing page once we've got a cookie
Like Juniper, F5 and Fortinet redirect the user to a landing page (“webtop”)
after a successful authentication. We should ignore HTTP errors in fetching
the landing page if/when we've received a valid cookie, as done for Juniper
in 3e77943692b511719d9217d2ecc43588b7c6c08b.
Tests for {f5,fortinet}-auth-and-config are still passing with these
changes.
David Woodhouse [Thu, 22 Apr 2021 15:46:17 +0000 (16:46 +0100)]
Rework cstp_options and ip_info handling
Build up the new opt list and ip_info first, then have a function to
"install" them into the vpninfo, doing the sanity check at that point.
Should make the whole thing a little bit less vile.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Wed, 21 Apr 2021 04:17:29 +0000 (21:17 -0700)]
GlobalProtect IPv6 support
Turns out that I got *almost* everything right in d6db0ec03394234d41fbec7ffc794ceeb486a8f0, including the Ethertype field; the
only part I overlooked is that `ipv6-support=yes` also needs to be included
in the `/ssl-vpn/getconfig.esp` request parameters.
Sincere thanks to the user and GlobalProtect VPN administrator who provided
me with access to a GlobalProtect VPN supporting IPv6. This was a huge
help! 🙏
Still unknown/TODO: ensure that GlobalProtect IPv6 works sanely with ESP.
- For Fortinet, vpninfo->ppp_dtls_connect_req differs from
vpninfo->ppp_tls_connect_req. For F5 they are identical.
- For Fortinet, the DTLS/UDP port always matches the TLS/TCP port.
For F5 they can differ.
Daniel Lenski [Wed, 14 Apr 2021 19:40:37 +0000 (12:40 -0700)]
Fortinet: don't keep retrying if cookie is invalid on reconnect
The fortinet_configure() requests return 302 redirects to '/remote/login'
if the auth session/cookie is no longer valid. We should detect this and
return -EPERM rather than -EINVAL, so that ssl_reconnect() doesn't keep
trying to reconnect.
NB: Detecting this redirect is perhaps a bit harder than it should be,
because do_https_request() returns 0, rather than the real HTTP status
code (e.g. 302), in the case of an successful-but-unfetched redirect.
David Woodhouse [Mon, 12 Apr 2021 16:40:17 +0000 (17:40 +0100)]
Add F5 DTLS support
Add a new f5_udp_mainloop() which handles the DTLS handshakes and then
sends the GET request to start the tunnel, and handles the HTTP response
with the IP addresses (using some more crappy HTTP parsing of its own).
Instead of storing all the parameters required to create that request,
it's created in advance and stored in a new vpninfo->ppp_tls_connect_req
field by the f5_configure() function which has been split out from
f5_connect(). (Originally intended not to be called a second time, but
actually the server doesn't send packets down a newly-established
tunnel to us unless we refetch the profile etc.)
There's also a new vpninfo->ppp_dtls_connect_req field which is unused
for now but Fortinet will want it since its request format differs for
TLS vs. DTLS.
What's left of the f5_connect() function now only actually establishes
the PPP connection over TLS if DTLS is disabled, as expected by the
recent changes to ppp_tcp_mainloop().
NB: If there is packet loss and we drop the *response* to the GET
request, the server won't resend it; it just goes on to PPP negotiation.
We might actually be able to cope with that at least for Legacy IP,
since it'll give us our address that way. Not for IPv6 though, as the LL
address it gives us in PPP isn't useful.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Tue, 13 Apr 2021 04:52:14 +0000 (05:52 +0100)]
Add full DTLS support for PPP
Based on the logic in gpst_mainloop() for ESP deferral, the TCP mainloop
first waits and does nothing, giving DTLS 5 seconds to connect.
Only if/when DTLS is out of the picture does it start trying to establish
PPP for itself. If the PPP state is still PPPS_DEAD, that means that the
protocol's ->tcp_connect() method never actually established the tunnel,
so call it again directly; falling back to a full ssl_reconnect() if
needed.
This time round (based on the DTLS state) the protocol's ->tcp_connect()
method is then expected to establish the tunnel *and* call
ppp_start_tcp_mainloop() which will do one pass through the state
machine and shift from PPPS_DEAD to PPPS_ESTABLISH.
Subsequent calls to ppp_tcp_mainloop() see a state other than PPPS_DEAD
(as well as DTLS disabled) and call directly through to the core PPP
mainloop for processing.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Wed, 14 Apr 2021 22:42:50 +0000 (23:42 +0100)]
Fix handling of lost TERMACK
The comment said we'd send a TERMREQ "once more" after 3 seconds if we
didn't get a TERMACK in response to the first one. It transpires this
claim was false. If we didn't get a response to the first one, we would
send a *flood* of TERMREQs until we filled the socket sndbuf, with no
delay between them at all because we weren't updating ->lcp.last_req.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Sun, 11 Apr 2021 16:46:09 +0000 (17:46 +0100)]
ppp: Clean up negotiated IP/DNS option handling
We were leaking these strings. Fix it up to properly attempt negotiation
and set options in the option list with the results.
Also make it abort IPCP if the server doesn't accept a Legacy IP address
which had already been configured. We can't do the same for IPv6 as PPP
only actually configures the link-local address.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Fri, 12 Feb 2021 01:48:40 +0000 (17:48 -0800)]
Add start_dtls_anon_handshake() for PPP protocols
Instead of PSK or fake-resumed sessions, F5 and Fortinet just do normal
DTLS.
Which isn't actually "normal" for us, so add start_dtls_anon_handshake()
to support it. This needs to add certificate validation, which wasn't
necessary with PSK or session resumption which were effectively mutual
authentication anyway.
It is invoked whenever the vpninfo->dtls_cipher field is empty. The
Cisco AnyConnect support *always* sets that; the normal DTLS protocols
never will.
Rename and repurpose the vpninfo->cisco_dtls12 field to ->dtls12. For
Cisco it doesn't change its semantics, but for normal DTLS it indicates
whether we can try DTLSv1.2 or not. It doesn't guarantee that the server
*will* do DTLSv1.2; it only lets us *try*. So it should be set
everywhere except for the older F5 BIG-IP servers (v15 and earlier)
which don't set the <dtls_v1_2_supported> tag and which actually crap
themselves and abort the connection if we try DTLSv1.2 instead of
negotiating down to DTLSv1.0 sanely.
Signed-off-by: Daniel Lenski <dlenski@gmail.com> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Tue, 20 Apr 2021 12:52:44 +0000 (13:52 +0100)]
Add DTLS_ESTABLISHED state
Cisco DTLS is preauthenticated by PSK or the session resume hack, but
with anonymous DTLS there will be authentication over the DTLS channel
once it has been connected. Add a new state DTLS_ESTABLISHED, and let
DTLS_CONNECTED mean merely *connected* at the transport level.
ESP used to use DTLS_CONNECTING state to indicate that the ESP probes
had been received and we were ready to tell the server (over the TCP
channel) to switch over to use ESP. Use DTLS_CONNECTED for that state
now, since it's a better fit. And DTLS_ESTABLISHED when we've actually
told the server to change.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Wed, 14 Apr 2021 10:22:39 +0000 (11:22 +0100)]
Fix timeout handling for DTLS handshake retries
We weren't actually waking the mainloop up when the TLS library wanted
to (re)send a DTLS handshake. Plumb the timeout pointer through
dtls_try_handshake() to fix that.
Also feed it to dtls_reconnect(), which helps us ensure that we wake up
after dtls_attempt_period() to try again.
Export dtls_reconnect() while we're at it, since PPP will want to be
able to invoke it.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Sat, 17 Apr 2021 14:12:45 +0000 (07:12 -0700)]
Fix f5-auth-and-config tests not to depend on cookies
Tests depended on cookies propagating all the way to the tunnel
connection ('GET /tmyvpn'). Real servers don't need to see the cookie
at this point though, and for the DTLS connection we don't *want* to
include it because the resulting packet could end up too large. So
don't require it.
Daniel Lenski [Wed, 21 Apr 2021 00:26:30 +0000 (17:26 -0700)]
Speculatively enable no_terminate_on_pause for Fortinet
Given what we *have* learned so far about Fortinet reconnects
(https://gitlab.com/openconnect/openconnect/-/issues/235#note_552995833), it
appears likely that, as for F5, the PPP connection should *not* be
Terminated prior to a pause-and-reconnect.
Daniel Lenski [Sat, 17 Apr 2021 09:59:51 +0000 (02:59 -0700)]
Pulse should fallback to Juniper logout
The Pulse logout function, pulse_bye(), can fail to logout the session if the IFT-T tunnel is already closed. As a fallback, use oncp_bye() which logs out via a new HTTP request.
David Woodhouse [Mon, 12 Apr 2021 11:02:36 +0000 (12:02 +0100)]
Add DTLS support to ssl_nonblock_read() / ssl_nonblock_write()
We previously lived with #ifdef hacks for OpenSSL vs. GnuTLS in dtls.c
because there were only a few call sites and only one each for send/recv
where the -EAGAIN handling actually matters.
Now that we're going to want to send/receive DTLS frames for PPP too,
clean it up a bit.
While we're at it, fix up the OpenSSL version of ssl_nonblock_read()
so that it only ignores SSL_ERROR_WANT_READ and SSL_ERROR_WANT_WRITE.
Previously it ignored *every* error except SSL_ERROR_ZERO_RETURN and
SSL_ERROR_SYSCALL.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Wed, 14 Apr 2021 22:32:02 +0000 (15:32 -0700)]
Handle F5 split-exclude routes
Minor related changes:
• Align the cstp_options names for netmask and split {in,ex}clude routes
with other protocols
• Stop logging IP version for DNS servers and split routes, since we store
them in the same place regardless of version.
David Woodhouse [Mon, 12 Apr 2021 09:57:23 +0000 (10:57 +0100)]
Remove Cisco-specific option handling from dtls_setup()
As we start to use DTLS for protocols other than Cisco AnyConnect, we
need to start disentangling the underlying DTLS support from the Cisco
protocol. Start by moving the X-DTLS-Foo: header processing into cstp.c
where half of it was anyway.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Thu, 15 Apr 2021 15:36:15 +0000 (16:36 +0100)]
Fix -EAGAIN on writing DTLS socket for PPP mainloop
If the socket sndbuf is full, we fail to send a packet. This should
actually happen less often for UDP than it does for TCP because of the
lack of back pressure from the peer, but it's still a fairly normal
condition and needs to be handled properly.
The correct handling is basically to do nothing and wait for the jam to
clear, unless it's been *so* long that DPD has caused us to think the
peer is dead, or we need a rekey (which we do be reconnecting the
transport channel). The ka_stalled_action() function exists specifically
to handle this case, and returns one of KA_NONE, KA_DPD_DEAD or KA_REKEY.
There's no ponit in it returning KA_DPD or KA_KEEPALIVE like the normal
keepalive_action() does because the point here is that we *can't* send
anything at the moment anyway.
The problem here is that the PPP mainloop doesn't handle those three
cases correctly, and KA_REKEY and KA_NONE were just falling through to
end up at the fatal error condition for a *short* write, which should
never happen. I went too far when commenting out the cut and pasted
mainloop to create ppp_mainloop.
Fix KA_REKEY to do a reconnect, and KA_NONE to just return work_done
as it should.
Fixes: a47406b43 ("add support for PPP-based protocols") Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Tue, 13 Apr 2021 16:49:44 +0000 (09:49 -0700)]
Don't call connection script in ssl_reconnect if tunnel is not up
This refines the DTLS-to-TLS fallback in ppp_tcp_mainloop.
Calling the connection script (with the attempt-reconnect and reconnect
reasons) is confusing if this is actually the *initial* connection, and
the tunnel isn't up yet.
Daniel Lenski [Thu, 15 Apr 2021 00:55:42 +0000 (17:55 -0700)]
NC/Pulse idle timeout
Per #234, it appears that:
- oNCP server termination reason 8 indicates idle timeout
- Pulse AVP 0x583/0xd75 indicates the idle timeout in seconds
- No sign of an idle timeout TLV for oNCP
Daniel Lenski [Tue, 13 Apr 2021 20:49:14 +0000 (13:49 -0700)]
Update 'Getting Started / Connecting' docs
- Remove section on patching for DTLS support (thankfully, long-obsolete)
- Clarify meaning of "certificate" as TLS/SSL client certificate; lots of
users appear to be confused by this
- Mention both -c and -k options
- Refer to manual for more automation of authentication
- Clarify when Jailbreak might be needed to extract certificates on
Windows (when the private key is marked as "non-exportable")
Daniel Lenski [Mon, 12 Apr 2021 23:49:00 +0000 (16:49 -0700)]
Accept IPv6 netmasks like /dead:beef::, in addition to /N
Some F5 server configurations appear to require an IPv6 netmask in this
form.
This adds a netmasklen6() function, analogous to the Legacy IP version in
netmasklen(). It also adds gcc-optimized forms, using __builtin_clz, for
both netmasklen functions.
[dwmw2: Detect __builtin_clz() with autoconf, wrap it once in cls()] Signed-off-by: Daniel Lenski <dlenski@gmail.com> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Mon, 12 Apr 2021 22:05:07 +0000 (23:05 +0100)]
Handle empty response buf in process_http_response()
If there is no body and the 'body' buf has never had any data in it, then
body->data can be NULL. So don't dereference it when trying to ensure a
NUL at the end.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Mon, 12 Apr 2021 03:23:11 +0000 (20:23 -0700)]
Set Fortinet DPD interval from server's config
The name of the tag implies that its purpose is DTLS-specific (<dtls-config heartbeat-interval="X">)
and that it requires the use of the DTLS heartbeat extension (https://tools.ietf.org/html/rfc6520).
However, PPP already contains natural analogues for keepalive (Discard-Request) and DPD (Echo-Request
and Echo-Reply), which appear to work just fine with Fortinet servers. We may as well use the server's
heartbeat/keepalive interval for a more generic, transport-agnostic DPD mechanism, since OpenConnect
and PPP already support it.
The official Fortinet client software for Windows/Mac appears to be very deficient at dead peer
detection, so this may represent a substantial improvement in functionality for some users.
David Woodhouse [Sun, 11 Apr 2021 17:02:39 +0000 (18:02 +0100)]
Fix --disable-ipv6 option
Commit 7e2129f8a9 ("ensure that openconnect_disable_{dtls,ipv6} do
nothing if vpninfo has ever been connected") added a check to disallow
setting this... if it wasn't already set, OR the connection had
previously been established. Which seems wrong; I think that was
supposed to have been *AND*.
Not that I object very much to this not working, a whole quarter of a
century after RFC1883 was published. But I wanted to use it for testing
PPP negotiation, and it wasn't working.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
David Woodhouse [Fri, 9 Apr 2021 12:09:11 +0000 (13:09 +0100)]
Add 'proto' integer value to struct vpn_proto
Despite making no mention of this in the commit message, commit 423449253 ("rename (resp_buf, form_buf) → (req_buf, resp_buf) in f5.c
and fortinet.c") added a 'flavor' field to a bunch of functions in
auth-html.c and passed it through from the protocol code to indicate
some protocol-specific quirks for form parsing.
Until then, we had mostly structured the code so that there weren't many
cases where 'core' code needed to behave differently depending on the
protocol. There was only one case, in fact, to handle the stupid Pulse
ESP address family bug. That one *used* to check which protocol was in
use by comparing proto->udp_send_probes with &oncp_esp_send_probes.
So, add an integer 'proto' field to struct vpn_proto so it can be
checked without a strcmp. Remove the now-redundant 'flavor' field from
all those functions in auth-html.c since it's now in the vpninfo, and
make the ESP code check that for NC|PULSE too instead of the trick of
checking the proto->udp_send_probes function. Which was kind of icky.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Thu, 8 Apr 2021 21:32:32 +0000 (14:32 -0700)]
Multi-protocol support documentation
Mention F5 and Fortinet on the index page, explain the multi-protocol design
philosophy a bit more, and add a page with a concise list of the supported
protocols.
Daniel Lenski [Thu, 8 Apr 2021 21:35:08 +0000 (14:35 -0700)]
Expand F5 and Fortinet documentation
Explain currently-supported authentication modes, and request feedback on additional modes.
Explain Fortinet's misfeature/design flaw, which prevents it from automatically reconnecting after a dropped or roamed connection, without a new authentication. Request feedback if anyone has acceess to a Fortinet VPN that *doesn't* have this flaw.
Daniel Lenski [Mon, 5 Apr 2021 15:37:00 +0000 (08:37 -0700)]
Use oc_text_buf for internal_get_url()
Commit c8ca180f ("factor out internal_get_url function") consolidated
two pieces of code which reconstruct the URL from its constituent parts
into a single internal_get_url() function.
Both the original versions used oc_text_buf() and built up the URL
string in a simple and readable fashion. The new internal_get_url()
function used asprintf with "%s%s%s" and ternary operators ?"/":"" in an
apparent attempt to win an obfuscated C contest.
Unsurprisingly, the first attempt was buggy; this was subsequently fixed
in commit f78f0eccc ("bugfix internal_get_url") but the horridness
remained.
In addition, the original two versions had slightly different behaviour —
the one in auth-juniper.c would *always* append a trailing / at the end
of the URL string even if vpninfo->urlpath was NULL, while the one in
AnyConnect's auth.c would only do so if it was non-NULL (including when
it's set to "").
As well as switching back to using oc_text_buf to build up the string so
that we can read the code without our eyes bleeding, fix it to always
add the trailing / even when vpninfo->urlpath is NULL.
Signed-off-by: Daniel Lenski <dlenski@gmail.com> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Fri, 2 Apr 2021 07:57:14 +0000 (00:57 -0700)]
add test path including frmSelectRoles
frmSelectRoles is a "form" that sort of acts like a realm dropdown except that…
(1) It comes AFTER the credentials have been submitted (¯\_(ツ)_/¯)
(2) It doesn't actually contain any form fields. It only contains links to
a small number of role choices. It then redirects to 'GET login.cgi'.
Inane, but apparently used in the real world. See examples at:
https://gitlab.gnome.org/GNOME/NetworkManager-openconnect/-/issues/30#note_988366
David Woodhouse [Sat, 27 Mar 2021 16:17:57 +0000 (16:17 +0000)]
Add Wintun support
Refactor the device discovery a little bit to accommodate it. We need
to scan the registry as we did for TAP-Windows anyway, in order to
find the TUNIDX that vpnc-script-win.js will use to configure routes.
So run through the scan as before, but *if* nothing is found and the
user specified an ifname, attempt to create a Wintun device with that
name. Then scan again and presumably we'll find it this time.
This should hopefully make it relatively harmless to existing setups
using TAP-Windows.
There's lots of memcpy here as Wintun *really* wants us to use its pool.
More thought required for that one, as it can't even handle stripping
packet headers so even if we decrypt directly into its buffer that
wouldn't be enough.
The configuration is also fairly horrid, as vpnc-script-win.js seems
to depend on the Legacy IP address being set on the interface before
it will set up the routes. Which of course assumes that there *is*
Legacy IP at all, and it isn't an IPv6-only tunnel.
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Daniel Lenski [Mon, 29 Mar 2021 23:59:08 +0000 (16:59 -0700)]
bugfix !165 Juniper forms handling
I missed a small piece of my original "fix for forms with 'id' but no 'name'" (this one:
https://gitlab.com/openconnect/openconnect/-/commit/c8a6ee94180a09339eafefe0a2d889e7aa508e56#5503a3e92468be6db7c76e16b82bb5f3e9b14131_260_259)
in rebasing the ppp_core branch to merge cleanly with the changes from !171.
Caused this crash: https://gitlab.com/openconnect/openconnect/-/merge_requests/169#note_540411103
TODO: add Flask-based tests of Juniper auth to verify Juniper form handling.
projects / users / dwmw2 / openconnect.git / log