- Jun 12, 2021
-
-
Simon Tatham authored
A user pointed out that once we've identified the key algorithm from an apparent public-key blob, we call ssh_key_new_pub on the blob data and assume it will succeed. But there are plenty of ways it could still fail, and ssh_key_new_pub could return NULL. (cherry picked from commit 0c21eb44)
-
- Apr 20, 2021
-
-
Jacob Nevins authored
No functional change, probably.
-
- Apr 19, 2021
-
-
Jacob Nevins authored
PuTTYgen and its documentation are pretty consistent about calling their encryption key a 'passphrase', as opposed to a 'password' supplied directly to a server; but the Argon2 parameters UI reverted to 'password hash', which seemed unecessarily confusing. I think it's better to use the term 'passphrase' consistently in the UI. (People who are used to Argon2 being called a 'password hash' can probably deal.) This required tweaking the coordinates of the Windows PuTTYgen UI.
-
Jacob Nevins authored
Instead of 'Kb', which could be misread as 'Kbit'.
-
- Apr 07, 2021
-
-
Jacob Nevins authored
This seems more useful than the previous behaviour of not prompting for a passphrase and only emitting the public part; if we want that back I suppose we could invent a "-O text-public". Also, document the text dump format a bit in the man page.
-
- Mar 13, 2021
-
-
Simon Tatham authored
I've added the -E option, similar to ssh-keygen's, and cgtest checks it against the OpenSSH version to ensure they match.
-
Simon Tatham authored
There's a new enumeration of fingerprint types, and you tell ssh2_fingerprint() or ssh2_fingerprint_blob() which of them to use. So far, this is only implemented behind the scenes, and exposed for testcrypt to test. All the call sites of ssh2_fingerprint pass a fixed default fptype, which is still set to the old MD5. That will change shortly.
-
- Feb 23, 2021
-
-
Simon Tatham authored
I left this out of yesterday's collection of cmdgen CLI options and GUI PuTTYgen dialog box, but only because I forgot about it. I don't know off the top of my head why someone would particularly want to configure this detail, but given that it _is_ configurable, it seems like no extra trouble to expose it along with the rest of the parameters, just in case.
-
- Feb 22, 2021
-
-
Simon Tatham authored
This allows you to manually adjust the Argon2 parameters so that you can trade off CPU requirements in legitimate use against difficulty of brute-force attack. It also allows downgrading the key file version back to the widespread PPK v2, so you can manually back-port a key that you accidentally generated too new.
-
Simon Tatham authored
This allows you to load and save the same key without making any semantic changes to it. Currently, you can only do that by pretending to make a change, like changing the passphrase or the comment to the same thing it was before. With two key file formats now supported, and a bunch of reconfigurable parameters in the v3 key derivation, it's now more likely that you'd want to re-encrypt the same key in a different way, to upgrade or downgrade or tinker with it. (Or perhaps even just re-randomise the salt, so that someone reading the key file doesn't know _whether_ you've changed the passphrase!)
-
- Feb 20, 2021
-
-
Simon Tatham authored
This removes both uses of SHA-1 in the file format: it was used as the MAC protecting the key file against tamperproofing, and also used in the key derivation step that converted the user's passphrase to cipher and MAC keys. The MAC is simply upgraded from HMAC-SHA-1 to HMAC-SHA-256; it is otherwise unchanged in how it's applied (in particular, to what data). The key derivation is totally reworked, to be based on Argon2, which I've just added to the code base. This should make stolen encrypted key files more resistant to brute-force attack. Argon2 has assorted configurable parameters for memory and CPU usage; the new key format includes all those parameters. So there's no reason we can't have them under user control, if a user wants to be particularly vigorous or particularly lightweight with their own key files. They could even switch to one of the other flavours of Argon2, if they thought side channels were an especially large or small risk in their particular environment. In this commit I haven't added any UI for controlling that kind of thing, but the PPK loading function is all set up to cope, so that can all be added in a future commit without having to change the file format. While I'm at it, I've also switched the CBC encryption to using a random IV (or rather, one derived from the passphrase along with the cipher and MAC keys). That's more like normal SSH-2 practice.
-
- Mar 10, 2020
-
-
Simon Tatham authored
This is a sweeping change applied across the whole code base by a spot of Emacs Lisp. Now, everywhere I declare a vtable filled with function pointers (and the occasional const data member), all the members of the vtable structure are initialised by name using the '.fieldname = value' syntax introduced in C99. We were already using this syntax for a handful of things in the new key-generation progress report system, so it's not new to the code base as a whole. The advantage is that now, when a vtable only declares a subset of the available fields, I can initialise the rest to NULL or zero just by leaving them out. This is most dramatic in a couple of the outlying vtables in things like psocks (which has a ConnectionLayerVtable containing only one non-NULL method), but less dramatically, it means that the new 'flags' field in BackendVtable can be completely left out of every backend definition except for the SUPDUP one which defines it to a nonzero value. Similarly, the test_for_upstream method only used by SSH doesn't have to be mentioned in the rest of the backends; network Plugs for listening sockets don't have to explicitly null out 'receive' and 'sent', and vice versa for 'accepting', and so on. While I'm at it, I've normalised the declarations so they don't use the unnecessarily verbose 'struct' keyword. Also a handful of them weren't const; now they are.
-
- Mar 07, 2020
-
-
Simon Tatham authored
A 'strong' prime, as defined by the Handbook of Applied Cryptography, is a prime p such that each of p-1 and p+1 has a large prime factor, and that the large factor q of p-1 is such that q-1 in turn _also_ has a large prime factor. HoAC says that making your RSA key using primes of this form defeats some factoring algorithms - but there are other faster algorithms to which it makes no difference. So this is probably not a useful precaution in practice. However, it has been recommended in the past by some official standards, and it's easy to implement given the new general facility in PrimeCandidateSource that lets you ask for your prime to satisfy an arbitrary modular congruence. (And HoAC also says there's no particular reason _not_ to use strong primes.) So I provide it as an option, just in case anyone wants to select it. The change to the key generation algorithm is entirely in sshrsag.c, and is neatly independent of the prime-generation system in use. If you're using Maurer provable prime generation, then the known factor q of p-1 can be used to help certify p, and the one for q-1 to help with q in turn; if you switch to probabilistic prime generation then you still get an RSA key with the right structure, except that every time the definition says 'prime factor' you just append '(probably)'. (The probabilistic version of this procedure is described as 'Gordon's algorithm' in HoAC section 4.4.2.)
-
- Mar 02, 2020
-
-
Simon Tatham authored
This is standardised by RFC 8709 at SHOULD level, and for us it's not too difficult (because we use general-purpose elliptic-curve code). So let's be up to date for a change, and add it. This implementation uses all the formats defined in the RFC. But we also have to choose a wire format for the public+private key blob sent to an agent, and since the OpenSSH agent protocol is the de facto standard but not (yet?) handled by the IETF, OpenSSH themselves get to say what the format for a key should or shouldn't be. So if they don't support a particular key method, what do you do? I checked with them, and they agreed that there's an obviously right format for Ed448 keys, which is to do them exactly like Ed25519 except that you have a 57-byte string everywhere Ed25519 had a 32-byte string. So I've done that.
-
Simon Tatham authored
In the Windows GUI, all the controls that were previously named or labelled Ed25519 are now labelled EdDSA, and when you select that top-level key type, there's a dropdown for the specific curve (just like for ECDSA), whose only current value is Ed25519. In command-line PuTTYgen, you can say '-t eddsa' and give a number of bits, just like '-t ecdsa'. You can also still say '-t ed25519', for backwards compatibility. Also in command-line PuTTYgen, I've reworked the error messages if you give a number of bits that doesn't correspond to a known elliptic curve. Now the messages are generated by consulting the list of curves, so that that list has to be updated by hand in one fewer place.
-
- Mar 01, 2020
-
-
Simon Tatham authored
There's always something. I added the actual option, but forgot to advertise it in the online help. While I'm here, I've also allowed the word 'proven' as an alternative spelling for 'provable', because having the approved spellings be 'probable' and 'provable' is just asking for hilarious typos.
-
Simon Tatham authored
In Windows PuTTYgen, this is selected by an extra set of radio-button style menu options in the Key menu. In the command-line version, there's a new --primes=provable option. This whole system is new, so I'm not enabling it by default just yet. I may in future, though: it's running faster than I expected (in particular, a lot faster than any previous prototype of the same algorithm I attempted in standalone Python).
-
Simon Tatham authored
The old system I removed in commit 79d3c178 had both linear and exponential phase types, but the new one only had exponential, because at that point I'd just thrown away all the clients of the linear phase type. But I'm going to add another one shortly, so I have to put it back in.
-
Simon Tatham authored
The functions primegen() and primegen_add_progress_phase() are gone. In their place is a small vtable system with two methods corresponding to them, plus the usual admin of allocating and freeing contexts. This API change is the starting point for being able to drop in different prime generation algorithms at run time in response to user configuration.
-
- Feb 29, 2020
-
-
Simon Tatham authored
The old API was one of those horrible things I used to do when I was young and foolish, in which you have just one function, and indicate which of lots of things it's doing by passing in flags. It was crying out to be replaced with a vtable. While I'm at it, I've reworked the code on the Windows side that decides what to do with the progress bar, so that it's based on actually justifiable estimates of probability rather than magic integer constants. Since computers are generally faster now than they were at the start of this project, I've also decided there's no longer any point in making the fixed final part of RSA key generation bother to report progress at all. So the progress bars are now only for the variable part, i.e. the actual prime generations. (This is a reapplication of commit a7bdefb3, without the Miller-Rabin refactoring accidentally folded into it. Also this time I've added -lm to the link options, which for some reason _didn't_ cause me a link failure last time round. No idea why not.)
-
Simon Tatham authored
This reverts commit a7bdefb3. I had accidentally mashed it together with another commit. I did actually want to push both of them, but I'd rather push them separately! So I'm backing out the combined blob, and I'll re-push them with their proper comments and explanations.
-
Simon Tatham authored
The old API was one of those horrible things I used to do when I was young and foolish, in which you have just one function, and indicate which of lots of things it's doing by passing in flags. It was crying out to be replaced with a vtable. While I'm at it, I've reworked the code on the Windows side that decides what to do with the progress bar, so that it's based on actually justifiable estimates of probability rather than magic integer constants. Since computers are generally faster now than they were at the start of this project, I've also decided there's no longer any point in making the fixed final part of RSA key generation bother to report progress at all. So the progress bars are now only for the variable part, i.e. the actual prime generations.
-
- Feb 23, 2020
-
-
Simon Tatham authored
There's always something.
-
- Feb 22, 2020
-
-
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.
-
- 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 09, 2020
-
-
Simon Tatham authored
(cherry picked from commit c25dc9c2)
-
Simon Tatham authored
In commit b4c8fd9d which introduced the Seat trait, I got a bit confused about the prototype of new_prompts(). Previously it took a 'Frontend *' parameter; I edited the call sites to pass a 'Seat *' instead, but the actual function definition takes no parameters at all - and rightly so, because the 'Frontend *' inside the prompts_t has been removed and _not_ replaced with a 'Seat *', so the constructor would have nothing to do with such a thing anyway. But I wrote the function declaration in putty.h with '()' rather than '(void)' (too much time spent in C++), and so the compiler never spotted the mismatch. Now new_prompts() is consistently nullary everywhere it appears: the prototype in the header is a proper (void) one, and the call sites have been modified to not pointlessly give it a Seat or null pointer. (cherry picked from commit d1834847)
-
Simon Tatham authored
I've added the gcc-style attribute("printf") to a lot of printf-shaped functions in this code base that didn't have it. To make that easier, I moved the wrapping macro into defs.h, and also enabled it if we detect the __clang__ macro as well as __GNU__ (hence, it will be used when building for Windows using clang-cl). The result is that a great many format strings in the code are now checked by the compiler, where they were previously not. This causes build failures, which I'll fix in the next commit. (cherry picked from commit cbfba7a0)
-
Simon Tatham authored
UBsan pointed out another memcpy from NULL (again with length 0) in the prompts_t system. When I looked at it, I realised that firstly prompt_ensure_result_size was an early not-so-good implementation of sgrowarray_nm that would benefit from being replaced with a call to the real one, and secondly, the whole system for storing prompt results should really have been replaced with strbufs with the no-move option, because that's doing all the same jobs better. So, now each prompt_t holds a strbuf in place of its previous manually managed string. prompt_ensure_result_size is gone (the console prompt-reading functions use strbuf_append, and everything else just adds to the strbuf in the usual marshal.c way). New functions exist to retrieve a prompt_t's result, either by reference or copied. (cherry picked from commit cd6bc14f)
-
Simon Tatham authored
In setting up the ECC tests for cmdgen, I noticed that OpenSSH and PuTTYgen disagree on the bit length to put in a key fingerprint for an ed25519 key: we think 255, they think 256. On reflection, I think 255 is more accurate, which is why I bodged get_fp() in the test suite to ignore that difference when checking our key fingerprint against OpenSSH's. But having done that, it now seems silly that if you unnecessarily specify a bit count at ed25519 generation time, cmdgen will insist that it be 256! 255 is now permitted everywhere an ed25519 bit count is input. 256 is also still allowed for backwards compatibility but 255 is preferred by the error message if you give any other value. (cherry picked from commit 187cc8bf)
-
Pavel I. Kryukov authored
(cherry picked from commit 83408f92)
-
- Feb 02, 2020
-
-
Simon Tatham authored
I don't really know why it was still in cmdgen.c at all. There's no reason it shouldn't live in its own source file, and keep cmdgen.c for the actual code of the key generation program!
-
Simon Tatham authored
This reworks the cmdgen main program so that it loads the input file into a LoadedFile right at the start, and then every time it needs to do something with the contents, it calls one of the API functions taking a BinarySource instead of one taking a Filename. The usefulness of this is that now we can read from things that aren't regular files, and can't be rewound or reopened. In particular, the filename "-" is now taken (per the usual convention) to mean standard input. So now you can pipe a public or private key file into cmdgen's standard input and have it do something useful. For example, I was recently experimenting with the SFTP-only SSH server that comes with 'proftpd', which keeps its authorized_keys file in RFC 4716 format instead of the OpenSSH one-liner format, and I found I wanted to do grep 'my-key-comment' ~/.ssh/authorized_keys | puttygen -p - to quickly get hold of my existing public key to put in that file. But I had to go via a temporary file to make that work, because puttygen couldn't read from standard input. Next time, it will be able to!
-
Simon Tatham authored
-
Simon Tatham authored
In the previous trawl of this, I didn't bother with the statics in main-program modules, on the grounds that my main aim was to avoid 'library' objects (shared between multiple programs) from polluting the global namespace. But I think it's worth being more strict after all, so this commit adds 'static' to a lot more file-scope variables that aren't needed outside their own module.
-
Simon Tatham authored
Now it's no longer used, we can get rid of it, and better still, get rid of every #define PUTTY_DO_GLOBALS in the many source files that previously had them.
-
- Jan 29, 2020
-
-
Simon Tatham authored
In commit b4c8fd9d which introduced the Seat trait, I got a bit confused about the prototype of new_prompts(). Previously it took a 'Frontend *' parameter; I edited the call sites to pass a 'Seat *' instead, but the actual function definition takes no parameters at all - and rightly so, because the 'Frontend *' inside the prompts_t has been removed and _not_ replaced with a 'Seat *', so the constructor would have nothing to do with such a thing anyway. But I wrote the function declaration in putty.h with '()' rather than '(void)' (too much time spent in C++), and so the compiler never spotted the mismatch. Now new_prompts() is consistently nullary everywhere it appears: the prototype in the header is a proper (void) one, and the call sites have been modified to not pointlessly give it a Seat or null pointer.
-
- Jan 26, 2020
-
-
Simon Tatham authored
I've added the gcc-style attribute("printf") to a lot of printf-shaped functions in this code base that didn't have it. To make that easier, I moved the wrapping macro into defs.h, and also enabled it if we detect the __clang__ macro as well as __GNU__ (hence, it will be used when building for Windows using clang-cl). The result is that a great many format strings in the code are now checked by the compiler, where they were previously not. This causes build failures, which I'll fix in the next commit.
-
- Jan 21, 2020
-
-
Simon Tatham authored
UBsan pointed out another memcpy from NULL (again with length 0) in the prompts_t system. When I looked at it, I realised that firstly prompt_ensure_result_size was an early not-so-good implementation of sgrowarray_nm that would benefit from being replaced with a call to the real one, and secondly, the whole system for storing prompt results should really have been replaced with strbufs with the no-move option, because that's doing all the same jobs better. So, now each prompt_t holds a strbuf in place of its previous manually managed string. prompt_ensure_result_size is gone (the console prompt-reading functions use strbuf_append, and everything else just adds to the strbuf in the usual marshal.c way). New functions exist to retrieve a prompt_t's result, either by reference or copied.
-
- Jan 14, 2020
-
-
Simon Tatham authored
This stops cgtest from leaving detritus all over my git checkout. There's a --keep option to revert to the previous behaviour, just in case I actually want the detritus on some occasion - although in that situation I might also need to arrange that the various intermediate files all go by different names, because otherwise there's a good chance that the one I cared about would already have been overwritten.
-