mirror of
https://git.tartarus.org/simon/putty.git
synced 2025-01-09 17:38:00 +00:00
pre-0.83
10 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Simon Tatham
|
98200d1bfe |
Arm: turn on PSTATE.DIT if available and needed.
DIT, for 'Data-Independent Timing', is a bit you can set in the
processor state on sufficiently new Arm CPUs, which promises that a
long list of instructions will deliberately avoid varying their timing
based on the input register values. Just what you want for keeping
your constant-time crypto primitives constant-time.
As far as I'm aware, no CPU has _yet_ implemented any data-dependent
optimisations, so DIT is a safety precaution against them doing so in
future. It would be embarrassing to be caught without it if a future
CPU does do that, so we now turn on DIT in the PuTTY process state.
I've put a call to the new enable_dit() function at the start of every
main() and WinMain() belonging to a program that might do
cryptography (even testcrypt, in case someone uses it for something!),
and in case I missed one there, also added a second call at the first
moment that any cryptography-using part of the code looks as if it
might become active: when an instance of the SSH protocol object is
configured, when the system PRNG is initialised, and when selecting
any cryptographic authentication protocol in an HTTP or SOCKS proxy
connection. With any luck those precautions between them should ensure
it's on whenever we need it.
Arm's own recommendation is that you should carefully choose the
granularity at which you enable and disable DIT: there's a potential
time cost to turning it on and off (I'm not sure what, but plausibly
something of the order of a pipeline flush), so it's a performance hit
to do it _inside_ each individual crypto function, but if CPUs start
supporting significant data-dependent optimisation in future, then it
will also become a noticeable performance hit to just leave it on
across the whole process. So you'd like to do it somewhere in the
middle: for example, you might turn on DIT once around the whole
process of verifying and decrypting an SSH packet, instead of once for
decryption and once for MAC.
With all respect to that recommendation as a strategy for maximum
performance, I'm not following it here. I turn on DIT at the start of
the PuTTY process, and then leave it on. Rationale:
1. PuTTY is not otherwise a performance-critical application: it's
not likely to max out your CPU for any purpose _other_ than
cryptography. The most CPU-intensive non-cryptographic thing I can
imagine a PuTTY process doing is the complicated computation of
font rendering in the terminal, and that will normally be cached
(you don't recompute each glyph from its outline and hints for
every time you display it).
2. I think a bigger risk lies in accidental side channels from having
DIT turned off when it should have been on. I can imagine lots of
causes for that. Missing a crypto operation in some unswept corner
of the code; confusing control flow (like my coroutine macros)
jumping with DIT clear into the middle of a region of code that
expected DIT to have been set at the beginning; having a reference
counter of DIT requests and getting it out of sync.
In a more sophisticated programming language, it might be possible to
avoid the risk in #2 by cleverness with the type system. For example,
in Rust, you could have a zero-sized type that acts as a proof token
for DIT being enabled (it would be constructed by a function that also
sets DIT, have a Drop implementation that clears DIT, and be !Send so
you couldn't use it in a thread other than the one where DIT was set),
and then you could require all the actual crypto functions to take a
DitToken as an extra parameter, at zero runtime cost. Then "oops I
forgot to set DIT around this piece of crypto" would become a compile
error. Even so, you'd have to take some care with coroutine-structured
code (what happens if a Rust async function yields while holding a DIT
token?) and with nesting (if you have two DIT tokens, you don't want
dropping the inner one to clear DIT while the outer one is still there
to wrongly convince callees that it's set). Maybe in Rust you could
get this all to work reliably. But not in C!
DIT is an optional feature of the Arm architecture, so we must first
test to see if it's supported. This is done the same way as we already
do for the various Arm crypto accelerators: on ELF-based systems,
check the appropriate bit in the 'hwcap' words in the ELF aux vector;
on Mac, look for an appropriate sysctl flag.
On Windows I don't know of a way to query the DIT feature, _or_ of a
way to write the necessary enabling instruction in an MSVC-compatible
way. I've _heard_ that it might not be necessary, because Windows
might just turn on DIT unconditionally and leave it on, in an even
more extreme version of my own strategy. I don't have a source for
that - I heard it by word of mouth - but I _hope_ it's true, because
that would suit me very well! Certainly I can't write code to enable
DIT without knowing (a) how to do it, (b) how to know if it's safe.
Nonetheless, I've put the enable_dit() call in all the right places in
the Windows main programs as well as the Unix and cross-platform code,
so that if I later find out that I _can_ put in an explicit enable of
DIT in some way, I'll only have to arrange to set HAVE_ARM_DIT and
compile the enable_dit() function appropriately.
|
||
|
Simon Tatham
|
841bf321d4 |
New abstraction for command-line arguments.
This begins the process of enabling our Windows applications to handle Unicode characters on their command lines which don't fit in the system code page. Instead of passing plain strings to cmdline_process_param, we now pass a partially opaque and platform-specific thing called a CmdlineArg. This has a method that extracts the argument word as a default-encoded string, and another one that tries to extract it as UTF-8 (though it may fail if the UTF-8 isn't available). On Windows, the command line is now constructed by calling split_into_argv_w on the Unicode command line returned by GetCommandLineW(), and the UTF-8 method returns text converted directly from that wide-character form, not going via the system code page. So it _can_ include UTF-8 characters that wouldn't have round-tripped via CP_ACP. This commit introduces the abstraction and switches over the cross-platform and Windows argv-handling code to use it, with minimal functional change. Nothing yet tries to call cmdline_arg_get_utf8(). I say 'cross-platform and Windows' because on the Unix side there's still a lot of use of plain old argv which I haven't converted. That would be a much larger project, and isn't currently needed: the _current_ aim of this abstraction is to get the right things to happen relating to Unicode on Windows, so for code that doesn't run on Windows anyway, it's not adding value. (Also there's a tension with GTK, which wants to talk to standard argv and extract arguments _it_ knows about, so at the very least we'd have to let it munge argv before importing it into this new system.) |
||
|
Simon Tatham
|
d509a2dc1e |
Formatting: normalise to put a space after condition keywords.
'if (thing)' is the local style here, not 'if(thing)'. Similarly with 'for' and 'while'. |
||
|
Simon Tatham
|
20f818af12 |
Rename 'ret' variables passed from allocation to return.
I mentioned recently (in commit
|
||
|
Simon Tatham
|
04c1617f20 |
Formatting: realign labels and case/default statements.
My aim has always been to have those back-dented by 2 spaces (half an indent level) compared to the statements around them, so that in particular switch statements have distinct alignment for the statement, the cases and the interior code without consuming two whole indent levels. This patch sweeps up all the violations of that principle found by my bulk-reindentation exercise. |
||
|
Simon Tatham
|
787c358d37 |
Fix command-line password handling in Restart Session.
When the user provides a password on the PuTTY command line, via -pw or -pwfile, the flag 'tried_once' inside cmdline_get_passwd_input() is intended to arrange that we only try sending that password once, and after we've sent it, we don't try again. But this plays badly with the 'Restart Session' operation. If the connection is lost and then restarted at user request, we _do_ want to send that password again! So this commit moves that static variable out into a small state structure held by the client of cmdline_get_passwd_input. Each client can decide how to manage that state itself. Clients that support 'Restart Session' - i.e. just GUI PuTTY itself - will initialise the state at the same time as instantiating the backend, so that every time the session is restarted, we return to (correctly) believing that we _haven't_ yet tried the password provided on the command line. But clients that don't support 'Restart Session' - i.e. Plink and file transfer tools - can do the same thing that cmdline.c was doing before: just keep the state in a static variable. This also means that the GUI login tools will now retain the command-line password in memory, whereas previously they'd have wiped it out once it was used. But the other tools will still wipe and free the password, because I've also added a 'bool restartable' flag to cmdline_get_passwd_input to let it know when it _is_ allowed to do that. In the GUI tools, I don't see any way to get round that, because if the session is restarted you _have_ to still have the password to use again. (And you can't infer that that will never happen from the CONF_close_on_exit setting, because that too could be changed in mid-session.) On the other hand, I think it's not all that worrying, because the use of either -pw or -pwfile means that a persistent copy of your password is *already* stored somewhere, so another one isn't too big a stretch. (Due to the change of -pw policy in 0.77, the effect of this bug was that an attempt to reconnect in a session set up this way would lead to "Configured password was not accepted". In 0.76, the failure mode was different: PuTTY would interactively prompt for the password, having wiped it out of memory after it was used the first time round.) |
||
|
Simon Tatham
|
5935c68288 |
Update source file names in comments and docs.
Correcting a source file name in the docs just now reminded me that I've seen a lot of outdated source file names elsewhere in the code, due to all the reorganisation since we moved to cmake. Here's a giant pass of trying to make them all accurate again. |
||
|
Simon Tatham
|
a2ff884512 |
Richer data type for interactive prompt results.
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!) |
||
|
Simon Tatham
|
cd8a7181fd |
Complete rework of terminal userpass input system.
The system for handling seat_get_userpass_input has always been structured differently between GUI PuTTY and CLI tools like Plink. In the CLI tools, password input is read directly from the OS terminal/console device by console_get_userpass_input; this means that you need to ensure the same terminal input data _hasn't_ already been consumed by the main event loop and sent on to the backend. This is achieved by the backend_sendok() method, which tells the event loop when the backend has finished issuing password prompts, and hence, when it's safe to start passing standard input to backend_send(). But in the GUI tools, input generated by the terminal window has always been sent straight to backend_send(), regardless of whether backend_sendok() says it wants it. So the terminal-based implementation of username and password prompts has to work by consuming input data that had _already_ been passed to the backend - hence, any backend that needs to do that must keep its input on a bufchain, and pass that bufchain to seat_get_userpass_input. It's awkward that these two totally different systems coexist in the first place. And now that SSH proxying needs to present interactive prompts of its own, it's clear which one should win: the CLI style is the Right Thing. So this change reworks the GUI side of the mechanism to be more similar: terminal data now goes into a queue in the Ldisc, and is not sent on to the backend until the backend says it's ready for it via backend_sendok(). So terminal-based userpass prompts can now consume data directly from that queue during the connection setup stage. As a result, the 'bufchain *' parameter has vanished from all the userpass_input functions (both the official implementations of the Seat trait method, and term_get_userpass_input() to which some of those implementations delegate). The only function that actually used that bufchain, namely term_get_userpass_input(), now instead reads from the ldisc's input queue via a couple of new Ldisc functions. (Not _trivial_ functions, since input buffered by Ldisc can be a mixture of raw bytes and session specials like SS_EOL! The input queue inside Ldisc is a bufchain containing a fiddly binary encoding that can represent an arbitrary interleaving of those things.) This greatly simplifies the calls to seat_get_userpass_input in backends, which now don't have to mess about with passing their own user_input bufchain around, or toggling their want_user_input flag back and forth to request data to put on to that bufchain. But the flip side is that now there has to be some _other_ method for notifying the terminal when there's more input to be consumed during an interactive prompt, and for notifying the backend when prompt input has finished so that it can proceed to the next stage of the protocol. This is done by a pair of extra callbacks: when more data is put on to Ldisc's input queue, it triggers a call to term_get_userpass_input, and when term_get_userpass_input finishes, it calls a callback function provided in the prompts_t. Therefore, any use of a prompts_t which *might* be asynchronous must fill in the latter callback when setting up the prompts_t. In SSH, the callback is centralised into a common PPL helper function, which reinvokes the same PPL's process_queue coroutine; in rlogin we have to set it up ourselves. I'm sorry for this large and sprawling patch: I tried fairly hard to break it up into individually comprehensible sub-patches, but I just couldn't tease out any part of it that would stand sensibly alone. |
||
|
Simon Tatham
|
f39c51f9a7 |
Rename most of the platform source files.
This gets rid of all those annoying 'win', 'ux' and 'gtk' prefixes which made filenames annoying to type and to tab-complete. Also, as with my other recent renaming sprees, I've taken the opportunity to expand and clarify some of the names so that they're not such cryptic abbreviations. |