Skip to content
Snippets Groups Projects
  1. Mar 10, 2020
    • Simon Tatham's avatar
      Change vtable defs to use C99 designated initialisers. · b4e1bca2
      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.
      b4e1bca2
  2. Feb 12, 2020
  3. Feb 07, 2020
  4. Jan 29, 2020
    • Simon Tatham's avatar
      Add lots of missing 'static' keywords. · 8d747d80
      Simon Tatham authored
      A trawl through the code with -Wmissing-prototypes and
      -Wmissing-variable-declarations turned up a lot of things that should
      have been internal to a particular source file, but were accidentally
      global. Keep the namespace clean by making them all static.
      
      (Also, while I'm here, a couple of them were missing a 'const': the
      ONE and ZERO arrays in sshcrcda.c, and EMPTY_WINDOW_TITLE in
      terminal.c.)
      8d747d80
  5. Jan 27, 2020
    • Simon Tatham's avatar
      Merge the two low-level portfwd setup systems. · 2160205a
      Simon Tatham authored
      In commit 09954a87 I introduced the portfwdmgr_connect_socket()
      system, which opened a port forwarding given a callback to create the
      Socket itself, with the aim of using it to make forwardings to Unix-
      domain sockets and Windows named pipes (both initially for agent
      forwarding).
      
      But I forgot that a year and a bit ago, in commit 83439617, I already
      introduced a similar low-level system for creating a PortForwarding
      around an unusual kind of socket: the portfwd_raw_new() system, which
      in place of a callback uses a two-phase setup protocol (you create the
      socket in between the two setup calls, and can roll it back if the
      socket can't be created).
      
      There's really no need to have _both_ these systems! So now I'm
      merging them, which is to say, I'm enhancing portfwd_raw_new to have
      the one new feature it needs, and throwing away the newer system
      completely. The new feature is to be able to control the initial state
      of the 'ready' flag: portfwd_raw_new was always used for initiating
      port forwardings in response to an incoming local connection, which
      means you need to start off with ready=false and set it true when the
      other end of the SSH connection sends back OPEN_CONFIRMATION. Now it's
      being used for initiating port forwardings in response to a
      CHANNEL_OPEN, we need to be able to start with ready=true.
      
      This commit reverts 09954a87 and its
      followup fix 12aa06cc, and simplifies
      the agent_connect system down to a single trivial function that makes
      a Socket given a Plug.
      2160205a
  6. Jan 14, 2020
    • Simon Tatham's avatar
      Fix double-free in remote->local forwardings. · 12aa06cc
      Simon Tatham authored
      This bug applies to both the new stream-based agent forwarding, and
      ordinary remote->local TCP port forwardings, because it was introduced
      by the preliminary infrastructure in commit 09954a87.
      
      new_connection() and sk_new() accept a SockAddr *, and take ownership
      of it. So it's a mistake to make an address, connect to it, and then
      sk_addr_free() it: the free will decrement its reference count to
      zero, and then the Socket made by the connection will be holding a
      stale pointer. But that's exactly what I was doing in the version of
      portfwdmgr_connect() that I rewrote in that refactoring. And then I
      made the same error again in commit ae114826 in the Unix stream-based
      agent forwarding.
      
      Now both fixed. Rather than remove the sk_addr_free() to make the code
      look more like it used to, I've instead solved the problem by adding
      an sk_addr_dup() at the point of making the connection. The idea is
      that that should be more robust, in that it will still do the right
      thing if portfwdmgr_connect_socket should later change so as not to
      call its connect helper function at all.
      
      The new Windows stream-based agent forwarding is unaffected by this
      bug, because it calls new_named_pipe_client() with a pathname in
      string format, without first wrapping it into a SockAddr.
      12aa06cc
  7. Jan 04, 2020
    • Simon Tatham's avatar
      Low-level API to open nonstandard port forwardings. · 09954a87
      Simon Tatham authored
      The new portfwdmgr_connect_socket() works basically like the existing
      portfwdmgr_connect(), in that it opens an SSH forwarding channel and
      gateways it to a Socket. But where portfwdmgr_connect() started from a
      (hostname,port) pair and used mgr->conf to inform name lookup and
      proxy settings, portfwdmgr_connect_socket() simply takes a callback
      that it will call when it wants you to make a Socket for a given Plug,
      and that callback can make any kind of Socket it likes.
      
      The idea is that this way you can make port forwardings that talk to
      things other than genuine TCP connections, by simply providing an
      appropriate callback.
      
      My immediate use case for this is for agent forwarding, and will
      appear in the next commit. But it's easy to imagine other purposes you
      might use a thing like this for, such as forwarding SSH channels to
      AF_UNIX sockets in general.
      09954a87
  8. Sep 08, 2019
    • Simon Tatham's avatar
      Whitespace rationalisation of entire code base. · 5d718ef6
      Simon Tatham authored
      The number of people has been steadily increasing who read our source
      code with an editor that thinks tab stops are 4 spaces apart, as
      opposed to the traditional tty-derived 8 that the PuTTY code expects.
      
      So I've been wondering for ages about just fixing it, and switching to
      a spaces-only policy throughout the code. And I recently found out
      about 'git blame -w', which should make this change not too disruptive
      for the purposes of source-control archaeology; so perhaps now is the
      time.
      
      While I'm at it, I've also taken the opportunity to remove all the
      trailing spaces from source lines (on the basis that git dislikes
      them, and is the only thing that seems to have a strong opinion one
      way or the other).
          
      Apologies to anyone downstream of this code who has complicated patch
      sets to rebase past this change. I don't intend it to be needed again.
      5d718ef6
  9. Mar 25, 2019
    • Simon Tatham's avatar
      portfwdmgr_config: null out pointers we're destroying. · fe408562
      Simon Tatham authored
      In particular, a report today pointed out that the call to
      pfl_terminate(pfr->local) directly from portfwdmgr_config() was then
      repeated from inside pfr_free(pfr) which we called four lines later,
      leading to a double-free crash. Now we null out pfr->local the first
      time, so the call in pfr_free is skipped.
      
      While I'm at it, I've nulled out pfr->remote similarly; that doesn't
      cause any crash that I can see, but it's a good habit to get into for
      futureproofing.
      fe408562
  10. Feb 06, 2019
    • Simon Tatham's avatar
      Make lots of 'int' length fields into size_t. · 0cda34c6
      Simon Tatham authored
      This is a general cleanup which has been overdue for some time: lots
      of length fields are now the machine word type rather than the (in
      practice) fixed 'int'.
      0cda34c6
    • Simon Tatham's avatar
      Add some missing 'const'. · 0aa8cf7b
      Simon Tatham authored
      plug_receive(), sftp_senddata() and handle_gotdata() in particular now
      take const pointers. Also fixed 'char *receive_data' in struct
      ProxySocket.
      0aa8cf7b
  11. Jan 03, 2019
    • Simon Tatham's avatar
      Replace assert(false) with an unreachable() macro. · 0112936e
      Simon Tatham authored
      Taking a leaf out of the LLVM code base: this macro still includes an
      assert(false) so that the message will show up in a typical build, but
      it follows it up with a call to a function explicitly marked as no-
      return.
      
      So this ought to do a better job of convincing compilers that once a
      code path hits this function it _really doesn't_ have to still faff
      about with making up a bogus return value or filling in a variable
      that 'might be used uninitialised' in the following code that won't be
      reached anyway.
      
      I've gone through the existing code looking for the assert(false) /
      assert(0) idiom and replaced all the ones I found with the new macro,
      which also meant I could remove a few pointless return statements and
      variable initialisations that I'd already had to put in to placate
      compiler front ends.
      0112936e
  12. Nov 03, 2018
    • Simon Tatham's avatar
      Convert a lot of 'int' variables to 'bool'. · 3214563d
      Simon Tatham authored
      My normal habit these days, in new code, is to treat int and bool as
      _almost_ completely separate types. I'm still willing to use C's
      implicit test for zero on an integer (e.g. 'if (!blob.len)' is fine,
      no need to spell it out as blob.len != 0), but generally, if a
      variable is going to be conceptually a boolean, I like to declare it
      bool and assign to it using 'true' or 'false' rather than 0 or 1.
      
      PuTTY is an exception, because it predates the C99 bool, and I've
      stuck to its existing coding style even when adding new code to it.
      But it's been annoying me more and more, so now that I've decided C99
      bool is an acceptable thing to require from our toolchain in the first
      place, here's a quite thorough trawl through the source doing
      'boolification'. Many variables and function parameters are now typed
      as bool rather than int; many assignments of 0 or 1 to those variables
      are now spelled 'true' or 'false'.
      
      I managed this thorough conversion with the help of a custom clang
      plugin that I wrote to trawl the AST and apply heuristics to point out
      where things might want changing. So I've even managed to do a decent
      job on parts of the code I haven't looked at in years!
      
      To make the plugin's work easier, I pushed platform front ends
      generally in the direction of using standard 'bool' in preference to
      platform-specific boolean types like Windows BOOL or GTK's gboolean;
      I've left the platform booleans in places they _have_ to be for the
      platform APIs to work right, but variables only used by my own code
      have been converted wherever I found them.
      
      In a few places there are int values that look very like booleans in
      _most_ of the places they're used, but have a rarely-used third value,
      or a distinction between different nonzero values that most users
      don't care about. In these cases, I've _removed_ uses of 'true' and
      'false' for the return values, to emphasise that there's something
      more subtle going on than a simple boolean answer:
       - the 'multisel' field in dialog.h's list box structure, for which
         the GTK front end in particular recognises a difference between 1
         and 2 but nearly everything else treats as boolean
       - the 'urgent' parameter to plug_receive, where 1 vs 2 tells you
         something about the specific location of the urgent pointer, but
         most clients only care about 0 vs 'something nonzero'
       - the return value of wc_match, where -1 indicates a syntax error in
         the wildcard.
       - the return values from SSH-1 RSA-key loading functions, which use
         -1 for 'wrong passphrase' and 0 for all other failures (so any
         caller which already knows it's not loading an _encrypted private_
         key can treat them as boolean)
       - term->esc_query, and the 'query' parameter in toggle_mode in
         terminal.c, which _usually_ hold 0 for ESC[123h or 1 for ESC[?123h,
         but can also hold -1 for some other intervening character that we
         don't support.
      
      In a few places there's an integer that I haven't turned into a bool
      even though it really _can_ only take values 0 or 1 (and, as above,
      tried to make the call sites consistent in not calling those values
      true and false), on the grounds that I thought it would make it more
      confusing to imply that the 0 value was in some sense 'negative' or
      bad and the 1 positive or good:
       - the return value of plug_accepting uses the POSIXish convention of
         0=success and nonzero=error; I think if I made it bool then I'd
         also want to reverse its sense, and that's a job for a separate
         piece of work.
       - the 'screen' parameter to lineptr() in terminal.c, where 0 and 1
         represent the default and alternate screens. There's no obvious
         reason why one of those should be considered 'true' or 'positive'
         or 'success' - they're just indices - so I've left it as int.
      
      ssh_scp_recv had particularly confusing semantics for its previous int
      return value: its call sites used '<= 0' to check for error, but it
      never actually returned a negative number, just 0 or 1. Now the
      function and its call sites agree that it's a bool.
      
      In a couple of places I've renamed variables called 'ret', because I
      don't like that name any more - it's unclear whether it means the
      return value (in preparation) for the _containing_ function or the
      return value received from a subroutine call, and occasionally I've
      accidentally used the same variable for both and introduced a bug. So
      where one of those got in my way, I've renamed it to 'toret' or 'retd'
      (the latter short for 'returned') in line with my usual modern
      practice, but I haven't done a thorough job of finding all of them.
      
      Finally, one amusing side effect of doing this is that I've had to
      separate quite a few chained assignments. It used to be perfectly fine
      to write 'a = b = c = TRUE' when a,b,c were int and TRUE was just a
      the 'true' defined by stdbool.h, that idiom provokes a warning from
      gcc: 'suggest parentheses around assignment used as truth value'!
      3214563d
    • Simon Tatham's avatar
      Switch some Conf settings over to being bool. · 1378bb04
      Simon Tatham authored
      I think this is the full set of things that ought logically to be
      boolean.
      
      One annoyance is that quite a few radio-button controls in config.c
      address Conf fields that are now bool rather than int, which means
      that the shared handler function can't just access them all with
      conf_{get,set}_int. Rather than back out the rigorous separation of
      int and bool in conf.c itself, I've just added a similar alternative
      handler function for the bool-typed ones.
      1378bb04
    • 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
  13. Oct 21, 2018
    • Simon Tatham's avatar
      Add an actual SSH server program. · 1d323d5c
      Simon Tatham authored
      This server is NOT SECURE! If anyone is reading this commit message,
      DO NOT DEPLOY IT IN A HOSTILE-FACING ENVIRONMENT! Its purpose is to
      speak the server end of everything PuTTY speaks on the client side, so
      that I can test that I haven't broken PuTTY when I reorganise its
      code, even things like RSA key exchange or chained auth methods which
      it's hard to find a server that speaks at all.
      
      (For this reason, it's declared with [UT] in the Recipe file, so that
      it falls into the same category as programs like testbn, which won't
      be installed by 'make install'.)
      
      Working title is 'Uppity', partly for 'Universal PuTTY Protocol
      Interaction Test Yoke', but mostly because it looks quite like the
      word 'PuTTY' with part of it reversed. (Apparently 'test yoke' is a
      very rarely used term meaning something not altogether unlike 'test
      harness', which is a bit of a stretch, but it'll do.)
      
      It doesn't actually _support_ everything I want yet. At the moment,
      it's a proof of concept only. But it has most of the machinery
      present, and the parts it's missing - such as chained auth methods -
      should be easy enough to add because I've built in the required
      flexibility, in the form of an AuthPolicy object which can request
      them if it wants to. However, the current AuthPolicy object is
      entirely trivial, and will let in any user with the password "weasel".
      
      (Another way in which this is not a production-ready server is that it
      also has no interaction with the OS's authentication system. In
      particular, it will not only let in any user with the same password,
      but it won't even change uid - it will open shells and forwardings
      under whatever user id you started it up as.)
      
      Currently, the program can only speak the SSH protocol on its standard
      I/O channels (using the new FdSocket facility), so if you want it to
      listen on a network port, you'll have to run it from some kind of
      separate listening program similar to inetd. For my own tests, I'm not
      even doing that: I'm just having PuTTY spawn it as a local proxy
      process, which also conveniently eliminates the risk of anyone hostile
      connecting to it.
      
      The bulk of the actual code reorganisation is already done by previous
      commits, so this change is _mostly_ just dropping in a new set of
      server-specific source files alongside the client-specific ones I
      created recently. The remaining changes in the shared SSH code are
      numerous, but all minor:
      
       - a few extra parameters to BPP and PPL constructors (e.g. 'are you
         in server mode?'), and pass both sets of SSH-1 protocol flags from
         the login to the connection layer
       - in server mode, unconditionally send our version string _before_
         waiting for the remote one
       - a new hook in the SSH-1 BPP to handle enabling compression in
         server mode, where the message exchange works the other way round
       - new code in the SSH-2 BPP to do _deferred_ compression the other
         way round (the non-deferred version is still nicely symmetric)
       - in the SSH-2 transport layer, some adjustments to do key derivation
         either way round (swapping round the identifying letters in the
         various hash preimages, and making sure to list the KEXINITs in the
         right order)
       - also in the SSH-2 transport layer, an if statement that controls
         whether we send SERVICE_REQUEST and wait for SERVICE_ACCEPT, or
         vice versa
       - new ConnectionLayer methods for opening outgoing channels for X and
         agent forwardings
       - new functions in portfwd.c to establish listening sockets suitable
         for remote-to-local port forwarding (i.e. not under the direction
         of a Conf the way it's done on the client side).
      1d323d5c
    • Simon Tatham's avatar
      Server prep: factor out portfwd_raw_new(). · 83439617
      Simon Tatham authored
      This new function contains the core setup for a PortForwarding
      structure, and should be reusable for any kind of forwarding that will
      simply be passing data between a local socket and an SSH channel
      without any tricky modifications. On the server side, X11 and agent
      forwarding both work exactly like this, so they will find this
      refactored function useful during setup.
      
      The contents of the function was originally part of pfl_accepting,
      which now does all that by calling the new function. pfl_accepting is
      not _quite_ doing a simple unmodified forwarding, because it might
      have to prefix it with a SOCKS exchange; in that situation it rewrites
      a few fields of the PortForwarding to some less generic values once
      portfwd_raw_new() has returned.
      83439617
    • Simon Tatham's avatar
      Server prep: reword messages to be client/server agnostic. · 21a7ce7a
      Simon Tatham authored
      Lots of user-facing messages that claim that the 'server' just did
      something or other unexpected will now need to be issued _by_ the
      server, when the client does the same unexpected thing. So I've
      reworded them all to talk about the 'remote side' instead of the
      'server', and the SSH-2 key setup messages talk about initialising
      inbound and outbound crypto primitives rather than client->server and
      server->client.
      21a7ce7a
    • Simon Tatham's avatar
      Server prep: parse a lot of new channel requests. · 9fe719f4
      Simon Tatham authored
      ssh2connection.c now knows how to unmarshal the message formats for
      all the channel requests we'll need to handle when we're the server
      and a client sends them. Each one is translated into a call to a new
      method in the Channel vtable, which is implemented by a trivial
      'always fail' routine in every channel type we know about so far.
      9fe719f4
    • Simon Tatham's avatar
      Allow channels not to close immediately after two EOFs. · d3a9142d
      Simon Tatham authored
      Some kinds of channel, even after they've sent EOF in both directions,
      still have something to do before they initiate the CLOSE mechanism
      and wind up the channel completely. For example, a session channel
      with a subprocess running inside it will want to be sure to send the
      "exit-status" or "exit-signal" notification, even if that happens
      after bidirectional EOF of the data channels.
      
      Previously, the SSH-2 connection layer had the standard policy that
      once EOF had been both sent and received, it would start the final
      close procedure. There's a method chan_want_close() by which a Channel
      could vary this policy in one direction, by indicating that it wanted
      the close procedure to commence after EOF was sent in only one
      direction. Its parameters are a pair of booleans saying whether EOF
      has been sent, and whether it's been received.
      
      Now chan_want_close can vary the policy in the other direction as
      well: if it returns FALSE even when _both_ parameters are true, the
      connection layer will honour that, and not send CHANNEL_CLOSE. If it
      does that, the Channel is responsible for indicating when it _does_
      want close later, by calling sshfwd_initiate_close.
      d3a9142d
    • 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
      Rename sshfwd_unclean_close to sshfwd_initiate_close. · 1bde6869
      Simon Tatham authored
      Turns out that initiation of a CHANNEL_CLOSE message before both sides
      have sent EOF is not only for _unclean_ closures or emergencies; it's
      actually a perfectly normal thing that some channel types want to do.
      (For example, a channel with a pty at the server end of it has no real
      concept of sending EOF independently in both directions: when the pty
      master sends EIO, the pty is no longer functioning, and you can no
      longer send to it any more than you can receive.)
      1bde6869
    • Simon Tatham's avatar
      Move channel-opening logic out into subroutines. · d1cd8b25
      Simon Tatham authored
      Each of the new subroutines corresponds to one of the channel types
      for which we know how to parse a CHANNEL_OPEN, and has a collection of
      parameters corresponding to the fields of that message structure.
      ssh2_connection_filter_queue now confines itself to parsing the
      message, calling one of those functions, and constructing an
      appropriate reply message if any.
      d1cd8b25
    • Simon Tatham's avatar
      Devolve channel-request handling to Channel vtable. · 2339efcd
      Simon Tatham authored
      Instead of the central code in ssh2_connection_filter_queue doing both
      the job of parsing the channel request and deciding whether it's
      acceptable, each Channel vtable now has a method for every channel
      request type we recognise.
      2339efcd
    • Simon Tatham's avatar
      Add some missing 'const' in pfl_listen. · dfb8d5da
      Simon Tatham authored
      dfb8d5da
  14. 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
  15. 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
  16. 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
  17. 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
    • Simon Tatham's avatar
      Replace enum+union of local channel types with a vtable. · 6a8b9d38
      Simon Tatham authored
      There's now an interface called 'Channel', which handles the local
      side of an SSH connection-layer channel, in terms of knowing where to
      send incoming channel data to, whether to close the channel, etc.
      
      Channel and the previous 'struct ssh_channel' mutually refer. The
      latter contains all the SSH-specific parts, and as much of the common
      logic as possible: in particular, Channel doesn't have to know
      anything about SSH packet formats, or which SSH protocol version is in
      use, or deal with all the fiddly stuff about window sizes - with the
      exception that x11fwd.c's implementation of it does have to be able to
      ask for a small fixed initial window size for the bodgy system that
      distinguishes upstream from downstream X forwardings.
      
      I've taken the opportunity to move the code implementing the detailed
      behaviour of agent forwarding out of ssh.c, now that all of it is on
      the far side of a uniform interface. (This also means that if I later
      implement agent forwarding directly to a Unix socket as an
      alternative, it'll be a matter of changing just the one call to
      agentf_new() that makes the Channel to plug into a forwarding.)
      6a8b9d38
  18. Sep 19, 2018
  19. Jun 02, 2018
    • Simon Tatham's avatar
      Rewrite SOCKS client code using BinarySource. · 4d8c0335
      Simon Tatham authored
      I've also replaced the entire SOCKS state machine whose states were
      barely-documented literal integers with one that uses an actual enum.
      I think the result is a great deal clearer.
      
      In the course of this rewrite I noticed that PuTTY's dynamic port
      forwarding had never got round to supporting the SOCKS5 IPv6 address
      format - though there was a FIXME comment saying it ought to. So now
      it does: if a SOCKS5 client provides a binary IPv6 address (which
      PuTTY's _own_ SOCKS5 client, in proxy.c, is quite capable of doing!),
      then that will be translated into the usual IPv6 hex literal
      representation to put in the "direct-tcpip" channel open request.
      4d8c0335
    • Simon Tatham's avatar
      Fix some missing void * and const in existing APIs. · 8d882756
      Simon Tatham authored
      Several changes here that should have been in commit 7babe66a but I
      missed them.
      8d882756
  20. May 27, 2018
    • Simon Tatham's avatar
      Modernise the Socket/Plug vtable system. · 5129c40b
      Simon Tatham authored
      Now I've got FROMFIELD, I can rework it so that structures providing
      an implementation of the Socket or Plug trait no longer have to have
      the vtable pointer as the very first thing in the structure. In
      particular, this means that the ProxySocket structure can now directly
      implement _both_ the Socket and Plug traits, which is always
      _logically_ how it's worked, but previously it had to be implemented
      via two separate structs linked to each other.
      5129c40b
  21. May 26, 2018
    • Simon Tatham's avatar
      Make lots of generic data parameters into 'void *'. · 7babe66a
      Simon Tatham authored
      This is a cleanup I started to notice a need for during the BinarySink
      work. It removes a lot of faffing about casting things to char * or
      unsigned char * so that some API will accept them, even though lots of
      such APIs really take a plain 'block of raw binary data' argument and
      don't care what C thinks the signedness of that data might be - they
      may well reinterpret it back and forth internally.
      
      So I've tried to arrange for all the function call APIs that ought to
      have a void * (or const void *) to have one, and those that need to do
      pointer arithmetic on the parameter internally can cast it back at the
      top of the function. That saves endless ad-hoc casts at the call
      sites.
      7babe66a
    • Simon Tatham's avatar
      Centralise TRUE and FALSE definitions into defs.h. · c9bad331
      Simon Tatham authored
      This removes a lot of pointless duplications of those constants.
      
      Of course, _ideally_, I should upgrade to C99 bool throughout the code
      base, replacing TRUE and FALSE with true and false and tagging
      variables explicitly as bool when that's what they semantically are.
      But that's a much bigger piece of work, and shouldn't block this
      trivial cleanup!
      c9bad331
Loading