- Feb 23, 2020
-
-
Simon Tatham authored
I've replaced the random number generation and small delta-finding loop in primegen() with a much more elaborate system in its own source file, with unit tests and everything. Immediate benefits: - fixes a theoretical possibility of overflowing the target number of bits, if the random number was so close to the top of the range that the addition of delta * factor pushed it over. However, this only happened with negligible probability. - fixes a directional bias in delta-finding. The previous code incremented the number repeatedly until it found a value coprime to all the right things, which meant that a prime preceded by a particularly long sequence of numbers with tiny factors was more likely to be chosen. Now we select candidate delta values at random, that bias should be eliminated. - changes the semantics of the outermost primegen() function to make them easier to use, because now the caller specifies the 'bits' and 'firstbits' values for the actual returned prime, rather than having to account for the factor you're multiplying it by in DSA. DSA client code is correspondingly adjusted. Future benefits: - having the candidate generation in a separate function makes it easy to reuse in alternative prime generation strategies - the available constraints support applications such as Maurer's algorithm for generating provable primes, or strong primes for RSA in which both p-1 and p+1 have a large factor. So those become things we could experiment with in future.
-
Simon Tatham authored
This still isn't the true random generator used in the live tools: it's deterministic, for repeatable testing. The Python side of testcrypt can now call random_make_prng(), which will instantiate a PRNG with the given seed. random_clear() still gets rid of it. So I can still have some tests control the precise random numbers received by the function under test, but for others (especially key generation, with its uncertainty about how much randomness it will actually use) I can just say 'here, have a seed, generate as much stuff from that seed as you need'.
-
Simon Tatham authored
This is another application of the existing mp_bezout_into, which needed a tweak or two to cope with the numbers not necessarily being coprime, plus a wrapper function to deal with shared factors of 2. It reindents the entire second half of mp_bezout_into, so the patch is best viewed with whitespace differences ignored.
-
Simon Tatham authored
This is a third random-number generation function, with an API in between the too-specific mp_random_bits and the too-general mp_random_in_range. Now you can generate a value between 0 and n without having to either make n a power of 2, or tediously allocate a zero mp_int to be the lower limit for mp_random_in_range. Implementation is done by sawing the existing mp_random_in_range into two pieces and exposing the API between them.
-
Simon Tatham authored
This is a version of mp_lshift_fixed_into() which allocates the output number, which it can do because you know the size of the original number and are allowed to treat the shift count as non-secret. (By contrast, mp_lshift_safe() would be a nonsensical function - if you're trying to keep the shift count secret, you _can't_ use it as a parameter of memory allocation! In that situation you have no choice but to allocate memory based on a fixed upper bound.)
-
Simon Tatham authored
There was previously no safe left shift at all, which is an omission. And rshift_safe_into was an odd thing to be missing, so while I'm here, I've added it on the basis that it will probably be useful sooner or later.
-
Simon Tatham authored
Unlike the ones in mpint.c proper, these are not intended to respect the constant-time guarantees. They're going to be the kind of thing you use in key generation, which is too random to be constant-time in any case. I've arranged several precautions to try to make sure these functions don't accidentally get linked into the main SSH application, only into key generators: - declare them in a separate header with "unsafe" in the name - put "unsafe" in the name of every actual function - don't even link the mpunsafe.c translation unit into PuTTY proper - in fact, define global symbols of the same name in that file and the SSH client code, so that there will be a link failure if we ever try to do it by accident The initial contents of the new source file consist of the subroutine mp_mod_short() that previously lived in sshprime.c (and was not in mpint.c proper precisely because it was unsafe). While I'm here, I've turned it into mp_unsafe_mod_integer() and let it take a modulus of up to 32 bits instead of 16. Also added some obviously useful functions to shrink an mpint to the smallest physical size that can hold the contained number (rather like bn_restore_invariant in the old Bignum system), which I expect to be using shortly.
-
Simon Tatham authored
Mostly because I just had a neat idea about how to expose that large mutable array without it being a mutable global variable: make it a static in its own module, and expose only a _pointer_ to it, which is const-qualified. While I'm there, changed the name to something more descriptive.
-
Simon Tatham authored
I've got opt_val_mpint already in the test system, so it makes sense to use it.
-
Simon Tatham authored
There's always something.
-
Simon Tatham authored
When I implemented the puttygen --dump option recently, my aim was that all the components related to the public key would have names beginning with "public_", and the private components' names should begin with "private_". Anything not beginning with either is a _parameter of the system_, i.e. something that can safely be shared between multiple users' key pairs without giving any of them the ability to break another's key. (In particular, in integer DSA, p, q and g are unprefixed, y is labelled as public, and x as private. In principle, p,q,g are safe to share; I think the only reason nobody bothers is that standardisation is more difficult than generating a fresh prime every time. In elliptic-curve DSA, the effort equation reverses, because finding a good curve is a pain, so everybody standardises on one of a small number of well-known ones.) Anyway. This is all very well except that I left the 'public' prefix off the EdDSA x and y values. Now fixed.
-
- Feb 22, 2020
-
-
Simon Tatham authored
While looking over the code for other reasons, I happened to notice that the internal function mp_add_masked_integer_into was using a totally wrong condition to check whether it was about to do an out-of-range right shift: it was comparing a shift count measured in bits against BIGNUM_INT_BYTES. The resulting bug hasn't shown up in the code so far, which I assume is just because no caller is passing any RHS to mp_add_integer_into bigger than about 1 or 2. And it doesn't show up in the test suite because I hadn't tested those functions. Now I am testing them, and the newly added test fails when built for 16-bit BignumInt if you back out the actual fix in this commit.
-
Simon Tatham authored
Also spelled '-O text', this takes a public or private key as input, and produces on standard output a dump of all the actual numbers involved in the key: the exponent and modulus for RSA, the p,q,g,y parameters for DSA, the affine x and y coordinates of the public elliptic curve point for ECC keys, and all the extra bits and pieces in the private keys too. Partly I expect this to be useful to me for debugging: I've had to paste key files a few too many times through base64 decoders and hex dump tools, then manually decode SSH marshalling and paste the result into the Python REPL to get an integer object. Now I should be able to get _straight_ to text I can paste into Python. But also, it's a way that other applications can use the key generator: if you need to generate, say, an RSA key in some format I don't support (I've recently heard of an XML-based one, for example), then you can run 'puttygen -t rsa --dump' and have it print the elements of a freshly generated keypair on standard output, and then all you have to do is understand the output format.
-
Simon Tatham authored
In the previous commit I introduced the ability for PuTTY to talk to a server speaking the bare ssh-connection protocol, and listed several applications for that ability. But none of those applications is any use without a server that speaks the same protocol. Until now, the only such server has been the Unix-domain socket presented by an upstream connection-sharing PuTTY - and we already had a way to connect to that. So here's the missing piece: by reusing code that already existed for the testing SSH server Uppity, I've created a program that will speak the bare ssh-connection protocol on its standard I/O channels. If you want to get a shell session over any of the transports I mentioned in the last commit, this is the program you need to run at the far end of it. I have yet to write the documentation, but just in case I forget, the name stands for 'Pseudo Ssh for Untappable, Separately Authenticated Networks'.
-
Simon Tatham authored
This is the same protocol that PuTTY's connection sharing has been using for years, to communicate between the downstream and upstream PuTTYs. I'm now promoting it to be a first-class member of the protocols list: if you have a server for it, you can select it in the GUI or on the command line, and write out a saved session that specifies it. This would be completely insecure if you used it as an ordinary network protocol, of course. Not only is it non-cryptographic and wide open to eavesdropping and hijacking, but it's not even _authenticated_ - it begins after the userauth phase of SSH. So there isn't even the mild security theatre of entering an easy-to-eavesdrop password, as there is with, say, Telnet. However, that's not what I want to use it for. My aim is to use it for various specialist and niche purposes, all of which involve speaking it over an 8-bit-clean data channel that is already set up, secured and authenticated by other methods. There are lots of examples of such channels: - a userv(1) invocation - the console of a UML kernel - the stdio channels into other kinds of container, such as Docker - the 'adb shell' channel (although it seems quite hard to run a custom binary at the far end of that) - a pair of pipes between PuTTY and a Cygwin helper process - and so on. So this protocol is intended as a convenient way to get a client at one end of any those to run a shell session at the other end. Unlike other approaches, it will give you all the SSH-flavoured amenities you're already used to, like forwarding your SSH agent into the container, or forwarding selected network ports in or out of it, or letting it open a window on your X server, or doing SCP/SFTP style file transfer. Of course another way to get all those amenities would be to run an ordinary SSH server over the same channel - but this approach avoids having to manage a phony password or authentication key, or taking up your CPU time with pointless crypto.
-
Simon Tatham authored
Now I can have multiple BackendVtable structures sharing all their function pointers, and still tell which is which when init is setting things up.
-
Simon Tatham authored
PSCP and PSFTP can only work over a protocol enough like SSH to be able to run subsystems (or at the very least a remote command, for old-style PSCP). Historically we've implemented this restriction by having them not support any protocol-selection command-line options at all, and hardwiring them to instantiating ssh_backend. This commit regularises them to be more like the rest of the tools. You can select a protocol using the appropriate command-line option, provided it's a protocol in those tools' backends[] array. And the setup code will find the BackendVtable to instantiate by the usual method of calling backend_vt_from_proto. Currently, this makes essentially no difference: those tools link in be_ssh.c, which means the only supported backend is SSH. So the effect is that now -ssh is an accepted option with no effect, instead of being rejected. But it opens the way to add other protocols that are SSH-like enough to run file transfer over.
-
Simon Tatham authored
Similarly to the previous commit, this is one fewer place where I need to make a handwritten change with each new protocol.
-
Simon Tatham authored
I'm reusing the 'id' string from each BackendVtable as the name of its command-line option, which means I don't need to manually implement an option for each new protocol.
-
Simon Tatham authored
The previous 'name' field was awkwardly serving both purposes: it was a machine-readable identifier for the backend used in the saved session format, and it was also used in error messages when Plink wanted to complain that it didn't support a particular backend. Now there are two separate name fields for those purposes.
-
Simon Tatham authored
This will fit nicely with Unix PuTTY's ability to connect to one.
-
Simon Tatham authored
Now you can type an absolute pathname (starting with '/') into the hostname box in Unix GUI PuTTY, or into the hostname slot on the Unix Plink command line, and the effect will be that PuTTY makes an AF_UNIX connection to the specified Unix-domain socket in place of a TCP/IP connection. I don't _yet_ know of anyone running SSH on a Unix-domain socket, but at the very least it'll be useful to me for debugging and testing, and I'm pretty sure there will be other specialist uses sooner or later.
-
Simon Tatham authored
This file exports several functions defined in sshserver.h, and the declarations weren't being type-checked against the definitions.
-
- Feb 16, 2020
-
-
Simon Tatham authored
Sometimes, within a switch statement, you want to declare local variables specific to the handler for one particular case. Until now I've mostly been writing this in the form switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; } break; } which is ugly because the two pieces of essentially similar code appear at different indent levels, and also inconvenient because you have less horizontal space available to write the complicated case handler in - particuarly undesirable because _complicated_ case handlers are the ones most likely to need all the space they can get! After encountering a rather nicer idiom in the LLVM source code, and after a bit of hackery this morning figuring out how to persuade Emacs's auto-indent to do what I wanted with it, I've decided to move to an idiom in which the open brace comes right after the case statement, and the code within it is indented the same as it would have been without the brace. Then the whole case handler (including the break) lives inside those braces, and you get something that looks more like this: switch (discriminant) { case SIMPLE: do stuff; break; case COMPLICATED: { declare variables; do stuff; break; } } This commit is a big-bang change that reformats all the complicated case handlers I could find into the new layout. This is particularly nice in the Pageant main function, in which almost _every_ case handler had a bundle of variables and was long and complicated. (In fact that's what motivated me to get round to this.) Some of the innermost parts of the terminal escape-sequence handling are also breathing a bit easier now the horizontal pressure on them is relieved. (Also, in a few cases, I was able to remove the extra braces completely, because the only variable local to the case handler was a loop variable which our new C99 policy allows me to move into the initialiser clause of its for statement.) Viewed with whitespace ignored, this is not too disruptive a change. Downstream patches that conflict with it may need to be reapplied using --ignore-whitespace or similar.
-
- Feb 15, 2020
-
-
Simon Tatham authored
This links up the new re-encryption facilities to the Unix Pageant client-mode command line. Analogously to -d and -D, 'pageant -r key-id' re-encrypts a single key, and 'pageant -R' re-encrypts everything.
-
Simon Tatham authored
The reencrypt-all request is unusual in its ability to be _partially_ successful. To handle this I've introduced a new return status, PAGEANT_ACTION_WARNING. At the moment, users of this client code don't expect it to appear on any request, and I'll make them watch for it only in the case where I know a particular function can generate it.
-
Simon Tatham authored
These requests parallel 'delete key' and 'delete all keys', but they work on keys which you originally uploaded in encrypted form: they cause Pageant to delete only the _decrypted_ form of the key, so that the next attempt to use the key will need to re-prompt for its passphrase.
-
Simon Tatham authored
We set it when we started prompting for a passphrase, and never unset it again when the passphrase prompt either succeeded or failed. Until now it hasn't mattered, because the only use of the flag is to suppress duplicate prompts, and once a key has been decrypted, we never need to prompt for it again, duplicate or otherwise. But that's about to change, so now this bug needs fixing.
-
Simon Tatham authored
There was a lot of ugly, repetitive, error-prone code that decoded agent responses in raw data buffers. Now my internal client query function is returning something that works as a BinarySource, so we can decode agent responses using the marshal.h system like any other SSH-formatted message in this code base. While I'm at it, I've centralised more of the parsing of key lists (saving repetition in pageant_add_key and pageant_enum_keys), including merging most of the logic between SSH-1 and SSH-2. The old functions pageant_get_keylist1 and pageant_get_keylist2 aren't exposed in pageant.h any more, because they no longer exist in that form, and also because nothing was using them anyway. (Windows Pageant was using the separate pageant_nth_ssh2_key() functions that talk directly to the core, and Unix Pageant was using the more cooked client function pageant_enum_keys.)
-
Simon Tatham authored
No real need - when we fail to free this strbuf, we were about to exit the whole process anyway - but it keeps Leak Sanitiser off my back, as usual.
-
- Feb 12, 2020
-
-
Simon Tatham authored
Apparently a handful of calls to that particular function managed to miss my big-bang conversion to using bool where appropriate, and were still being called with constants 0 and 1.
-
- Feb 11, 2020
-
-
Simon Tatham authored
We received a report that if you enable Windows 10's high-contrast mode, the text in PuTTY's installer UI becomes invisible, because it's displayed in the system default foreground colour against a background of the white right-hand side of our 'msidialog.bmp' image. That's fine when the system default fg is black, but high-contrast mode flips it to white, and now you have white on white text, oops. Some research in the WiX bug tracker suggests that in Windows 10 you don't actually have to use BMP files for your installer images any more: you can use PNG, and PNGs can be transparent. However, someone else reported that that only works in up-to-date versions of Windows. And in fact there's no need to go that far. A more elegant answer is to simply not cover the whole dialog box with our background image in the first place. I've reduced the size of the background image so that it _only_ contains the pretty picture on the left-hand side, and omits the big white rectangle that used to sit under the text. So now the RHS of the dialog is not covered by any image at all, which has the same effect as it being covered with a transparent image, except that it doesn't require transparency support from msiexec. Either way, the background for the text ends up being the system's default dialog-box background, in the absence of any images or controls placed on top of it - so when the high-contrast mode is enabled, it flips to black at the same time as the text flips to white, and everything works as it should. The slight snag is that the pre-cooked WiX UI dialog specifications let you override the background image itself, but not the Width and Height fields in the control specifications that refer to them. So if you just try to drop in a narrow image in the most obvious way, it gets stretched across the whole window. But that's not a show-stopper, because we're not 100% dependent on getting WiX to produce exactly the right output. We already have the technology to postprocess the MSI _after_ it comes out of WiX: we're using it to fiddle the target-platform field for the Windows on Arm installers. So all I had to do was to turn msiplatform.py into a more general msifixup.py, add a second option to change the width of the dialog background image, and run it on the x86 installers as well as the Arm ones.
-
Simon Tatham authored
A PageantSignOp for a not-yet-decrypted key was being linked on to its key's blocked_requests queue twice, mangling the linked list integrity and causing segfaults. Now we take care to NULL out the pointers within the signop to indicate that it isn't currently on the queue, and check whether it's currently linked before linking or unlinking it.
-
- Feb 10, 2020
-
-
Simon Tatham authored
Reading draft-miller-ssh-agent-04 more carefully, I see that I missed a few things from the extension-message spec. Firstly, there's an extension request "query" which is supposed to list all the extensions you support. Secondly, if you recognise an extension-request name but are then unable to fulfill the request for some other reason, you're supposed to return a new kind of failure message that's distinct from SSH_AGENT_FAILURE, because for extensions, the latter is reserved for "I don't even know what this extension name means at all". I've fixed both of those bugs in Pageant by making a centralised map of known extension names to an enumeration of internal ids, and an array containing the name for each id. So we can reliably answer the "query" extension by iterating over that array, and also use the same array to recognise known extensions up front and give them centralised processing (in particular, resetting the failure-message type) before switching on the particular extension index.
-
- Feb 09, 2020
-
-
Simon Tatham authored
This reads data from standard input, turns it into an SSH-2 sign request, and writes the resulting signature blob to standard output. I don't really anticipate many uses for this other than testing. But it _is_ convenient for testing changes to Pageant itself: it lets me ask for a signature without first having to construct a pointless SSH session that will accept the relevant key.
-
Simon Tatham authored
This will allow it to be used more conveniently for things other than key files. For the moment, the implementation still lives in sshpubk.c. Moving it out into utils.c or misc.c would be nicer, but it has awkward dependencies on marshal.c and the per-platform f_open function. Perhaps another time.
-
- Feb 08, 2020
-
-
Simon Tatham authored
Now I'm able to use the new feature in a less horrible UI, I'm exploring all the code paths that weren't tested before.
-
Simon Tatham authored
This makes all the new deferred-decryption business actually _useful_ for the first time: you can now load an encrypted key file and then get a prompt to decrypt it on first use, without Pageant being in the low-usability debug mode. Currently, the option to present runtime prompts is enabled if Pageant is running with an X display detected, regardless of lifetime mode.
-
Simon Tatham authored
I'm not really sure why that's necessary: by my understanding of the C standard, it shouldn't be. But my observation is that when compiling with {Address,Leak} Sanitiser enabled, pageant --askpass can somehow manage to exit without having actually written the passphrase to its standard output.
-
Simon Tatham authored
I'm about to need to call this from multiple places.
-