- Apr 10, 2021
-
-
Simon Tatham authored
Many of VS's warnings are too noisy to be useful, but I just tried the experiment of turning off the unrecoverable ones and seeing what was left, and I found a couple of things that actually seem worth fixing. In a few cases in mpint.c, and in one case in sshzlib.c, we had the idiom 'size_t var = 1 << bitpos;', and VS pointed out that when '1' is implicitly a 32-bit int and 'size_t' is 64 bits, this is probably not what you wanted. Writing '(size_t)1 << bitpos' is safer. Secondly, VS complained about lots of functions failing to return a value, or not returning a value on every code path. In every case this was somewhere that we'd used the local unreachable() idiom to indicate that those code paths didn't return at all. So the real problem was that that idiom didn't work in VS. And that's not because VS _can't_ mark functions as noreturn: it has a perfectly good declspec for it. It was just that we hadn't actually _done_ it. Now added a clause in the #if in defs.h that spots VS and uses the declspec.
-
- Apr 02, 2021
-
-
Jacob Nevins authored
Correct MinGW format checking depends on the __MINGW_PRINTF_FORMAT macro, which is defined in its stdio.h. cbfba7a0 moved the use of that macro to before the inclusion of stdio.h, introducing spurious "unknown conversion type character 'z'" and similar warnings.
-
- Feb 15, 2020
-
-
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.
-
- Feb 09, 2020
-
-
Simon Tatham authored
A user reports that Visual Studio 2013 and earlier have printf implementations in their C library that don't support the 'z' modifier to indicate that an integer argument is size_t. The 'I' modifier apparently works in place of it. To avoid littering ifdefs everywhere, I've invented my own inttypes.h style macros to wrap size_t formatting directives, which are defined to %zu and %zx normally, or %Iu and %Ix in old-VS mode. Those are in defs.h, and they're used everywhere that a %z might otherwise get into the Windows build. (cherry picked from commit 82a7e8c4)
-
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)
-
- Feb 02, 2020
-
-
Simon Tatham authored
I'm about to use it in cmdgen for a minor UI improvement. Also, I expect it to be useful in the Pageant client code sooner or later. While I'm here, I've also tweaked its UI a little so that it reports a more precise error, and provided a version that can read from an already open stdio stream.
-
- Jan 26, 2020
-
-
Simon Tatham authored
A user reports that Visual Studio 2013 and earlier have printf implementations in their C library that don't support the 'z' modifier to indicate that an integer argument is size_t. The 'I' modifier apparently works in place of it. To avoid littering ifdefs everywhere, I've invented my own inttypes.h style macros to wrap size_t formatting directives, which are defined to %zu and %zx normally, or %Iu and %Ix in old-VS mode. Those are in defs.h, and they're used everywhere that a %z might otherwise get into the Windows build.
-
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.
-
- Jun 19, 2019
-
-
Simon Tatham authored
Normally I never notice warnings in this build, because it runs inside bob and dumps all the warnings in a part of the build log I never look at. But I've had these fixes lying around for a while and should commit them. They're benign: all we need is an explicit declaration of strtoumax to replace the one that stdlib.h doesn't provide, and a couple more of those annoying NO_TYPECHECK modifiers on GET_WINDOWS_FUNCTION calls.
-
- Mar 28, 2019
-
-
Simon Tatham authored
This is much simpler than Conf, because I don't expect to have to copy it around, load or save it to disk (or the Windows registry), or serialise it between processes. So it can be a straightforward struct. As yet there's nothing actually _in_ it. I've just created the structure and arranged to pass it through to all the SSH layers. But now it's here, it will be a place I can add configuration items as I find I need them.
-
- Mar 06, 2019
-
-
Simon Tatham authored
Also, instead of insisting on modifying the UTF-8 decoding state inside the Terminal structure, it now takes a separate pointer to a small struct containing that decode state. The idea is that if a separate module wants to decode characters the same way the real terminal would, it can pass its own mutable state structure, but the same main Terminal pointer.
-
- Feb 20, 2019
-
-
Simon Tatham authored
This is for sanitising output that's going to be sent to a terminal, if you don't want it to be able to send arbitrary escape sequences and thereby (for example) move the cursor back up to existing text on the screen and overprint it confusingly. It works using the standard C library: we convert to a wide-character string and back, and then use wctype.h to spot control characters in the intermediate form. This means its idea of the conversion character set is locale-based rather than any of our own charset library's fixed settings - which is what you want if the aim is to protect your local terminal (which we assume the system locale represents accurately). This also means that the sanitiser strips things that will _act_ as control characters when sent to the local terminal, whether or not they were intended as control characters by a server that might have had a different character set in mind. Since the main aim is to protect the local terminal rather than to faithfully replicate the server's intention, I think that's the right criterion. It only strips control characters at the charset-independent layer, like backspace, carriage return and the escape character: wctype.h classifies those as control characters, but classifies as printing all of the more Unicode-specific controls like bidirectional overrides. But that's enough to prevent cursor repositioning, for example. stripctrl.c comes with a test main() of its own, which I wasn't able to fold into testcrypt and put in the test suite because of its dependence on the system locale - it wouldn't be guaranteed to work the same way on different test systems anyway. A knock-on build tweak: because you can feed data into this sanitiser in chunks of arbitrary size, including partial multibyte chars, I had to use mbrtowc() for the decoding, and that means that in the 'old' Win32 builds I have to link against the Visual Studio C++ library as well as the C library, because for some reason that's where mbrtowc lived in VS2003.
-
Simon Tatham authored
There's now a stdio_sink, whose write function calls fwrite on the given FILE *; a bufchain_sink, whose write function appends to the given bufchain; and on Windows there's a handle_sink whose write function writes to the given 'struct handle'. (That is, not the raw Windows HANDLE, but our event-loop-friendly wrapper on it.) Not yet used for anything, but they're about to be.
-
- Jan 23, 2019
-
-
Simon Tatham authored
This tears out the entire previous random-pool system in sshrand.c. In its place is a system pretty close to Ferguson and Schneier's 'Fortuna' generator, with the main difference being that I use SHA-256 instead of AES for the generation side of the system (rationale given in comment). The PRNG implementation lives in sshprng.c, and defines a self- contained data type with no state stored outside the object, so you can instantiate however many of them you like. The old sshrand.c still exists, but in place of the previous random pool system, it's just become a client of sshprng.c, whose job is to hold a single global instance of the PRNG type, and manage its reference count, save file, noise-collection timers and similar administrative business. Advantages of this change include: - Fortuna is designed with a more varied threat model in mind than my old home-grown random pool. For example, after any request for random numbers, it automatically re-seeds itself, so that if the state of the PRNG should be leaked, it won't give enough information to find out what past outputs _were_. - The PRNG type can be instantiated with any hash function; the instance used by the main tools is based on SHA-256, an improvement on the old pool's use of SHA-1. - The new PRNG only uses the completely standard interface to the hash function API, instead of having to have privileged access to the internal SHA-1 block transform function. This will make it easier to revamp the hash code in general, and also it means that hardware-accelerated versions of SHA-256 will automatically be used for the PRNG as well as for everything else. - The new PRNG can be _tested_! Because it has an actual (if not quite explicit) specification for exactly what the output numbers _ought_ to be derived from the hashes of, I can (and have) put tests in cryptsuite that ensure the output really is being derived in the way I think it is. The old pool could have been returning any old nonsense and it would have been very hard to tell for sure.
-
- Jan 18, 2019
-
-
Simon Tatham authored
The aim of this reorganisation is to make it easier to test all the ciphers in PuTTY in a uniform way. It was inconvenient that there were two separate vtable systems for the ciphers used in SSH-1 and SSH-2 with different functionality. Now there's only one type, called ssh_cipher. But really it's the old ssh2_cipher, just renamed: I haven't made any changes to the API on the SSH-2 side. Instead, I've removed ssh1_cipher completely, and adapted the SSH-1 BPP to use the SSH-2 style API. (The relevant differences are that ssh1_cipher encapsulated both the sending and receiving directions in one object - so now ssh1bpp has to make a separate cipher instance per direction - and that ssh1_cipher automatically initialised the IV to all zeroes, which ssh1bpp now has to do by hand.) The previous ssh1_cipher vtable for single-DES has been removed completely, because when converted into the new API it became identical to the SSH-2 single-DES vtable; so now there's just one vtable for DES-CBC which works in both protocols. The other two SSH-1 ciphers each had to stay separate, because 3DES is completely different between SSH-1 and SSH-2 (three layers of CBC structure versus one), and Blowfish varies in endianness and key length between the two. (Actually, while I'm here, I've only just noticed that the SSH-1 Blowfish cipher mis-describes itself in log messages as Blowfish-128. In fact it passes the whole of the input key buffer, which has length SSH1_SESSION_KEY_LENGTH == 32 bytes == 256 bits. So it's actually Blowfish-256, and has been all along!)
-
- Jan 04, 2019
-
-
Simon Tatham authored
This is the commit that f3295e0f _should_ have been. Yesterday I just added some typedefs so that I didn't have to wear out my fingers typing 'struct' in new code, but what I ought to have done is to move all the typedefs into defs.h with the rest, and then go through cleaning up the legacy 'struct's all through the existing code. But I was mostly trying to concentrate on getting the test suite finished, so I just did the minimum. Now it's time to come back and do it better.
-
Simon Tatham authored
After I moved parts of misc.c into utils.c, we started getting two versions of smemclr in the Windows builds, because utils.c didn't know to omit its one, having not included the main putty.h. But it was deliberate that utils.c didn't include putty.h, because I wanted it (along with the rest of testcrypt in particular) to be portable to unusual platforms without having to port the whole of the code base. So I've moved into the ubiquitous defs.h just the one decision about whether we're on a platform that will supersede utils.c's definition of smemclr. (Also, in the process of moving it, I've removed the clause that disabled the Windows smemclr in winelib mode, because it looks as if the claim that winelib doesn't have SecureZeroMemory is now out of date.)
-
- Jan 03, 2019
-
-
Simon Tatham authored
I've written a new standalone test program which incorporates all of PuTTY's crypto code, including the mp_int and low-level elliptic curve layers but also going all the way up to the implementations of the MAC, hash, cipher, public key and kex abstractions. The test program itself, 'testcrypt', speaks a simple line-oriented protocol on standard I/O in which you write the name of a function call followed by some inputs, and it gives you back a list of outputs preceded by a line telling you how many there are. Dynamically allocated objects are assigned string ids in the protocol, and there's a 'free' function that tells testcrypt when it can dispose of one. It's possible to speak that protocol by hand, but cumbersome. I've also provided a Python module that wraps it, by running testcrypt as a persistent subprocess and gatewaying all the function calls into things that look reasonably natural to call from Python. The Python module and testcrypt.c both read a carefully formatted header file testcrypt.h which contains the name and signature of every exported function, so it costs minimal effort to expose a given function through this test API. In a few cases it's necessary to write a wrapper in testcrypt.c that makes the function look more friendly, but mostly you don't even need that. (Though that is one of the motivations between a lot of API cleanups I've done recently!) I considered doing Python integration in the more obvious way, by linking parts of the PuTTY code directly into a native-code .so Python module. I decided against it because this way is more flexible: I can run the testcrypt program on its own, or compile it in a way that Python wouldn't play nicely with (I bet compiling just that .so with Leak Sanitiser wouldn't do what you wanted when Python loaded it!), or attach a debugger to it. I can even recompile testcrypt for a different CPU architecture (32- vs 64-bit, or even running it on a different machine over ssh or under emulation) and still layer the nice API on top of that via the local Python interpreter. All I need is a bidirectional data channel.
-
- Dec 31, 2018
-
-
Simon Tatham authored
The old 'Bignum' data type is gone completely, and so is sshbn.c. In its place is a new thing called 'mp_int', handled by an entirely new library module mpint.c, with API differences both large and small. The main aim of this change is that the new library should be free of timing- and cache-related side channels. I've written the code so that it _should_ - assuming I haven't made any mistakes - do all of its work without either control flow or memory addressing depending on the data words of the input numbers. (Though, being an _arbitrary_ precision library, it does have to at least depend on the sizes of the numbers - but there's a 'formal' size that can vary separately from the actual magnitude of the represented integer, so if you want to keep it secret that your number is actually small, it should work fine to have a very long mp_int and just happen to store 23 in it.) So I've done all my conditionalisation by means of computing both answers and doing bit-masking to swap the right one into place, and all loops over the words of an mp_int go up to the formal size rather than the actual size. I haven't actually tested the constant-time property in any rigorous way yet (I'm still considering the best way to do it). But this code is surely at the very least a big improvement on the old version, even if I later find a few more things to fix. I've also completely rewritten the low-level elliptic curve arithmetic from sshecc.c; the new ecc.c is closer to being an adjunct of mpint.c than it is to the SSH end of the code. The new elliptic curve code keeps all coordinates in Montgomery-multiplication transformed form to speed up all the multiplications mod the same prime, and only converts them back when you ask for the affine coordinates. Also, I adopted extended coordinates for the Edwards curve implementation. sshecc.c has also had a near-total rewrite in the course of switching it over to the new system. While I was there, I've separated ECDSA and EdDSA more completely - they now have separate vtables, instead of a single vtable in which nearly every function had a big if statement in it - and also made the externally exposed types for an ECDSA key and an ECDH context different. A minor new feature: since the new arithmetic code includes a modular square root function, we can now support the compressed point representation for the NIST curves. We seem to have been getting along fine without that so far, but it seemed a shame not to put it in, since it was suddenly easy. In sshrsa.c, one major change is that I've removed the RSA blinding step in rsa_privkey_op, in which we randomise the ciphertext before doing the decryption. The purpose of that was to avoid timing leaks giving away the plaintext - but the new arithmetic code should take that in its stride in the course of also being careful enough to avoid leaking the _private key_, which RSA blinding had no way to do anything about in any case. Apart from those specific points, most of the rest of the changes are more or less mechanical, just changing type names and translating code into the new API.
-
- Dec 01, 2018
-
-
Simon Tatham authored
This is mostly to make static analysers and compiler warnings a bit happier - now they know that a call to, say, modalfatalbox() means they don't have to worry about what the rest of the function will do.
-
- Nov 22, 2018
-
-
Simon Tatham authored
When I added a use of PRIx32 to one of Pageant's debugging messages a couple of days ago, I forgot that one of my build setups can't cope with inclusion of <inttypes.h>, and somehow also forgot the precautionary pre-push full build that would have reminded me.
-
- Nov 03, 2018
-
-
Simon Tatham authored
This commit includes <stdbool.h> from defs.h and deletes my traditional definitions of TRUE and FALSE, but other than that, it's a 100% mechanical search-and-replace transforming all uses of TRUE and FALSE into the C99-standardised lowercase spellings. No actual types are changed in this commit; that will come next. This is just getting the noise out of the way, so that subsequent commits can have a higher proportion of signal.
-
Simon Tatham authored
The annoying int64.h is completely retired, since C99 guarantees a 64-bit integer type that you can actually treat like an ordinary integer. Also, I've replaced the local typedefs uint32 and word32 (scattered through different parts of the crypto code) with the standard uint32_t.
-
- Oct 25, 2018
-
-
Simon Tatham authored
After the recent Seat and LogContext revamps, _nearly_ all the remaining uses of the type 'Frontend' were in terminal.c, which needs all sorts of interactions with the GUI window the terminal lives in, from the obvious (actually drawing text on the window, reading and writing the clipboard) to the obscure (minimising, maximising and moving the window in response to particular escape sequences). All of those functions are now provided by an abstraction called TermWin. The few remaining uses of Frontend after _that_ are internal to a particular platform directory, so as to spread the implementation of that particular kind of Frontend between multiple source files; so I've renamed all of those so that they take a more specifically named type that refers to the particular implementation rather than the general abstraction. So now the name 'Frontend' no longer exists in the code base at all, and everywhere one used to be used, it's completely clear whether it was operating in one of Frontend's three abstract roles (and if so, which), or whether it was specific to a particular implementation. Another type that's disappeared is 'Context', which used to be a typedef defined to something different on each platform, describing whatever short-lived resources were necessary to draw on the terminal window: the front end would provide a ready-made one when calling term_paint, and the terminal could request one with get_ctx/free_ctx if it wanted to do proactive window updates. Now that drawing context lives inside the TermWin itself, because there was never any need to have two of those contexts live at the same time. (Another minor API change is that the window-title functions - both reading and writing - have had a missing 'const' added to their char * parameters / return values.) I don't expect this change to enable any particularly interesting new functionality (in particular, I have no plans that need more than one implementation of TermWin in the same application). But it completes the tidying-up that began with the Seat and LogContext rework.
-
- Oct 21, 2018
-
-
Simon Tatham authored
Unlike the traditional Unix SSH server organisation, the SFTP server is built into the same process as all the rest of the code. sesschan.c spots a subsystem request for "sftp", and responds to it by instantiating an SftpServer object and swapping out its own vtable for one that talks to it. (I rather like the idea of an object swapping its own vtable for a different one in the middle of its lifetime! This is one of those tricks that would be absurdly hard to implement in a 'proper' OO language, but when you're doing vtables by hand in C, it's no more difficult than any other piece of ordinary pointer manipulation. As long as the methods in both vtables expect the same physical structure layout, it doesn't cause a problem.) The SftpServer object doesn't deal directly with SFTP packet formats; it implements the SFTP server logic in a more abstract way, by having a vtable method for each SFTP request type with an appropriate parameter list. It sends its replies by calling methods in another vtable called SftpReplyBuilder, which in the normal case will write an SFTP reply packet to send back to the client. So SftpServer can focus more or less completely on the details of a particular filesystem API - and hence, the implementation I've got lives in the unix source directory, and works directly with file descriptors and struct stat and the like. (One purpose of this abstraction layer is that I may well want to write a second dummy implementation, for test-suite purposes, with completely controllable behaviour, and now I have a handy place to plug it in in place of the live filesystem.) In between sesschan's parsing of the byte stream into SFTP packets and the SftpServer object, there's a layer in the new file sftpserver.c which does the actual packet decoding and encoding: each request packet is passed to that, which pulls the fields out of the request packet and calls the appropriate method of SftpServer. It also provides the default SftpReplyBuilder which makes the output packet. I've moved some code out of the previous SFTP client implementation - basic packet construction code, and in particular the BinarySink/ BinarySource marshalling fuinction for fxp_attrs - into sftpcommon.c, so that the two directions can share as much as possible.
-
Simon Tatham authored
Previously, it returned a human-readable string suitable for log files, which tried to say something useful about the remote end of a socket. Now it returns a whole SocketPeerInfo structure, of which that human-friendly log string is just one field, but also some of the same information - remote IP address and port, in particular - is provided in machine-readable form where it's available.
-
Simon Tatham authored
This gets another big pile of logic out of ssh2connection and puts it somewhere more central. Now the only thing left in ssh2connection is the formatting and parsing of the various channel requests; the logic deciding which ones to issue and what to do about them is devolved to the Channel implementation, as it properly should be.
-
- Oct 11, 2018
-
-
Simon Tatham authored
This is a new vtable-based abstraction which is passed to a backend in place of Frontend, and it implements only the subset of the Frontend functions needed by a backend. (Many other Frontend functions still exist, notably the wide range of things called by terminal.c providing platform-independent operations on the GUI terminal window.) The purpose of making it a vtable is that this opens up the possibility of creating a backend as an internal implementation detail of some other activity, by providing just that one backend with a custom Seat that implements the methods differently. For example, this refactoring should make it feasible to directly implement an SSH proxy type, aka the 'jump host' feature supported by OpenSSH, aka 'open a secondary SSH session in MAINCHAN_DIRECT_TCP mode, and then expose the main channel of that as the Socket for the primary connection'. (Which of course you can already do by spawning 'plink -nc' as a separate proxy process, but this would permit it in the _same_ process without anything getting confused.) I've centralised a full set of stub methods in misc.c for the new abstraction, which allows me to get rid of several annoying stubs in the previous code. Also, while I'm here, I've moved a lot of duplicated modalfatalbox() type functions from application main program files into wincons.c / uxcons.c, which I think saves duplication overall. (A minor visible effect is that the prefixes on those console-based fatal error messages will now be more consistent between applications.)
-
- Oct 10, 2018
-
-
Simon Tatham authored
LogContext is now the owner of the logevent() function that back ends and so forth are constantly calling. Previously, logevent was owned by the Frontend, which would store the message into its list for the GUI Event Log dialog (or print it to standard error, or whatever) and then pass it _back_ to LogContext to write to the currently open log file. Now it's the other way round: LogContext gets the message from the back end first, writes it to its log file if it feels so inclined, and communicates it back to the front end. This means that lots of parts of the back end system no longer need to have a pointer to a full-on Frontend; the only thing they needed it for was logging, so now they just have a LogContext (which many of them had to have anyway, e.g. for logging SSH packets or session traffic). LogContext itself also doesn't get a full Frontend pointer any more: it now talks back to the front end via a little vtable of its own called LogPolicy, which contains the method that passes Event Log entries through, the old askappend() function that decides whether to truncate a pre-existing log file, and an emergency function for printing an especially prominent message if the log file can't be created. One minor nice effect of this is that console and GUI apps can implement that last function subtly differently, so that Unix console apps can write it with a plain \n instead of the \r\n (harmless but inelegant) that the old centralised implementation generated. One other consequence of this is that the LogContext has to be provided to backend_init() so that it's available to backends from the instant of creation, rather than being provided via a separate API call a couple of function calls later, because backends have typically started doing things that need logging (like making network connections) before the call to backend_provide_logctx. Fortunately, there's no case in the whole code base where we don't already have logctx by the time we make a backend (so I don't actually remember why I ever delayed providing one). So that shortens the backend API by one function, which is always nice. While I'm tidying up, I've also moved the printf-style logeventf() and the handy logevent_and_free() into logging.c, instead of having copies of them scattered around other places. This has also let me remove some stub functions from a couple of outlying applications like Pageant. Finally, I've removed the pointless "_tag" at the end of LogContext's official struct name.
-
- Oct 06, 2018
-
-
Simon Tatham authored
Ian Jackson points out that the Linux kernel has a macro of this name with the same purpose, and suggests that it's a good idea to use the same name as they do, so that at least some people reading one code base might recognise it from the other. I never really thought very hard about what order FROMFIELD's parameters should go in, and therefore I'm pleasantly surprised to find that my order agrees with the kernel's, so I don't have to permute every call site as part of making this change :-)
-
Simon Tatham authored
I think that means that _every_ one of my traitoids is now a struct containing a vtable pointer as one of its fields (albeit sometimes the only field), and never just a bare pointer.
-
Simon Tatham authored
Now they're all called FooVtable, instead of a mixture of that and Foo_vtable.
-
- Oct 04, 2018
-
-
Simon Tatham authored
All the main backend structures - Ssh, Telnet, Pty, Serial etc - now describe structure types themselves rather than pointers to them. The same goes for the codebase-wide trait types Socket and Plug, and the supporting types SockAddr and Pinger. All those things that were typedefed as pointers are older types; the newer ones have the explicit * at the point of use, because that's what I now seem to be preferring. But whichever one of those is better, inconsistently using a mixture of the two styles is worse, so let's make everything consistent. A few types are still implicitly pointers, such as Bignum and some of the GSSAPI types; generally this is either because they have to be void *, or because they're typedefed differently on different platforms and aren't always pointers at all. Can't be helped. But I've got rid of the main ones, at least.
-
- Sep 24, 2018
-
-
Simon Tatham authored
I've tried to separate out as many individually coherent changes from this work as I could into their own commits, but here's where I run out and have to commit the rest of this major refactoring as a big-bang change. Most of ssh.c is now no longer in ssh.c: all five of the main coroutines that handle layers of the SSH-1 and SSH-2 protocols now each have their own source file to live in, and a lot of the supporting functions have moved into the appropriate one of those too. The new abstraction is a vtable called 'PacketProtocolLayer', which has an input and output packet queue. Each layer's main coroutine is invoked from the method ssh_ppl_process_queue(), which is usually (though not exclusively) triggered automatically when things are pushed on the input queue. In SSH-2, the base layer is the transport protocol, and it contains a pair of subsidiary queues by which it passes some of its packets to the higher SSH-2 layers - first userauth and then connection, which are peers at the same level, with the former abdicating in favour of the latter at the appropriate moment. SSH-1 is simpler: the whole login phase of the protocol (crypto setup and authentication) is all in one module, and since SSH-1 has no repeat key exchange, that setup layer abdicates in favour of the connection phase when it's done. ssh.c itself is now about a tenth of its old size (which all by itself is cause for celebration!). Its main job is to set up all the layers, hook them up to each other and to the BPP, and to funnel data back and forth between that collection of modules and external things such as the network and the terminal. Once it's set up a collection of packet protocol layers, it communicates with them partly by calling methods of the base layer (and if that's ssh2transport then it will delegate some functionality to the corresponding methods of its higher layer), and partly by talking directly to the connection layer no matter where it is in the stack by means of the separate ConnectionLayer vtable which I introduced in commit 8001dd4c, and to which I've now added quite a few extra methods replacing services that used to be internal function calls within ssh.c. (One effect of this is that the SSH-1 and SSH-2 channel storage is now no longer shared - there are distinct struct types ssh1_channel and ssh2_channel. That means a bit more code duplication, but on the plus side, a lot fewer confusing conditionals in the middle of half-shared functions, and less risk of a piece of SSH-1 escaping into SSH-2 or vice versa, which I remember has happened at least once in the past.) The bulk of this commit introduces the five new source files, their common header sshppl.h and some shared supporting routines in sshcommon.c, and rewrites nearly all of ssh.c itself. But it also includes a couple of other changes that I couldn't separate easily enough: Firstly, there's a new handling for socket EOF, in which ssh.c sets an 'input_eof' flag in the BPP, and that responds by checking a flag that tells it whether to report the EOF as an error or not. (This is the main reason for those new BPP_READ / BPP_WAITFOR macros - they can check the EOF flag every time the coroutine is resumed.) Secondly, the error reporting itself is changed around again. I'd expected to put some data fields in the public PacketProtocolLayer structure that it could set to report errors in the same way as the BPPs have been doing, but in the end, I decided propagating all those data fields around was a pain and that even the BPPs shouldn't have been doing it that way. So I've reverted to a system where everything calls back to functions in ssh.c itself to report any connection- ending condition. But there's a new family of those functions, categorising the possible such conditions by semantics, and each one has a different set of detailed effects (e.g. how rudely to close the network connection, what exit status should be passed back to the whole application, whether to send a disconnect message and/or display a GUI error box). I don't expect this to be immediately perfect: of course, the code has been through a big upheaval, new bugs are expected, and I haven't been able to do a full job of testing (e.g. I haven't tested every auth or kex method). But I've checked that it _basically_ works - both SSH protocols, all the different kinds of forwarding channel, more than one auth method, Windows and Linux, connection sharing - and I think it's now at the point where the easiest way to find further bugs is to let it out into the wild and see what users can spot.
-
Simon Tatham authored
This is a new idea I've had to make memory-management of PktIn even easier. The idea is that a PktIn is essentially _always_ an element of some linked-list queue: if it's not one of the queues by which packets move through ssh.c, then it's a special 'free queue' which holds packets that are unowned and due to be freed. pq_pop() on a PktInQueue automatically relinks the packet to the free queue, and also triggers an idempotent callback which will empty the queue and really free all the packets on it. Hence, you can pop a packet off a real queue, parse it, handle it, and then just assume it'll get tidied up at some point - the only constraint being that you have to finish with it before returning to the application's main loop. The exception is that it's OK to pq_push() the packet back on to some other PktInQueue, because a side effect of that will be to _remove_ it from the free queue again. (And if _all_ the incoming packets get that treatment, then when the free-queue handler eventually runs, it may find it has nothing to do - which is harmless.)
-
Simon Tatham authored
In order to list cross-certifiable host keys in the GUI specials menu, the SSH backend has been inventing new values on the end of the Telnet_Special enumeration, starting from the value TS_LOCALSTART. This is inelegant, and also makes it awkward to break up special handlers (e.g. to dispatch different specials to different SSH layers), since if all you know about a special is that it's somewhere in the TS_LOCALSTART+n space, you can't tell what _general kind_ of thing it is. Also, if I ever need another open-ended set of specials in future, I'll have to remember which TS_LOCALSTART+n codes are in which set. So here's a revamp that causes every special to take an extra integer argument. For all previously numbered specials, this argument is passed as zero and ignored, but there's a new main special code for SSH host key cross-certification, in which the integer argument is an index into the backend's list of available keys. TS_LOCALSTART is now a thing of the past: if I need any other open-ended sets of specials in future, I can add a new top-level code with a nicely separated space of arguments. While I'm at it, I've removed the legacy misnomer 'Telnet_Special' from the code completely; the enum is now SessionSpecialCode, the struct containing full details of a menu entry is SessionSpecial, and the enum values now start SS_ rather than TS_.
-
- Sep 21, 2018
-
-
Simon Tatham authored
Moved the typedef of BinaryPacketProtocol into defs.h on the general principle that it's just the kind of thing that ought to go there; also removed the declaration of pq_base_init from ssh.h on the grounds that there's never been any such function! (At least, not in public source control - it existed in an early draft of commit 6e24b7d5.)
-
- Sep 20, 2018
-
-
Simon Tatham authored
This is a vtable that wraps up all the functionality required from the SSH connection layer by associated modules like port forwarding and connection sharing. This extra layer of indirection adds nothing useful right now, but when I later separate the SSH-1 and SSH-2 connection layer implementations, it will be convenient for each one to be able to implement this vtable in terms of its own internal data structures. To simplify this vtable, I've moved a lot of the logging duties relating to connection sharing out of ssh.c into sshshare.c: now it handles nearly all the logging itself relating to setting up connection sharing in the first place and downstreams connecting and disconnecting. The only exception is the 'Reusing a shared connection' announcement in the console window, which is now done in ssh.c by detecting downstream status immediately after setup.
-
Simon Tatham authored
The tree234 storing currently active port forwardings - both local and remote - now lives in portfwd.c, as does the complicated function that updates it based on a Conf listing the new set of desired forwardings. Local port forwardings are passed to ssh.c via the same route as before - once the listening port receives a connection and portfwd.c knows where it should be directed to (in particular, after the SOCKS exchange, if any), it calls ssh_send_port_open. Remote forwardings are now initiated by calling ssh_rportfwd_alloc, which adds an entry to the rportfwds tree (which _is_ still in ssh.c, and still confusingly sorted by a different criterion depending on SSH protocol version) and sends out the appropriate protocol request. ssh_rportfwd_remove cancels one again, sending a protocol request too. Those functions look enough like ssh_{alloc,remove}_sharing_rportfwd that I've merged those into the new pair as well - now allocating an rportfwd allows you to specify either a destination host/port or a sharing context, and returns a handy pointer you can use to cancel the forwarding later.
-
Simon Tatham authored
Clients outside ssh.c - all implementations of Channel - will now not see the ssh_channel data type itself, but only a subobject of the interface type SshChannel. All the sshfwd_* functions have become methods in that interface type's vtable (though, wrapped in the usual kind of macros, the call sites look identical). This paves the way for me to split up the SSH-1 and SSH-2 connection layers and have each one lay out its channel bookkeeping structure as it sees fit; as long as they each provide an implementation of the sshfwd_ method family, the types behind that need not look different. A minor good effect of this is that the sshfwd_ methods are no longer global symbols, so they don't have to be stubbed in Unix Pageant to get it to compile.
-