FreeProxy sends 'algorithm="MD5"' instead of 'algorithm=MD5.' I'm
actually not sure whether that's legal by RFC 7616, but it's certainly
no trouble to parse if we see it.
With all these changes, PuTTY now _can_ successfully make connections
through FreeProxy again, whether it's in Basic or Digest mode.
Another awkward thing that FreeProxy does is to slam the connection
shut after sending its 407 response, at least in Basic auth mode. (It
keeps the connection alive in digest mode, which makes sense to me,
because that's a more stateful system.)
It was surprisingly easy to make the proxy code able to tolerate this!
I've set it up so that a ProxyNegotiator can just set its 'reconnect'
flag on return from the negotiation coroutine, and the effect will be
that proxy.c makes a new connection to the same proxy server before
doing anything else. In particular, you can set that flag _and_ put
data in the output bufchain, and there's no problem - the output data
will be queued directly into the new socket.
FreeProxy sends this as a substitute for the standard 'Connection'
header (with the same contents, i.e. 'keep-alive' or 'close' depending
on whether the TCP connection is going to continue afterwards). The
Internet reckons it's not standard, but it's easy to recognise as an
ad-hoc synonym for 'Connection'.
I had a report that the Windows free-as-in-beer proxy tool 'FreeProxy'
didn't work with the new HTTP proxy code, and it turns out that the
first reason why not is that the error-document in its 407 response is
sent via chunked transfer encoding, which is to say, instead of an
up-front Content-length header, you receive a sequence of chunks each
prefixed with a hex length.
(In 0.76, before the rewritten proxy support, we never even noticed
this because we sent Basic auth details up front in our first attempt,
rather than attempting a no-auth connection first and waiting to see
what kind of auth the proxy asks us for. So we'd only ever see a 407
if the auth details were refused - and since 0.76 didn't have
interactive proxy auth prompts, there was nothing we could do at that
point but abort immediately, without attempting to parse the rest of
the 407 at all.)
Now we spot the Transfer-encoding header and successfully parse
chunked transfers. Happily, we don't need to worry about the further
transfer-encodings such as 'gzip', because we're not actually _using_
the error document - we only have to skip over it to find the end of
the HTTP response.
This still doesn't make PuTTY work with FreeProxy, because there are
further problems hiding behind that one, which I'll fix in following
commits.
All the seat functions that request an interactive prompt of some kind
to the user - both the main seat_get_userpass_input and the various
confirmation dialogs for things like host keys - were using a simple
int return value, with the general semantics of 0 = "fail", 1 =
"proceed" (and in the case of seat_get_userpass_input, answers to the
prompts were provided), and -1 = "request in progress, wait for a
callback".
In this commit I change all those functions' return types to a new
struct called SeatPromptResult, whose primary field is an enum
replacing those simple integer values.
The main purpose is that the enum has not three but _four_ values: the
"fail" result has been split into 'user abort' and 'software abort'.
The distinction is that a user abort occurs as a result of an
interactive UI action, such as the user clicking 'cancel' in a dialog
box or hitting ^D or ^C at a terminal password prompt - and therefore,
there's no need to display an error message telling the user that the
interactive operation has failed, because the user already knows,
because they _did_ it. 'Software abort' is from any other cause, where
PuTTY is the first to know there was a problem, and has to tell the
user.
We already had this 'user abort' vs 'software abort' distinction in
other parts of the code - the SSH backend has separate termination
functions which protocol layers can call. But we assumed that any
failure from an interactive prompt request fell into the 'user abort'
category, which is not true. A couple of examples: if you configure a
host key fingerprint in your saved session via the SSH > Host keys
pane, and the server presents a host key that doesn't match it, then
verify_ssh_host_key would report that the user had aborted the
connection, and feel no need to tell the user what had gone wrong!
Similarly, if a password provided on the command line was not
accepted, then (after I fixed the semantics of that in the previous
commit) the same wrong handling would occur.
So now, those Seat prompt functions too can communicate whether the
user or the software originated a connection abort. And in the latter
case, we also provide an error message to present to the user. Result:
in those two example cases (and others), error messages should no
longer go missing.
Implementation note: to avoid the hassle of having the error message
in a SeatPromptResult being a dynamically allocated string (and hence,
every recipient of one must always check whether it's non-NULL and
free it on every exit path, plus being careful about copying the
struct around), I've instead arranged that the structure contains a
function pointer and a couple of parameters, so that the string form
of the message can be constructed on demand. That way, the only users
who need to free it are the ones who actually _asked_ for it in the
first place, which is a much smaller set.
(This is one of the rare occasions that I regret not having C++'s
extra features available in this code base - a unique_ptr or
shared_ptr to a string would have been just the thing here, and the
compiler would have done all the hard work for me of remembering where
to insert the frees!)
This is a piece I forgot in the initial implementation of HTTP Digest:
an HTTP server can send _more than one_ authentication request header
(WWW-Authenticate for normal servers, Proxy-Authenticate for proxies),
and if it does, they're supposed to be treated as alternatives to each
other, so that the client chooses one to reply to.
I suppose that technically we were 'complying' with that spec already,
in that HttpProxyNegotiator would have read each new header and
overwritten all the fields set by the previous one, so we'd always
have gone with the last header presented by the server. But that seems
inelegant: better to choose the one we actually like best.
So now we do that. All the details of an auth header are moved out of
the main HttpProxyNegotiator struct into a sub-struct we can have
multiple copies of. Each new header is parsed into a fresh struct of
that kind, and then we can compare it with the previous one and decide
which we prefer.
The preference order, naturally, is 'more secure is better': Digest
beats Basic, and between two Digest headers, SHA-256 beats MD5. (And
anything beats a header we can't make sense of at all.)
Another side effect of this change is that a 407 response which
contains _no_ Proxy-Authenticate headers will trigger an error message
saying so, instead of just going with whatever happened to be left in
the relevant variables from the previous attempt.
I was dubious about it to begin with, when I found that RFC 7616's
example seemed to be treating it as a 256-bit truncation of SHA-512,
and not the thing FIPS 180-4 section 6.7 specifies as "SHA-512/256"
(which also changes the initial hash state). Having failed to get a
clarifying response from the RFC authors, I had the idea this morning
of testing other HTTP clients to see what _they_ thought that hash
function meant, and then at least I could go with an existing
in-practice consensus.
There is no in-practice consensus. Firefox doesn't support that
algorithm at all (but they do support SHA-256); wget doesn't support
anything that RFC 7616 added to the original RFC 2617. But the prize
for weirdness goes to curl, which does accept the name "SHA-512-256"
and ... treats it as an alias for SHA-256!
So I think the situation among real clients is too confusing to even
try to work with, and I'm going to stop adding to it. PuTTY will
follow Firefox's policy: if a proxy server asks for SHA-256 digests
we'll happily provide them, but if they ask for SHA-512-256 we'll
refuse on the grounds that it's not clear enough what it means.
In http.c, this drops in reasonably neatly alongside the existing
support for Basic, now that we're waiting for an initial 407 response
from the proxy to tell us which auth mechanism it would prefer to use.
The rest of this patch is mostly contriving to add testcrypt support
for the function in cproxy.c that generates the complicated output
header to go in the HTTP request: you need about a dozen assorted
parameters, the actual response hash has two more hashes in its
preimage, and there's even an option to hash the username as well if
necessary. Much more complicated than CHAP (which is just plain
HMAC-MD5), so it needs testing!
Happily, RFC 7616 comes with some reasonably useful test cases, and
I've managed to transcribe them directly into cryptsuite.py and
demonstrate that my response-generator agrees with them.
End-to-end testing of the whole system was done against Squid 4.13
(specifically, the squid package in Debian bullseye, version 4.13-10).
Now, we always try an initial CONNECT request with no auth at all, and
wait for the proxy to reject it before sending a second try with
auth.
That way, we can wait to see what _kind_ of authentication the proxy
requests, which will enable us to support something more secure than
Basic, such as HTTP Digest.
(I mean, it would _work_ to try Basic in request #1 and then retrying
with Digest in #2 when the proxy asks for it. But if the aim of using
Digest is to avoid sending the password in cleartext, it defeats the
entire purpose to have sent it in cleartext anyway by the time you
realise the server is prepared to do something better!)
In HTTP proxying, we can (and do) send the username and password
immediately in the form of HTTP Basic, if we have them in the Conf.
But if they get rejected, or if we never sent them in the first place
and the server won't let us in without auth, then we get back an HTTP
407 response with a full set of headers and an error-document.
Assuming the HTTP connection doesn't close after that (which in
sensible HTTP/1.1 proxies it won't), this gives us the opportunity to
respond by sending a second CONNECT request, containing a fresh
username and password we just requested interactively from the user.
Previously, the proxy negotiation functions were written as explicit
state machines, with ps->state being manually set to a sequence of
positive integer values which would be tested by if statements in the
next call to the same negotiation function.
That's not how this code base likes to do things! We have a coroutine
system to allow those state machines to be implicit rather than
explicit, so that we can use ordinary control flow statements like
while loops. Reorganised each proxy negotiation function into a
coroutine-based system like that.
While I'm at it, I've also moved each proxy negotiator out into its
own source file, to make proxy.c less overcrowded and monolithic. And
_that_ gave me the opportunity to define each negotiator as an
implementation of a trait rather than as a single function - which
means now each one can define its own local variables and have its own
cleanup function, instead of all of them having to share the variables
inside the main ProxySocket struct.
In the new coroutine system, negotiators don't have to worry about the
mechanics of actually sending data down the underlying Socket any
more. The negotiator coroutine just appends to a bufchain (via a
provided bufchain_sink), and after every call to the coroutine,
central code in proxy.c transfers the data to the Socket itself. This
avoids a lot of intermediate allocations within the negotiators, which
previously kept having to make temporary strbufs or arrays in order to
have something to point an sk_write() at; now they can just put
formatted data directly into the output bufchain via the marshal.h
interface.
In this version of the code, I've also moved most of the SOCKS5 CHAP
implementation from cproxy.c into socks5.c, so that it can sit in the
same coroutine as the rest of the proxy negotiation control flow.
That's because calling a sub-coroutine (co-subroutine?) is awkward to
set up (though it is _possible_ - we do SSH-2 kex that way), and
there's no real need to bother in this case, since the only thing that
really needs to go in cproxy.c is the actual cryptography plus a flag
to tell socks5.c whether to offer CHAP authentication in the first
place.
