Skip to content
Snippets Groups Projects
  1. Apr 10, 2021
    • Simon Tatham's avatar
      Fix a few warnings reported by Visual Studio. · 736646b0
      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.
      736646b0
  2. Apr 02, 2021
    • Jacob Nevins's avatar
      Fix spurious printf format warnings with MinGW. · 44303409
      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.
      44303409
  3. Feb 15, 2020
    • Simon Tatham's avatar
      Pageant client: functions to send reencryption requests. · e563627d
      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.
      e563627d
  4. Feb 09, 2020
    • Simon Tatham's avatar
      New wrapper macro for printf("%zu"), for old VS compat. · 8453b923
      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)
      8453b923
    • Simon Tatham's avatar
      Greatly improve printf format-string checking. · 03f6e883
      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)
      03f6e883
  5. Feb 02, 2020
    • Simon Tatham's avatar
      Expose lf_load_keyfile outside sshpubk.c. · 5db2f4ca
      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.
      5db2f4ca
  6. Jan 26, 2020
    • Simon Tatham's avatar
      New wrapper macro for printf("%zu"), for old VS compat. · 82a7e8c4
      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.
      82a7e8c4
    • Simon Tatham's avatar
      Greatly improve printf format-string checking. · cbfba7a0
      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.
      cbfba7a0
  7. Jun 19, 2019
    • Simon Tatham's avatar
      Make the w32old build warning-clean. · 9dcf781d
      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.
      9dcf781d
  8. Mar 28, 2019
    • Simon Tatham's avatar
      Start of an SSH-server-specific config structure. · 8a884eae
      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.
      8a884eae
  9. Mar 06, 2019
    • Simon Tatham's avatar
      Expose term_translate outside terminal.c. · 0dcdb1b5
      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.
      0dcdb1b5
  10. Feb 20, 2019
    • Simon Tatham's avatar
      New utility object, StripCtrlChars. · 6593009b
      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.
      6593009b
    • Simon Tatham's avatar
      Add BinarySink wrappers on existing forms of output. · bc1aa9c6
      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.
      bc1aa9c6
  11. Jan 23, 2019
    • Simon Tatham's avatar
      Replace PuTTY's PRNG with a Fortuna-like system. · 320bf847
      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.
      320bf847
  12. Jan 18, 2019
    • Simon Tatham's avatar
      Merge the ssh1_cipher type into ssh2_cipher. · 986508a5
      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!)
      986508a5
  13. Jan 04, 2019
    • Simon Tatham's avatar
      Remove a lot of pointless 'struct' keywords. · 35690040
      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.
      35690040
    • Simon Tatham's avatar
      Move defn of PLATFORM_HAS_SMEMCLR into defs.h. · 188e2525
      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.)
      188e2525
  14. Jan 03, 2019
    • Simon Tatham's avatar
      New test system for mp_int and cryptography. · 5b14abc3
      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.
      5b14abc3
  15. Dec 31, 2018
    • Simon Tatham's avatar
      Complete rewrite of PuTTY's bignum library. · 25b034ee
      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.
      25b034ee
  16. Dec 01, 2018
  17. Nov 22, 2018
    • Simon Tatham's avatar
      Fix a build failure. · fa8f1cd9
      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.
      fa8f1cd9
  18. Nov 03, 2018
    • Simon Tatham's avatar
      Adopt C99 <stdbool.h>'s true/false. · a6f1709c
      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.
      a6f1709c
    • Simon Tatham's avatar
      Adopt C99 <stdint.h> integer types. · a647f2ba
      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.
      a647f2ba
  19. Oct 25, 2018
    • Simon Tatham's avatar
      Remove the 'Frontend' type and replace it with a vtable. · 64f8f68a
      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.
      64f8f68a
  20. Oct 21, 2018
    • Simon Tatham's avatar
      Add an SFTP server to the SSH server code. · a081dd0a
      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.
      a081dd0a
    • Simon Tatham's avatar
      Improve sk_peer_info. · 82c83c18
      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.
      82c83c18
    • Simon Tatham's avatar
      Move mainchan into its own file, like agentf. · 431f92ad
      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.
      431f92ad
  21. Oct 11, 2018
    • Simon Tatham's avatar
      New abstraction 'Seat', to pass to backends. · b4c8fd9d
      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.)
      b4c8fd9d
  22. Oct 10, 2018
    • Simon Tatham's avatar
      Refactor the LogContext type. · ad0c502c
      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.
      ad0c502c
  23. Oct 06, 2018
    • Simon Tatham's avatar
      Rename FROMFIELD to 'container_of'. · 9396fcc9
      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 :-)
      9396fcc9
    • Simon Tatham's avatar
      Make Socket and Plug into structs. · 884a7df9
      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.
      884a7df9
    • Simon Tatham's avatar
      Name vtable structure types more consistently. · b7982308
      Simon Tatham authored
      Now they're all called FooVtable, instead of a mixture of that and
      Foo_vtable.
      b7982308
  24. Oct 04, 2018
    • Simon Tatham's avatar
      Get rid of lots of implicit pointer types. · 96ec2c25
      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.
      96ec2c25
  25. Sep 24, 2018
    • Simon Tatham's avatar
      Move most of ssh.c out into separate source files. · 2ca0070f
      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.
      2ca0070f
    • Simon Tatham's avatar
      Replace PktIn reference count with a 'free queue'. · f6f8219a
      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.)
      f6f8219a
    • Simon Tatham's avatar
      Rework special-commands system to add an integer argument. · f4fbaa1b
      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_.
      f4fbaa1b
  26. Sep 21, 2018
    • Simon Tatham's avatar
      Minor header-file cleanups. · a19faa45
      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.)
      a19faa45
  27. Sep 20, 2018
    • Simon Tatham's avatar
      New abstraction 'ConnectionLayer'. · 8001dd4c
      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.
      8001dd4c
    • Simon Tatham's avatar
      Move port-forwarding setup out of ssh.c. · 895b09a4
      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.
      895b09a4
    • Simon Tatham's avatar
      Put a layer of abstraction in front of struct ssh_channel. · aa08e6ca
      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.
      aa08e6ca
Loading