1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-03-25 16:07:24 -05:00

489 lines
14 KiB
C
Raw Normal View History

/*
* Rlogin backend.
*/
#include <stdio.h>
#include <stdlib.h>
#include <limits.h>
#include <ctype.h>
#include "putty.h"
#define RLOGIN_MAX_BACKLOG 4096
typedef struct Rlogin Rlogin;
struct Rlogin {
Socket *s;
Convert a lot of 'int' variables to 'bool'. 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'!
2018-11-02 19:23:19 +00:00
bool closed_on_socket_error;
int bufsize;
bool socket_connected;
Convert a lot of 'int' variables to 'bool'. 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'!
2018-11-02 19:23:19 +00:00
bool firstbyte;
bool cansize;
int term_width, term_height;
New abstraction 'Seat', to pass to backends. 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.)
2018-10-11 19:58:42 +01:00
Seat *seat;
Refactor the LogContext type. 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.
2018-10-10 19:26:18 +01:00
LogContext *logctx;
Ldisc *ldisc;
Add 'description' methods for Backend and Plug. These will typically be implemented by objects that are both a Backend *and* a Plug, and the two methods will deliver the same results to any caller, regardless of which facet of the object is known to that caller. Their purpose is to deliver a user-oriented natural-language description of what network connection the object is handling, so that it can appear in diagnostic messages. The messages I specifically have in mind are going to appear in cases where proxies require interactive authentication: when PuTTY prompts interactively for a password, it will need to explain which *thing* it's asking for the password for, and these descriptions are what it will use to describe the thing in question. Each backend is allowed to compose these messages however it thinks best. In all cases at present, the description string is constructed by the new centralised default_description() function, which takes a host name and port number and combines them with the backend's display name. But the SSH backend does things a bit differently, because it uses the _logical_ host name (the one that goes with the SSH host key) rather than the physical destination of the network connection. That seems more appropriate when the question it's really helping the user to answer is "What host am I supposed to be entering the password for?" In this commit, no clients of the new methods are introduced. I have a draft implementation of actually using it for the purpose I describe above, but it needs polishing.
2021-10-24 09:18:12 +01:00
char *description;
Post-release destabilisation! Completely remove the struct type 'Config' in putty.h, which stores all PuTTY's settings and includes an arbitrary length limit on every single one of those settings which is stored in string form. In place of it is 'Conf', an opaque data type everywhere outside the new file conf.c, which stores a list of (key, value) pairs in which every key contains an integer identifying a configuration setting, and for some of those integers the key also contains extra parts (so that, for instance, CONF_environmt is a string-to-string mapping). Everywhere that a Config was previously used, a Conf is now; everywhere there was a Config structure copy, conf_copy() is called; every lookup, adjustment, load and save operation on a Config has been rewritten; and there's a mechanism for serialising a Conf into a binary blob and back for use with Duplicate Session. User-visible effects of this change _should_ be minimal, though I don't doubt I've introduced one or two bugs here and there which will eventually be found. The _intended_ visible effects of this change are that all arbitrary limits on configuration strings and lists (e.g. limit on number of port forwardings) should now disappear; that list boxes in the configuration will now be displayed in a sorted order rather than the arbitrary order in which they were added to the list (since the underlying data structure is now a sorted tree234 rather than an ad-hoc comma-separated string); and one more specific change, which is that local and dynamic port forwardings on the same port number are now mutually exclusive in the configuration (putting 'D' in the key rather than the value was a mistake in the first place). One other reorganisation as a result of this is that I've moved all the dialog.c standard handlers (dlg_stdeditbox_handler and friends) out into config.c, because I can't really justify calling them generic any more. When they took a pointer to an arbitrary structure type and the offset of a field within that structure, they were independent of whether that structure was a Config or something completely different, but now they really do expect to talk to a Conf, which can _only_ be used for PuTTY configuration, so I've renamed them all things like conf_editbox_handler and moved them out of the nominally independent dialog-box management module into the PuTTY-specific config.c. [originally from svn r9214]
2011-07-14 18:52:21 +00:00
Conf *conf;
/* In case we need to read a username from the terminal before starting */
prompts_t *prompt;
Plug plug;
Backend backend;
Interactor interactor;
};
static void rlogin_startup(Rlogin *rlogin, int prompt_result,
const char *ruser);
Complete rework of terminal userpass input system. The system for handling seat_get_userpass_input has always been structured differently between GUI PuTTY and CLI tools like Plink. In the CLI tools, password input is read directly from the OS terminal/console device by console_get_userpass_input; this means that you need to ensure the same terminal input data _hasn't_ already been consumed by the main event loop and sent on to the backend. This is achieved by the backend_sendok() method, which tells the event loop when the backend has finished issuing password prompts, and hence, when it's safe to start passing standard input to backend_send(). But in the GUI tools, input generated by the terminal window has always been sent straight to backend_send(), regardless of whether backend_sendok() says it wants it. So the terminal-based implementation of username and password prompts has to work by consuming input data that had _already_ been passed to the backend - hence, any backend that needs to do that must keep its input on a bufchain, and pass that bufchain to seat_get_userpass_input. It's awkward that these two totally different systems coexist in the first place. And now that SSH proxying needs to present interactive prompts of its own, it's clear which one should win: the CLI style is the Right Thing. So this change reworks the GUI side of the mechanism to be more similar: terminal data now goes into a queue in the Ldisc, and is not sent on to the backend until the backend says it's ready for it via backend_sendok(). So terminal-based userpass prompts can now consume data directly from that queue during the connection setup stage. As a result, the 'bufchain *' parameter has vanished from all the userpass_input functions (both the official implementations of the Seat trait method, and term_get_userpass_input() to which some of those implementations delegate). The only function that actually used that bufchain, namely term_get_userpass_input(), now instead reads from the ldisc's input queue via a couple of new Ldisc functions. (Not _trivial_ functions, since input buffered by Ldisc can be a mixture of raw bytes and session specials like SS_EOL! The input queue inside Ldisc is a bufchain containing a fiddly binary encoding that can represent an arbitrary interleaving of those things.) This greatly simplifies the calls to seat_get_userpass_input in backends, which now don't have to mess about with passing their own user_input bufchain around, or toggling their want_user_input flag back and forth to request data to put on to that bufchain. But the flip side is that now there has to be some _other_ method for notifying the terminal when there's more input to be consumed during an interactive prompt, and for notifying the backend when prompt input has finished so that it can proceed to the next stage of the protocol. This is done by a pair of extra callbacks: when more data is put on to Ldisc's input queue, it triggers a call to term_get_userpass_input, and when term_get_userpass_input finishes, it calls a callback function provided in the prompts_t. Therefore, any use of a prompts_t which *might* be asynchronous must fill in the latter callback when setting up the prompts_t. In SSH, the callback is centralised into a common PPL helper function, which reinvokes the same PPL's process_queue coroutine; in rlogin we have to set it up ourselves. I'm sorry for this large and sprawling patch: I tried fairly hard to break it up into individually comprehensible sub-patches, but I just couldn't tease out any part of it that would stand sensibly alone.
2021-09-14 11:57:21 +01:00
static void rlogin_try_username_prompt(void *ctx);
static void c_write(Rlogin *rlogin, const void *buf, size_t len)
{
size_t backlog = seat_stdout(rlogin->seat, buf, len);
sk_set_frozen(rlogin->s, backlog > RLOGIN_MAX_BACKLOG);
}
static void rlogin_log(Plug *plug, PlugLogType type, SockAddr *addr, int port,
const char *error_msg, int error_code)
{
Rlogin *rlogin = container_of(plug, Rlogin, plug);
New abstraction 'Seat', to pass to backends. 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.)
2018-10-11 19:58:42 +01:00
backend_socket_log(rlogin->seat, rlogin->logctx, type, addr, port,
error_msg, error_code,
rlogin->conf, rlogin->socket_connected);
Allow new_connection to take an optional Seat. (NFC) This is working towards allowing the subsidiary SSH connection in an SshProxy to share the main user-facing Seat, so as to be able to pass through interactive prompts. This is more difficult than the similar change with LogPolicy, because Seats are stateful. In particular, the trust-sigil status will need to be controlled by the SshProxy until it's ready to pass over control to the main SSH (or whatever) connection. To make this work, I've introduced a thing called a TempSeat, which is (yet) another Seat implementation. When a backend hands its Seat to new_connection(), it does it in a way that allows new_connection() to borrow it completely, and replace it in the main backend structure with a TempSeat, which acts as a temporary placeholder. If the main backend tries to do things like changing trust status or sending output, the TempSeat will buffer them; later on, when the connection is established, TempSeat will replay the changes into the real Seat. So, in each backend, I've made the following changes: - pass &foo->seat to new_connection, which may overwrite it with a TempSeat. - if it has done so (which we can tell via the is_tempseat() query function), then we have to free the TempSeat and reinstate our main Seat. The signal that we can do so is the PLUGLOG_CONNECT_SUCCESS notification, which indicates that SshProxy has finished all its connection setup work. - we also have to remember to free the TempSeat if our backend is disposed of without that having happened (e.g. because the connection _doesn't_ succeed). - in backends which have no local auth phase to worry about, ensure we don't call seat_set_trust_status on the main Seat _before_ it gets potentially replaced with a TempSeat. Moved some calls of seat_set_trust_status to just after new_connection(), so that now the initial trust status setup will go into the TempSeat (if appropriate) and be buffered until that seat is relinquished. In all other uses of new_connection, where we don't have a Seat available at all, we just pass NULL. This is NFC, because neither new_connection() nor any of its delegates will _actually_ do this replacement yet. We're just setting up the framework to enable it to do so in the next commit.
2021-09-13 17:17:20 +01:00
if (type == PLUGLOG_CONNECT_SUCCESS) {
rlogin->socket_connected = true;
char *ruser = get_remote_username(rlogin->conf);
if (ruser) {
/*
* If we already know the remote username, call
* rlogin_startup, which will send the initial protocol
* greeting including local username, remote username,
* terminal type and terminal speed.
*/
/* Next terminal output will come from server */
seat_set_trust_status(rlogin->seat, false);
rlogin_startup(rlogin, 1, ruser);
sfree(ruser);
} else {
/*
* Otherwise, set up a prompts_t asking for the local
* username. If it completes synchronously, call
* rlogin_startup as above; otherwise, wait until it does.
*/
rlogin->prompt = new_prompts();
rlogin->prompt->to_server = true;
rlogin->prompt->from_server = false;
rlogin->prompt->name = dupstr("Rlogin login name");
Complete rework of terminal userpass input system. The system for handling seat_get_userpass_input has always been structured differently between GUI PuTTY and CLI tools like Plink. In the CLI tools, password input is read directly from the OS terminal/console device by console_get_userpass_input; this means that you need to ensure the same terminal input data _hasn't_ already been consumed by the main event loop and sent on to the backend. This is achieved by the backend_sendok() method, which tells the event loop when the backend has finished issuing password prompts, and hence, when it's safe to start passing standard input to backend_send(). But in the GUI tools, input generated by the terminal window has always been sent straight to backend_send(), regardless of whether backend_sendok() says it wants it. So the terminal-based implementation of username and password prompts has to work by consuming input data that had _already_ been passed to the backend - hence, any backend that needs to do that must keep its input on a bufchain, and pass that bufchain to seat_get_userpass_input. It's awkward that these two totally different systems coexist in the first place. And now that SSH proxying needs to present interactive prompts of its own, it's clear which one should win: the CLI style is the Right Thing. So this change reworks the GUI side of the mechanism to be more similar: terminal data now goes into a queue in the Ldisc, and is not sent on to the backend until the backend says it's ready for it via backend_sendok(). So terminal-based userpass prompts can now consume data directly from that queue during the connection setup stage. As a result, the 'bufchain *' parameter has vanished from all the userpass_input functions (both the official implementations of the Seat trait method, and term_get_userpass_input() to which some of those implementations delegate). The only function that actually used that bufchain, namely term_get_userpass_input(), now instead reads from the ldisc's input queue via a couple of new Ldisc functions. (Not _trivial_ functions, since input buffered by Ldisc can be a mixture of raw bytes and session specials like SS_EOL! The input queue inside Ldisc is a bufchain containing a fiddly binary encoding that can represent an arbitrary interleaving of those things.) This greatly simplifies the calls to seat_get_userpass_input in backends, which now don't have to mess about with passing their own user_input bufchain around, or toggling their want_user_input flag back and forth to request data to put on to that bufchain. But the flip side is that now there has to be some _other_ method for notifying the terminal when there's more input to be consumed during an interactive prompt, and for notifying the backend when prompt input has finished so that it can proceed to the next stage of the protocol. This is done by a pair of extra callbacks: when more data is put on to Ldisc's input queue, it triggers a call to term_get_userpass_input, and when term_get_userpass_input finishes, it calls a callback function provided in the prompts_t. Therefore, any use of a prompts_t which *might* be asynchronous must fill in the latter callback when setting up the prompts_t. In SSH, the callback is centralised into a common PPL helper function, which reinvokes the same PPL's process_queue coroutine; in rlogin we have to set it up ourselves. I'm sorry for this large and sprawling patch: I tried fairly hard to break it up into individually comprehensible sub-patches, but I just couldn't tease out any part of it that would stand sensibly alone.
2021-09-14 11:57:21 +01:00
rlogin->prompt->callback = rlogin_try_username_prompt;
rlogin->prompt->callback_ctx = rlogin;
add_prompt(rlogin->prompt, dupstr("rlogin username: "), true);
Complete rework of terminal userpass input system. The system for handling seat_get_userpass_input has always been structured differently between GUI PuTTY and CLI tools like Plink. In the CLI tools, password input is read directly from the OS terminal/console device by console_get_userpass_input; this means that you need to ensure the same terminal input data _hasn't_ already been consumed by the main event loop and sent on to the backend. This is achieved by the backend_sendok() method, which tells the event loop when the backend has finished issuing password prompts, and hence, when it's safe to start passing standard input to backend_send(). But in the GUI tools, input generated by the terminal window has always been sent straight to backend_send(), regardless of whether backend_sendok() says it wants it. So the terminal-based implementation of username and password prompts has to work by consuming input data that had _already_ been passed to the backend - hence, any backend that needs to do that must keep its input on a bufchain, and pass that bufchain to seat_get_userpass_input. It's awkward that these two totally different systems coexist in the first place. And now that SSH proxying needs to present interactive prompts of its own, it's clear which one should win: the CLI style is the Right Thing. So this change reworks the GUI side of the mechanism to be more similar: terminal data now goes into a queue in the Ldisc, and is not sent on to the backend until the backend says it's ready for it via backend_sendok(). So terminal-based userpass prompts can now consume data directly from that queue during the connection setup stage. As a result, the 'bufchain *' parameter has vanished from all the userpass_input functions (both the official implementations of the Seat trait method, and term_get_userpass_input() to which some of those implementations delegate). The only function that actually used that bufchain, namely term_get_userpass_input(), now instead reads from the ldisc's input queue via a couple of new Ldisc functions. (Not _trivial_ functions, since input buffered by Ldisc can be a mixture of raw bytes and session specials like SS_EOL! The input queue inside Ldisc is a bufchain containing a fiddly binary encoding that can represent an arbitrary interleaving of those things.) This greatly simplifies the calls to seat_get_userpass_input in backends, which now don't have to mess about with passing their own user_input bufchain around, or toggling their want_user_input flag back and forth to request data to put on to that bufchain. But the flip side is that now there has to be some _other_ method for notifying the terminal when there's more input to be consumed during an interactive prompt, and for notifying the backend when prompt input has finished so that it can proceed to the next stage of the protocol. This is done by a pair of extra callbacks: when more data is put on to Ldisc's input queue, it triggers a call to term_get_userpass_input, and when term_get_userpass_input finishes, it calls a callback function provided in the prompts_t. Therefore, any use of a prompts_t which *might* be asynchronous must fill in the latter callback when setting up the prompts_t. In SSH, the callback is centralised into a common PPL helper function, which reinvokes the same PPL's process_queue coroutine; in rlogin we have to set it up ourselves. I'm sorry for this large and sprawling patch: I tried fairly hard to break it up into individually comprehensible sub-patches, but I just couldn't tease out any part of it that would stand sensibly alone.
2021-09-14 11:57:21 +01:00
rlogin_try_username_prompt(rlogin);
}
Allow new_connection to take an optional Seat. (NFC) This is working towards allowing the subsidiary SSH connection in an SshProxy to share the main user-facing Seat, so as to be able to pass through interactive prompts. This is more difficult than the similar change with LogPolicy, because Seats are stateful. In particular, the trust-sigil status will need to be controlled by the SshProxy until it's ready to pass over control to the main SSH (or whatever) connection. To make this work, I've introduced a thing called a TempSeat, which is (yet) another Seat implementation. When a backend hands its Seat to new_connection(), it does it in a way that allows new_connection() to borrow it completely, and replace it in the main backend structure with a TempSeat, which acts as a temporary placeholder. If the main backend tries to do things like changing trust status or sending output, the TempSeat will buffer them; later on, when the connection is established, TempSeat will replay the changes into the real Seat. So, in each backend, I've made the following changes: - pass &foo->seat to new_connection, which may overwrite it with a TempSeat. - if it has done so (which we can tell via the is_tempseat() query function), then we have to free the TempSeat and reinstate our main Seat. The signal that we can do so is the PLUGLOG_CONNECT_SUCCESS notification, which indicates that SshProxy has finished all its connection setup work. - we also have to remember to free the TempSeat if our backend is disposed of without that having happened (e.g. because the connection _doesn't_ succeed). - in backends which have no local auth phase to worry about, ensure we don't call seat_set_trust_status on the main Seat _before_ it gets potentially replaced with a TempSeat. Moved some calls of seat_set_trust_status to just after new_connection(), so that now the initial trust status setup will go into the TempSeat (if appropriate) and be buffered until that seat is relinquished. In all other uses of new_connection, where we don't have a Seat available at all, we just pass NULL. This is NFC, because neither new_connection() nor any of its delegates will _actually_ do this replacement yet. We're just setting up the framework to enable it to do so in the next commit.
2021-09-13 17:17:20 +01:00
}
}
static void rlogin_closing(Plug *plug, PlugCloseType type,
const char *error_msg)
{
Rlogin *rlogin = container_of(plug, Rlogin, plug);
/*
* We don't implement independent EOF in each direction for Telnet
* connections; as soon as we get word that the remote side has
* sent us EOF, we wind up the whole connection.
*/
if (rlogin->s) {
sk_close(rlogin->s);
rlogin->s = NULL;
if (error_msg)
rlogin->closed_on_socket_error = true;
seat_notify_remote_exit(rlogin->seat);
seat_notify_remote_disconnect(rlogin->seat);
}
if (type != PLUGCLOSE_NORMAL) {
/* A socket error has occurred. */
Refactor the LogContext type. 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.
2018-10-10 19:26:18 +01:00
logevent(rlogin->logctx, error_msg);
if (type != PLUGCLOSE_USER_ABORT)
seat_connection_fatal(rlogin->seat, "%s", error_msg);
}
/* Otherwise, the remote side closed the connection normally. */
}
static void rlogin_receive(
Plug *plug, int urgent, const char *data, size_t len)
{
Rlogin *rlogin = container_of(plug, Rlogin, plug);
if (len == 0)
return;
if (urgent == 2) {
char c;
c = *data++;
len--;
if (c == '\x80') {
rlogin->cansize = true;
backend_size(&rlogin->backend,
rlogin->term_width, rlogin->term_height);
}
/*
* We should flush everything (aka Telnet SYNCH) if we see
* 0x02, and we should turn off and on _local_ flow control
* on 0x10 and 0x20 respectively. I'm not convinced it's
* worth it...
*/
} else {
/*
* Main rlogin protocol. This is really simple: the first
* byte is expected to be NULL and is ignored, and the rest
* is printed.
*/
if (rlogin->firstbyte) {
if (data[0] == '\0') {
data++;
len--;
}
rlogin->firstbyte = false;
}
if (len > 0)
c_write(rlogin, data, len);
}
}
static void rlogin_sent(Plug *plug, size_t bufsize)
{
Rlogin *rlogin = container_of(plug, Rlogin, plug);
rlogin->bufsize = bufsize;
New Seat callback, seat_sent(). This is used to notify the Seat that some data has been cleared from the backend's outgoing data buffer. In other words, it notifies the Seat that it might be worth calling backend_sendbuffer() again. We've never needed this before, because until now, Seats have always been the 'main program' part of the application, meaning they were also in control of the event loop. So they've been able to call backend_sendbuffer() proactively, every time they go round the event loop, instead of having to wait for a callback. But now, the SSH proxy is the first example of a Seat without privileged access to the event loop, so it has no way to find out that the backend's sendbuffer has got smaller. And without that, it can't pass that notification on to plug_sent, to unblock in turn whatever the proxied connection might have been waiting to send. In fact, before this commit, sshproxy.c never called plug_sent at all. As a result, large data uploads over an SSH jump host would hang forever as soon as the outgoing buffer filled up for the first time: the main backend (to which sshproxy.c was acting as a Socket) would carefully stop filling up the buffer, and then never receive the call to plug_sent that would cause it to start again. The new callback is ignored everywhere except in sshproxy.c. It might be a good idea to remove backend_sendbuffer() entirely and convert all previous uses of it into non-empty implementations of this callback, so that we've only got one system; but for the moment, I haven't done that.
2021-06-27 13:52:48 +01:00
seat_sent(rlogin->seat, rlogin->bufsize);
}
static void rlogin_startup(Rlogin *rlogin, int prompt_result,
const char *ruser)
{
char z = 0;
char *p;
Post-release destabilisation! Completely remove the struct type 'Config' in putty.h, which stores all PuTTY's settings and includes an arbitrary length limit on every single one of those settings which is stored in string form. In place of it is 'Conf', an opaque data type everywhere outside the new file conf.c, which stores a list of (key, value) pairs in which every key contains an integer identifying a configuration setting, and for some of those integers the key also contains extra parts (so that, for instance, CONF_environmt is a string-to-string mapping). Everywhere that a Config was previously used, a Conf is now; everywhere there was a Config structure copy, conf_copy() is called; every lookup, adjustment, load and save operation on a Config has been rewritten; and there's a mechanism for serialising a Conf into a binary blob and back for use with Duplicate Session. User-visible effects of this change _should_ be minimal, though I don't doubt I've introduced one or two bugs here and there which will eventually be found. The _intended_ visible effects of this change are that all arbitrary limits on configuration strings and lists (e.g. limit on number of port forwardings) should now disappear; that list boxes in the configuration will now be displayed in a sorted order rather than the arbitrary order in which they were added to the list (since the underlying data structure is now a sorted tree234 rather than an ad-hoc comma-separated string); and one more specific change, which is that local and dynamic port forwardings on the same port number are now mutually exclusive in the configuration (putting 'D' in the key rather than the value was a mistake in the first place). One other reorganisation as a result of this is that I've moved all the dialog.c standard handlers (dlg_stdeditbox_handler and friends) out into config.c, because I can't really justify calling them generic any more. When they took a pointer to an arbitrary structure type and the offset of a field within that structure, they were independent of whether that structure was a Config or something completely different, but now they really do expect to talk to a Conf, which can _only_ be used for PuTTY configuration, so I've renamed them all things like conf_editbox_handler and moved them out of the nominally independent dialog-box management module into the PuTTY-specific config.c. [originally from svn r9214]
2011-07-14 18:52:21 +00:00
if (prompt_result == 0) {
/* User aborted at the username prompt. */
sk_close(rlogin->s);
rlogin->s = NULL;
seat_notify_remote_exit(rlogin->seat);
} else {
sk_write(rlogin->s, &z, 1);
p = conf_get_str(rlogin->conf, CONF_localusername);
sk_write(rlogin->s, p, strlen(p));
sk_write(rlogin->s, &z, 1);
sk_write(rlogin->s, ruser, strlen(ruser));
sk_write(rlogin->s, &z, 1);
p = conf_get_str(rlogin->conf, CONF_termtype);
sk_write(rlogin->s, p, strlen(p));
sk_write(rlogin->s, "/", 1);
p = conf_get_str(rlogin->conf, CONF_termspeed);
sk_write(rlogin->s, p, strspn(p, "0123456789"));
rlogin->bufsize = sk_write(rlogin->s, &z, 1);
}
rlogin->prompt = NULL;
if (rlogin->ldisc)
ldisc_check_sendok(rlogin->ldisc);
}
static const PlugVtable Rlogin_plugvt = {
.log = rlogin_log,
.closing = rlogin_closing,
.receive = rlogin_receive,
.sent = rlogin_sent,
};
static char *rlogin_description(Interactor *itr)
{
Rlogin *rlogin = container_of(itr, Rlogin, interactor);
return dupstr(rlogin->description);
}
static LogPolicy *rlogin_logpolicy(Interactor *itr)
{
Rlogin *rlogin = container_of(itr, Rlogin, interactor);
return log_get_policy(rlogin->logctx);
}
static Seat *rlogin_get_seat(Interactor *itr)
{
Rlogin *rlogin = container_of(itr, Rlogin, interactor);
return rlogin->seat;
}
static void rlogin_set_seat(Interactor *itr, Seat *seat)
{
Rlogin *rlogin = container_of(itr, Rlogin, interactor);
rlogin->seat = seat;
}
static const InteractorVtable Rlogin_interactorvt = {
.description = rlogin_description,
.logpolicy = rlogin_logpolicy,
.get_seat = rlogin_get_seat,
.set_seat = rlogin_set_seat,
};
/*
* Called to set up the rlogin connection.
*
* Returns an error message, or NULL on success.
*
* Also places the canonical host name into `realhost'. It must be
* freed by the caller.
*/
static char *rlogin_init(const BackendVtable *vt, Seat *seat,
Backend **backend_handle, LogContext *logctx,
Conf *conf, const char *host, int port,
char **realhost, bool nodelay, bool keepalive)
{
SockAddr *addr;
const char *err;
Rlogin *rlogin;
Post-release destabilisation! Completely remove the struct type 'Config' in putty.h, which stores all PuTTY's settings and includes an arbitrary length limit on every single one of those settings which is stored in string form. In place of it is 'Conf', an opaque data type everywhere outside the new file conf.c, which stores a list of (key, value) pairs in which every key contains an integer identifying a configuration setting, and for some of those integers the key also contains extra parts (so that, for instance, CONF_environmt is a string-to-string mapping). Everywhere that a Config was previously used, a Conf is now; everywhere there was a Config structure copy, conf_copy() is called; every lookup, adjustment, load and save operation on a Config has been rewritten; and there's a mechanism for serialising a Conf into a binary blob and back for use with Duplicate Session. User-visible effects of this change _should_ be minimal, though I don't doubt I've introduced one or two bugs here and there which will eventually be found. The _intended_ visible effects of this change are that all arbitrary limits on configuration strings and lists (e.g. limit on number of port forwardings) should now disappear; that list boxes in the configuration will now be displayed in a sorted order rather than the arbitrary order in which they were added to the list (since the underlying data structure is now a sorted tree234 rather than an ad-hoc comma-separated string); and one more specific change, which is that local and dynamic port forwardings on the same port number are now mutually exclusive in the configuration (putting 'D' in the key rather than the value was a mistake in the first place). One other reorganisation as a result of this is that I've moved all the dialog.c standard handlers (dlg_stdeditbox_handler and friends) out into config.c, because I can't really justify calling them generic any more. When they took a pointer to an arbitrary structure type and the offset of a field within that structure, they were independent of whether that structure was a Config or something completely different, but now they really do expect to talk to a Conf, which can _only_ be used for PuTTY configuration, so I've renamed them all things like conf_editbox_handler and moved them out of the nominally independent dialog-box management module into the PuTTY-specific config.c. [originally from svn r9214]
2011-07-14 18:52:21 +00:00
int addressfamily;
char *loghost;
rlogin = snew(Rlogin);
memset(rlogin, 0, sizeof(Rlogin));
rlogin->plug.vt = &Rlogin_plugvt;
rlogin->backend.vt = vt;
rlogin->interactor.vt = &Rlogin_interactorvt;
rlogin->backend.interactor = &rlogin->interactor;
rlogin->s = NULL;
rlogin->closed_on_socket_error = false;
New abstraction 'Seat', to pass to backends. 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.)
2018-10-11 19:58:42 +01:00
rlogin->seat = seat;
Refactor the LogContext type. 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.
2018-10-10 19:26:18 +01:00
rlogin->logctx = logctx;
Post-release destabilisation! Completely remove the struct type 'Config' in putty.h, which stores all PuTTY's settings and includes an arbitrary length limit on every single one of those settings which is stored in string form. In place of it is 'Conf', an opaque data type everywhere outside the new file conf.c, which stores a list of (key, value) pairs in which every key contains an integer identifying a configuration setting, and for some of those integers the key also contains extra parts (so that, for instance, CONF_environmt is a string-to-string mapping). Everywhere that a Config was previously used, a Conf is now; everywhere there was a Config structure copy, conf_copy() is called; every lookup, adjustment, load and save operation on a Config has been rewritten; and there's a mechanism for serialising a Conf into a binary blob and back for use with Duplicate Session. User-visible effects of this change _should_ be minimal, though I don't doubt I've introduced one or two bugs here and there which will eventually be found. The _intended_ visible effects of this change are that all arbitrary limits on configuration strings and lists (e.g. limit on number of port forwardings) should now disappear; that list boxes in the configuration will now be displayed in a sorted order rather than the arbitrary order in which they were added to the list (since the underlying data structure is now a sorted tree234 rather than an ad-hoc comma-separated string); and one more specific change, which is that local and dynamic port forwardings on the same port number are now mutually exclusive in the configuration (putting 'D' in the key rather than the value was a mistake in the first place). One other reorganisation as a result of this is that I've moved all the dialog.c standard handlers (dlg_stdeditbox_handler and friends) out into config.c, because I can't really justify calling them generic any more. When they took a pointer to an arbitrary structure type and the offset of a field within that structure, they were independent of whether that structure was a Config or something completely different, but now they really do expect to talk to a Conf, which can _only_ be used for PuTTY configuration, so I've renamed them all things like conf_editbox_handler and moved them out of the nominally independent dialog-box management module into the PuTTY-specific config.c. [originally from svn r9214]
2011-07-14 18:52:21 +00:00
rlogin->term_width = conf_get_int(conf, CONF_width);
rlogin->term_height = conf_get_int(conf, CONF_height);
rlogin->socket_connected = false;
Convert a lot of 'int' variables to 'bool'. 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'!
2018-11-02 19:23:19 +00:00
rlogin->firstbyte = true;
rlogin->cansize = false;
rlogin->prompt = NULL;
Post-release destabilisation! Completely remove the struct type 'Config' in putty.h, which stores all PuTTY's settings and includes an arbitrary length limit on every single one of those settings which is stored in string form. In place of it is 'Conf', an opaque data type everywhere outside the new file conf.c, which stores a list of (key, value) pairs in which every key contains an integer identifying a configuration setting, and for some of those integers the key also contains extra parts (so that, for instance, CONF_environmt is a string-to-string mapping). Everywhere that a Config was previously used, a Conf is now; everywhere there was a Config structure copy, conf_copy() is called; every lookup, adjustment, load and save operation on a Config has been rewritten; and there's a mechanism for serialising a Conf into a binary blob and back for use with Duplicate Session. User-visible effects of this change _should_ be minimal, though I don't doubt I've introduced one or two bugs here and there which will eventually be found. The _intended_ visible effects of this change are that all arbitrary limits on configuration strings and lists (e.g. limit on number of port forwardings) should now disappear; that list boxes in the configuration will now be displayed in a sorted order rather than the arbitrary order in which they were added to the list (since the underlying data structure is now a sorted tree234 rather than an ad-hoc comma-separated string); and one more specific change, which is that local and dynamic port forwardings on the same port number are now mutually exclusive in the configuration (putting 'D' in the key rather than the value was a mistake in the first place). One other reorganisation as a result of this is that I've moved all the dialog.c standard handlers (dlg_stdeditbox_handler and friends) out into config.c, because I can't really justify calling them generic any more. When they took a pointer to an arbitrary structure type and the offset of a field within that structure, they were independent of whether that structure was a Config or something completely different, but now they really do expect to talk to a Conf, which can _only_ be used for PuTTY configuration, so I've renamed them all things like conf_editbox_handler and moved them out of the nominally independent dialog-box management module into the PuTTY-specific config.c. [originally from svn r9214]
2011-07-14 18:52:21 +00:00
rlogin->conf = conf_copy(conf);
Add 'description' methods for Backend and Plug. These will typically be implemented by objects that are both a Backend *and* a Plug, and the two methods will deliver the same results to any caller, regardless of which facet of the object is known to that caller. Their purpose is to deliver a user-oriented natural-language description of what network connection the object is handling, so that it can appear in diagnostic messages. The messages I specifically have in mind are going to appear in cases where proxies require interactive authentication: when PuTTY prompts interactively for a password, it will need to explain which *thing* it's asking for the password for, and these descriptions are what it will use to describe the thing in question. Each backend is allowed to compose these messages however it thinks best. In all cases at present, the description string is constructed by the new centralised default_description() function, which takes a host name and port number and combines them with the backend's display name. But the SSH backend does things a bit differently, because it uses the _logical_ host name (the one that goes with the SSH host key) rather than the physical destination of the network connection. That seems more appropriate when the question it's really helping the user to answer is "What host am I supposed to be entering the password for?" In this commit, no clients of the new methods are introduced. I have a draft implementation of actually using it for the purpose I describe above, but it needs polishing.
2021-10-24 09:18:12 +01:00
rlogin->description = default_description(vt, host, port);
*backend_handle = &rlogin->backend;
Post-release destabilisation! Completely remove the struct type 'Config' in putty.h, which stores all PuTTY's settings and includes an arbitrary length limit on every single one of those settings which is stored in string form. In place of it is 'Conf', an opaque data type everywhere outside the new file conf.c, which stores a list of (key, value) pairs in which every key contains an integer identifying a configuration setting, and for some of those integers the key also contains extra parts (so that, for instance, CONF_environmt is a string-to-string mapping). Everywhere that a Config was previously used, a Conf is now; everywhere there was a Config structure copy, conf_copy() is called; every lookup, adjustment, load and save operation on a Config has been rewritten; and there's a mechanism for serialising a Conf into a binary blob and back for use with Duplicate Session. User-visible effects of this change _should_ be minimal, though I don't doubt I've introduced one or two bugs here and there which will eventually be found. The _intended_ visible effects of this change are that all arbitrary limits on configuration strings and lists (e.g. limit on number of port forwardings) should now disappear; that list boxes in the configuration will now be displayed in a sorted order rather than the arbitrary order in which they were added to the list (since the underlying data structure is now a sorted tree234 rather than an ad-hoc comma-separated string); and one more specific change, which is that local and dynamic port forwardings on the same port number are now mutually exclusive in the configuration (putting 'D' in the key rather than the value was a mistake in the first place). One other reorganisation as a result of this is that I've moved all the dialog.c standard handlers (dlg_stdeditbox_handler and friends) out into config.c, because I can't really justify calling them generic any more. When they took a pointer to an arbitrary structure type and the offset of a field within that structure, they were independent of whether that structure was a Config or something completely different, but now they really do expect to talk to a Conf, which can _only_ be used for PuTTY configuration, so I've renamed them all things like conf_editbox_handler and moved them out of the nominally independent dialog-box management module into the PuTTY-specific config.c. [originally from svn r9214]
2011-07-14 18:52:21 +00:00
addressfamily = conf_get_int(conf, CONF_addressfamily);
/*
* Try to find host.
*/
addr = name_lookup(host, port, realhost, conf, addressfamily,
Refactor the LogContext type. 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.
2018-10-10 19:26:18 +01:00
rlogin->logctx, "rlogin connection");
if ((err = sk_addr_error(addr)) != NULL) {
sk_addr_free(addr);
return dupstr(err);
}
if (port < 0)
port = 513; /* default rlogin port */
/*
* Open socket.
*/
Convert a lot of 'int' variables to 'bool'. 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'!
2018-11-02 19:23:19 +00:00
rlogin->s = new_connection(addr, *realhost, port, true, false,
nodelay, keepalive, &rlogin->plug, conf,
&rlogin->interactor);
if ((err = sk_socket_error(rlogin->s)) != NULL)
return dupstr(err);
Post-release destabilisation! Completely remove the struct type 'Config' in putty.h, which stores all PuTTY's settings and includes an arbitrary length limit on every single one of those settings which is stored in string form. In place of it is 'Conf', an opaque data type everywhere outside the new file conf.c, which stores a list of (key, value) pairs in which every key contains an integer identifying a configuration setting, and for some of those integers the key also contains extra parts (so that, for instance, CONF_environmt is a string-to-string mapping). Everywhere that a Config was previously used, a Conf is now; everywhere there was a Config structure copy, conf_copy() is called; every lookup, adjustment, load and save operation on a Config has been rewritten; and there's a mechanism for serialising a Conf into a binary blob and back for use with Duplicate Session. User-visible effects of this change _should_ be minimal, though I don't doubt I've introduced one or two bugs here and there which will eventually be found. The _intended_ visible effects of this change are that all arbitrary limits on configuration strings and lists (e.g. limit on number of port forwardings) should now disappear; that list boxes in the configuration will now be displayed in a sorted order rather than the arbitrary order in which they were added to the list (since the underlying data structure is now a sorted tree234 rather than an ad-hoc comma-separated string); and one more specific change, which is that local and dynamic port forwardings on the same port number are now mutually exclusive in the configuration (putting 'D' in the key rather than the value was a mistake in the first place). One other reorganisation as a result of this is that I've moved all the dialog.c standard handlers (dlg_stdeditbox_handler and friends) out into config.c, because I can't really justify calling them generic any more. When they took a pointer to an arbitrary structure type and the offset of a field within that structure, they were independent of whether that structure was a Config or something completely different, but now they really do expect to talk to a Conf, which can _only_ be used for PuTTY configuration, so I've renamed them all things like conf_editbox_handler and moved them out of the nominally independent dialog-box management module into the PuTTY-specific config.c. [originally from svn r9214]
2011-07-14 18:52:21 +00:00
loghost = conf_get_str(conf, CONF_loghost);
if (*loghost) {
char *colon;
sfree(*realhost);
*realhost = dupstr(loghost);
colon = host_strrchr(*realhost, ':');
if (colon)
*colon++ = '\0';
}
return NULL;
}
static void rlogin_free(Backend *be)
{
Rlogin *rlogin = container_of(be, Rlogin, backend);
Allow new_connection to take an optional Seat. (NFC) This is working towards allowing the subsidiary SSH connection in an SshProxy to share the main user-facing Seat, so as to be able to pass through interactive prompts. This is more difficult than the similar change with LogPolicy, because Seats are stateful. In particular, the trust-sigil status will need to be controlled by the SshProxy until it's ready to pass over control to the main SSH (or whatever) connection. To make this work, I've introduced a thing called a TempSeat, which is (yet) another Seat implementation. When a backend hands its Seat to new_connection(), it does it in a way that allows new_connection() to borrow it completely, and replace it in the main backend structure with a TempSeat, which acts as a temporary placeholder. If the main backend tries to do things like changing trust status or sending output, the TempSeat will buffer them; later on, when the connection is established, TempSeat will replay the changes into the real Seat. So, in each backend, I've made the following changes: - pass &foo->seat to new_connection, which may overwrite it with a TempSeat. - if it has done so (which we can tell via the is_tempseat() query function), then we have to free the TempSeat and reinstate our main Seat. The signal that we can do so is the PLUGLOG_CONNECT_SUCCESS notification, which indicates that SshProxy has finished all its connection setup work. - we also have to remember to free the TempSeat if our backend is disposed of without that having happened (e.g. because the connection _doesn't_ succeed). - in backends which have no local auth phase to worry about, ensure we don't call seat_set_trust_status on the main Seat _before_ it gets potentially replaced with a TempSeat. Moved some calls of seat_set_trust_status to just after new_connection(), so that now the initial trust status setup will go into the TempSeat (if appropriate) and be buffered until that seat is relinquished. In all other uses of new_connection, where we don't have a Seat available at all, we just pass NULL. This is NFC, because neither new_connection() nor any of its delegates will _actually_ do this replacement yet. We're just setting up the framework to enable it to do so in the next commit.
2021-09-13 17:17:20 +01:00
if (is_tempseat(rlogin->seat))
tempseat_free(rlogin->seat);
if (rlogin->prompt)
free_prompts(rlogin->prompt);
if (rlogin->s)
sk_close(rlogin->s);
Post-release destabilisation! Completely remove the struct type 'Config' in putty.h, which stores all PuTTY's settings and includes an arbitrary length limit on every single one of those settings which is stored in string form. In place of it is 'Conf', an opaque data type everywhere outside the new file conf.c, which stores a list of (key, value) pairs in which every key contains an integer identifying a configuration setting, and for some of those integers the key also contains extra parts (so that, for instance, CONF_environmt is a string-to-string mapping). Everywhere that a Config was previously used, a Conf is now; everywhere there was a Config structure copy, conf_copy() is called; every lookup, adjustment, load and save operation on a Config has been rewritten; and there's a mechanism for serialising a Conf into a binary blob and back for use with Duplicate Session. User-visible effects of this change _should_ be minimal, though I don't doubt I've introduced one or two bugs here and there which will eventually be found. The _intended_ visible effects of this change are that all arbitrary limits on configuration strings and lists (e.g. limit on number of port forwardings) should now disappear; that list boxes in the configuration will now be displayed in a sorted order rather than the arbitrary order in which they were added to the list (since the underlying data structure is now a sorted tree234 rather than an ad-hoc comma-separated string); and one more specific change, which is that local and dynamic port forwardings on the same port number are now mutually exclusive in the configuration (putting 'D' in the key rather than the value was a mistake in the first place). One other reorganisation as a result of this is that I've moved all the dialog.c standard handlers (dlg_stdeditbox_handler and friends) out into config.c, because I can't really justify calling them generic any more. When they took a pointer to an arbitrary structure type and the offset of a field within that structure, they were independent of whether that structure was a Config or something completely different, but now they really do expect to talk to a Conf, which can _only_ be used for PuTTY configuration, so I've renamed them all things like conf_editbox_handler and moved them out of the nominally independent dialog-box management module into the PuTTY-specific config.c. [originally from svn r9214]
2011-07-14 18:52:21 +00:00
conf_free(rlogin->conf);
Add 'description' methods for Backend and Plug. These will typically be implemented by objects that are both a Backend *and* a Plug, and the two methods will deliver the same results to any caller, regardless of which facet of the object is known to that caller. Their purpose is to deliver a user-oriented natural-language description of what network connection the object is handling, so that it can appear in diagnostic messages. The messages I specifically have in mind are going to appear in cases where proxies require interactive authentication: when PuTTY prompts interactively for a password, it will need to explain which *thing* it's asking for the password for, and these descriptions are what it will use to describe the thing in question. Each backend is allowed to compose these messages however it thinks best. In all cases at present, the description string is constructed by the new centralised default_description() function, which takes a host name and port number and combines them with the backend's display name. But the SSH backend does things a bit differently, because it uses the _logical_ host name (the one that goes with the SSH host key) rather than the physical destination of the network connection. That seems more appropriate when the question it's really helping the user to answer is "What host am I supposed to be entering the password for?" In this commit, no clients of the new methods are introduced. I have a draft implementation of actually using it for the purpose I describe above, but it needs polishing.
2021-10-24 09:18:12 +01:00
sfree(rlogin->description);
sfree(rlogin);
}
/*
* Stub routine (we don't have any need to reconfigure this backend).
*/
static void rlogin_reconfig(Backend *be, Conf *conf)
{
}
Complete rework of terminal userpass input system. The system for handling seat_get_userpass_input has always been structured differently between GUI PuTTY and CLI tools like Plink. In the CLI tools, password input is read directly from the OS terminal/console device by console_get_userpass_input; this means that you need to ensure the same terminal input data _hasn't_ already been consumed by the main event loop and sent on to the backend. This is achieved by the backend_sendok() method, which tells the event loop when the backend has finished issuing password prompts, and hence, when it's safe to start passing standard input to backend_send(). But in the GUI tools, input generated by the terminal window has always been sent straight to backend_send(), regardless of whether backend_sendok() says it wants it. So the terminal-based implementation of username and password prompts has to work by consuming input data that had _already_ been passed to the backend - hence, any backend that needs to do that must keep its input on a bufchain, and pass that bufchain to seat_get_userpass_input. It's awkward that these two totally different systems coexist in the first place. And now that SSH proxying needs to present interactive prompts of its own, it's clear which one should win: the CLI style is the Right Thing. So this change reworks the GUI side of the mechanism to be more similar: terminal data now goes into a queue in the Ldisc, and is not sent on to the backend until the backend says it's ready for it via backend_sendok(). So terminal-based userpass prompts can now consume data directly from that queue during the connection setup stage. As a result, the 'bufchain *' parameter has vanished from all the userpass_input functions (both the official implementations of the Seat trait method, and term_get_userpass_input() to which some of those implementations delegate). The only function that actually used that bufchain, namely term_get_userpass_input(), now instead reads from the ldisc's input queue via a couple of new Ldisc functions. (Not _trivial_ functions, since input buffered by Ldisc can be a mixture of raw bytes and session specials like SS_EOL! The input queue inside Ldisc is a bufchain containing a fiddly binary encoding that can represent an arbitrary interleaving of those things.) This greatly simplifies the calls to seat_get_userpass_input in backends, which now don't have to mess about with passing their own user_input bufchain around, or toggling their want_user_input flag back and forth to request data to put on to that bufchain. But the flip side is that now there has to be some _other_ method for notifying the terminal when there's more input to be consumed during an interactive prompt, and for notifying the backend when prompt input has finished so that it can proceed to the next stage of the protocol. This is done by a pair of extra callbacks: when more data is put on to Ldisc's input queue, it triggers a call to term_get_userpass_input, and when term_get_userpass_input finishes, it calls a callback function provided in the prompts_t. Therefore, any use of a prompts_t which *might* be asynchronous must fill in the latter callback when setting up the prompts_t. In SSH, the callback is centralised into a common PPL helper function, which reinvokes the same PPL's process_queue coroutine; in rlogin we have to set it up ourselves. I'm sorry for this large and sprawling patch: I tried fairly hard to break it up into individually comprehensible sub-patches, but I just couldn't tease out any part of it that would stand sensibly alone.
2021-09-14 11:57:21 +01:00
static void rlogin_try_username_prompt(void *ctx)
{
Rlogin *rlogin = (Rlogin *)ctx;
Framework for announcing which Interactor is talking. All this Interactor business has been gradually working towards being able to inform the user _which_ network connection is currently presenting them with a password prompt (or whatever), in situations where more than one of them might be, such as an SSH connection being used as a proxy for another SSH connection when neither one has one-touch login configured. At some point, we have to arrange that any attempt to do a user interaction during connection setup - be it a password prompt, a host key confirmation dialog, or just displaying an SSH login banner - makes it clear which host it's come from. That's going to mean calling some kind of announcement function before doing any of those things. But there are several of those functions in the Seat API, and calls to them are scattered far and wide across the SSH backend. (And not even just there - the Rlogin backend also uses seat_get_userpass_input). How can we possibly make sure we don't forget a vital call site on some obscure little-tested code path, and leave the user confused in just that one case which nobody might notice for years? Today I thought of a trick to solve that problem. We can use the C type system to enforce it for us! The plan is: we invent a new struct type which contains nothing but a 'Seat *'. Then, for every Seat method which does a thing that ought to be clearly identified as relating to a particular Interactor, we adjust the API for that function to take the new struct type where it previously took a plain 'Seat *'. Or rather - doing less violence to the existing code - we only need to adjust the API of the dispatch functions inline in putty.h. How does that help? Because the way you _get_ one of these struct-wrapped Seat pointers is by calling interactor_announce() on your Interactor, which will in turn call interactor_get_seat(), and wrap the returned pointer into one of these structs. The effect is that whenever the SSH (or Rlogin) code wants to call one of those particular Seat methods, it _has_ to call interactor_announce() just beforehand, which (once I finish all of this) will make sure the user is aware of who is presenting the prompt or banner or whatever. And you can't forget to call it, because if you don't call it, then you just don't have a struct of the right type to give to the Seat method you wanted to call! (Of course, there's nothing stopping code from _deliberately_ taking a Seat * it already has and wrapping it into the new struct. In fact SshProxy has to do that, in order to forward these requests up the chain of Seats. But the point is that you can't do it _by accident_, just by forgetting to make a vital function call - when you do that, you _know_ you're doing it on purpose.) No functional change: the new interactor_announce() function exists, and the type-system trick ensures it's called in all the right places, but it doesn't actually _do_ anything yet.
2021-10-30 18:05:36 +01:00
int ret = seat_get_userpass_input(
interactor_announce(&rlogin->interactor), rlogin->prompt);
Complete rework of terminal userpass input system. The system for handling seat_get_userpass_input has always been structured differently between GUI PuTTY and CLI tools like Plink. In the CLI tools, password input is read directly from the OS terminal/console device by console_get_userpass_input; this means that you need to ensure the same terminal input data _hasn't_ already been consumed by the main event loop and sent on to the backend. This is achieved by the backend_sendok() method, which tells the event loop when the backend has finished issuing password prompts, and hence, when it's safe to start passing standard input to backend_send(). But in the GUI tools, input generated by the terminal window has always been sent straight to backend_send(), regardless of whether backend_sendok() says it wants it. So the terminal-based implementation of username and password prompts has to work by consuming input data that had _already_ been passed to the backend - hence, any backend that needs to do that must keep its input on a bufchain, and pass that bufchain to seat_get_userpass_input. It's awkward that these two totally different systems coexist in the first place. And now that SSH proxying needs to present interactive prompts of its own, it's clear which one should win: the CLI style is the Right Thing. So this change reworks the GUI side of the mechanism to be more similar: terminal data now goes into a queue in the Ldisc, and is not sent on to the backend until the backend says it's ready for it via backend_sendok(). So terminal-based userpass prompts can now consume data directly from that queue during the connection setup stage. As a result, the 'bufchain *' parameter has vanished from all the userpass_input functions (both the official implementations of the Seat trait method, and term_get_userpass_input() to which some of those implementations delegate). The only function that actually used that bufchain, namely term_get_userpass_input(), now instead reads from the ldisc's input queue via a couple of new Ldisc functions. (Not _trivial_ functions, since input buffered by Ldisc can be a mixture of raw bytes and session specials like SS_EOL! The input queue inside Ldisc is a bufchain containing a fiddly binary encoding that can represent an arbitrary interleaving of those things.) This greatly simplifies the calls to seat_get_userpass_input in backends, which now don't have to mess about with passing their own user_input bufchain around, or toggling their want_user_input flag back and forth to request data to put on to that bufchain. But the flip side is that now there has to be some _other_ method for notifying the terminal when there's more input to be consumed during an interactive prompt, and for notifying the backend when prompt input has finished so that it can proceed to the next stage of the protocol. This is done by a pair of extra callbacks: when more data is put on to Ldisc's input queue, it triggers a call to term_get_userpass_input, and when term_get_userpass_input finishes, it calls a callback function provided in the prompts_t. Therefore, any use of a prompts_t which *might* be asynchronous must fill in the latter callback when setting up the prompts_t. In SSH, the callback is centralised into a common PPL helper function, which reinvokes the same PPL's process_queue coroutine; in rlogin we have to set it up ourselves. I'm sorry for this large and sprawling patch: I tried fairly hard to break it up into individually comprehensible sub-patches, but I just couldn't tease out any part of it that would stand sensibly alone.
2021-09-14 11:57:21 +01:00
if (ret < 0)
return;
/* Next terminal output will come from server */
seat_set_trust_status(rlogin->seat, false);
/* Send the rlogin setup protocol data, and then we're ready to
* start receiving normal input to send down the wire, which
* rlogin_startup will signal to rlogin_sendok by nulling out
* rlogin->prompt. */
rlogin_startup(
rlogin, ret, prompt_get_result_ref(rlogin->prompt->prompts[0]));
}
/*
* Called to send data down the rlogin connection.
*/
static void rlogin_send(Backend *be, const char *buf, size_t len)
{
Rlogin *rlogin = container_of(be, Rlogin, backend);
if (rlogin->s == NULL)
return;
Complete rework of terminal userpass input system. The system for handling seat_get_userpass_input has always been structured differently between GUI PuTTY and CLI tools like Plink. In the CLI tools, password input is read directly from the OS terminal/console device by console_get_userpass_input; this means that you need to ensure the same terminal input data _hasn't_ already been consumed by the main event loop and sent on to the backend. This is achieved by the backend_sendok() method, which tells the event loop when the backend has finished issuing password prompts, and hence, when it's safe to start passing standard input to backend_send(). But in the GUI tools, input generated by the terminal window has always been sent straight to backend_send(), regardless of whether backend_sendok() says it wants it. So the terminal-based implementation of username and password prompts has to work by consuming input data that had _already_ been passed to the backend - hence, any backend that needs to do that must keep its input on a bufchain, and pass that bufchain to seat_get_userpass_input. It's awkward that these two totally different systems coexist in the first place. And now that SSH proxying needs to present interactive prompts of its own, it's clear which one should win: the CLI style is the Right Thing. So this change reworks the GUI side of the mechanism to be more similar: terminal data now goes into a queue in the Ldisc, and is not sent on to the backend until the backend says it's ready for it via backend_sendok(). So terminal-based userpass prompts can now consume data directly from that queue during the connection setup stage. As a result, the 'bufchain *' parameter has vanished from all the userpass_input functions (both the official implementations of the Seat trait method, and term_get_userpass_input() to which some of those implementations delegate). The only function that actually used that bufchain, namely term_get_userpass_input(), now instead reads from the ldisc's input queue via a couple of new Ldisc functions. (Not _trivial_ functions, since input buffered by Ldisc can be a mixture of raw bytes and session specials like SS_EOL! The input queue inside Ldisc is a bufchain containing a fiddly binary encoding that can represent an arbitrary interleaving of those things.) This greatly simplifies the calls to seat_get_userpass_input in backends, which now don't have to mess about with passing their own user_input bufchain around, or toggling their want_user_input flag back and forth to request data to put on to that bufchain. But the flip side is that now there has to be some _other_ method for notifying the terminal when there's more input to be consumed during an interactive prompt, and for notifying the backend when prompt input has finished so that it can proceed to the next stage of the protocol. This is done by a pair of extra callbacks: when more data is put on to Ldisc's input queue, it triggers a call to term_get_userpass_input, and when term_get_userpass_input finishes, it calls a callback function provided in the prompts_t. Therefore, any use of a prompts_t which *might* be asynchronous must fill in the latter callback when setting up the prompts_t. In SSH, the callback is centralised into a common PPL helper function, which reinvokes the same PPL's process_queue coroutine; in rlogin we have to set it up ourselves. I'm sorry for this large and sprawling patch: I tried fairly hard to break it up into individually comprehensible sub-patches, but I just couldn't tease out any part of it that would stand sensibly alone.
2021-09-14 11:57:21 +01:00
rlogin->bufsize = sk_write(rlogin->s, buf, len);
}
/*
* Called to query the current socket sendability status.
*/
static size_t rlogin_sendbuffer(Backend *be)
{
Rlogin *rlogin = container_of(be, Rlogin, backend);
return rlogin->bufsize;
}
/*
* Called to set the size of the window
*/
static void rlogin_size(Backend *be, int width, int height)
{
Rlogin *rlogin = container_of(be, Rlogin, backend);
char b[12] = { '\xFF', '\xFF', 0x73, 0x73, 0, 0, 0, 0, 0, 0, 0, 0 };
rlogin->term_width = width;
rlogin->term_height = height;
if (rlogin->s == NULL || !rlogin->cansize)
return;
b[6] = rlogin->term_width >> 8;
b[7] = rlogin->term_width & 0xFF;
b[4] = rlogin->term_height >> 8;
b[5] = rlogin->term_height & 0xFF;
rlogin->bufsize = sk_write(rlogin->s, b, 12);
return;
}
/*
* Send rlogin special codes.
*/
static void rlogin_special(Backend *be, SessionSpecialCode code, int arg)
{
/* Do nothing! */
return;
}
/*
* Return a list of the special codes that make sense in this
* protocol.
*/
static const SessionSpecial *rlogin_get_specials(Backend *be)
{
return NULL;
}
Convert a lot of 'int' variables to 'bool'. 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'!
2018-11-02 19:23:19 +00:00
static bool rlogin_connected(Backend *be)
{
Rlogin *rlogin = container_of(be, Rlogin, backend);
return rlogin->s != NULL;
}
Convert a lot of 'int' variables to 'bool'. 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'!
2018-11-02 19:23:19 +00:00
static bool rlogin_sendok(Backend *be)
{
Complete rework of terminal userpass input system. The system for handling seat_get_userpass_input has always been structured differently between GUI PuTTY and CLI tools like Plink. In the CLI tools, password input is read directly from the OS terminal/console device by console_get_userpass_input; this means that you need to ensure the same terminal input data _hasn't_ already been consumed by the main event loop and sent on to the backend. This is achieved by the backend_sendok() method, which tells the event loop when the backend has finished issuing password prompts, and hence, when it's safe to start passing standard input to backend_send(). But in the GUI tools, input generated by the terminal window has always been sent straight to backend_send(), regardless of whether backend_sendok() says it wants it. So the terminal-based implementation of username and password prompts has to work by consuming input data that had _already_ been passed to the backend - hence, any backend that needs to do that must keep its input on a bufchain, and pass that bufchain to seat_get_userpass_input. It's awkward that these two totally different systems coexist in the first place. And now that SSH proxying needs to present interactive prompts of its own, it's clear which one should win: the CLI style is the Right Thing. So this change reworks the GUI side of the mechanism to be more similar: terminal data now goes into a queue in the Ldisc, and is not sent on to the backend until the backend says it's ready for it via backend_sendok(). So terminal-based userpass prompts can now consume data directly from that queue during the connection setup stage. As a result, the 'bufchain *' parameter has vanished from all the userpass_input functions (both the official implementations of the Seat trait method, and term_get_userpass_input() to which some of those implementations delegate). The only function that actually used that bufchain, namely term_get_userpass_input(), now instead reads from the ldisc's input queue via a couple of new Ldisc functions. (Not _trivial_ functions, since input buffered by Ldisc can be a mixture of raw bytes and session specials like SS_EOL! The input queue inside Ldisc is a bufchain containing a fiddly binary encoding that can represent an arbitrary interleaving of those things.) This greatly simplifies the calls to seat_get_userpass_input in backends, which now don't have to mess about with passing their own user_input bufchain around, or toggling their want_user_input flag back and forth to request data to put on to that bufchain. But the flip side is that now there has to be some _other_ method for notifying the terminal when there's more input to be consumed during an interactive prompt, and for notifying the backend when prompt input has finished so that it can proceed to the next stage of the protocol. This is done by a pair of extra callbacks: when more data is put on to Ldisc's input queue, it triggers a call to term_get_userpass_input, and when term_get_userpass_input finishes, it calls a callback function provided in the prompts_t. Therefore, any use of a prompts_t which *might* be asynchronous must fill in the latter callback when setting up the prompts_t. In SSH, the callback is centralised into a common PPL helper function, which reinvokes the same PPL's process_queue coroutine; in rlogin we have to set it up ourselves. I'm sorry for this large and sprawling patch: I tried fairly hard to break it up into individually comprehensible sub-patches, but I just couldn't tease out any part of it that would stand sensibly alone.
2021-09-14 11:57:21 +01:00
/*
* We only want to receive input data if the socket is connected
* and we're not still at the username prompt stage.
*/
Rlogin *rlogin = container_of(be, Rlogin, backend);
Complete rework of terminal userpass input system. The system for handling seat_get_userpass_input has always been structured differently between GUI PuTTY and CLI tools like Plink. In the CLI tools, password input is read directly from the OS terminal/console device by console_get_userpass_input; this means that you need to ensure the same terminal input data _hasn't_ already been consumed by the main event loop and sent on to the backend. This is achieved by the backend_sendok() method, which tells the event loop when the backend has finished issuing password prompts, and hence, when it's safe to start passing standard input to backend_send(). But in the GUI tools, input generated by the terminal window has always been sent straight to backend_send(), regardless of whether backend_sendok() says it wants it. So the terminal-based implementation of username and password prompts has to work by consuming input data that had _already_ been passed to the backend - hence, any backend that needs to do that must keep its input on a bufchain, and pass that bufchain to seat_get_userpass_input. It's awkward that these two totally different systems coexist in the first place. And now that SSH proxying needs to present interactive prompts of its own, it's clear which one should win: the CLI style is the Right Thing. So this change reworks the GUI side of the mechanism to be more similar: terminal data now goes into a queue in the Ldisc, and is not sent on to the backend until the backend says it's ready for it via backend_sendok(). So terminal-based userpass prompts can now consume data directly from that queue during the connection setup stage. As a result, the 'bufchain *' parameter has vanished from all the userpass_input functions (both the official implementations of the Seat trait method, and term_get_userpass_input() to which some of those implementations delegate). The only function that actually used that bufchain, namely term_get_userpass_input(), now instead reads from the ldisc's input queue via a couple of new Ldisc functions. (Not _trivial_ functions, since input buffered by Ldisc can be a mixture of raw bytes and session specials like SS_EOL! The input queue inside Ldisc is a bufchain containing a fiddly binary encoding that can represent an arbitrary interleaving of those things.) This greatly simplifies the calls to seat_get_userpass_input in backends, which now don't have to mess about with passing their own user_input bufchain around, or toggling their want_user_input flag back and forth to request data to put on to that bufchain. But the flip side is that now there has to be some _other_ method for notifying the terminal when there's more input to be consumed during an interactive prompt, and for notifying the backend when prompt input has finished so that it can proceed to the next stage of the protocol. This is done by a pair of extra callbacks: when more data is put on to Ldisc's input queue, it triggers a call to term_get_userpass_input, and when term_get_userpass_input finishes, it calls a callback function provided in the prompts_t. Therefore, any use of a prompts_t which *might* be asynchronous must fill in the latter callback when setting up the prompts_t. In SSH, the callback is centralised into a common PPL helper function, which reinvokes the same PPL's process_queue coroutine; in rlogin we have to set it up ourselves. I'm sorry for this large and sprawling patch: I tried fairly hard to break it up into individually comprehensible sub-patches, but I just couldn't tease out any part of it that would stand sensibly alone.
2021-09-14 11:57:21 +01:00
return rlogin->socket_connected && !rlogin->prompt;
}
static void rlogin_unthrottle(Backend *be, size_t backlog)
{
Rlogin *rlogin = container_of(be, Rlogin, backend);
sk_set_frozen(rlogin->s, backlog > RLOGIN_MAX_BACKLOG);
}
Convert a lot of 'int' variables to 'bool'. 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'!
2018-11-02 19:23:19 +00:00
static bool rlogin_ldisc(Backend *be, int option)
{
/* Rlogin *rlogin = container_of(be, Rlogin, backend); */
Convert a lot of 'int' variables to 'bool'. 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'!
2018-11-02 19:23:19 +00:00
return false;
}
static void rlogin_provide_ldisc(Backend *be, Ldisc *ldisc)
{
Rlogin *rlogin = container_of(be, Rlogin, backend);
rlogin->ldisc = ldisc;
}
static int rlogin_exitcode(Backend *be)
{
Rlogin *rlogin = container_of(be, Rlogin, backend);
if (rlogin->s != NULL)
return -1; /* still connected */
else if (rlogin->closed_on_socket_error)
return INT_MAX; /* a socket error counts as an unclean exit */
else
/* If we ever implement RSH, we'll probably need to do this properly */
return 0;
}
/*
* cfg_info for rlogin does nothing at all.
*/
static int rlogin_cfg_info(Backend *be)
{
return 0;
}
const BackendVtable rlogin_backend = {
.init = rlogin_init,
.free = rlogin_free,
.reconfig = rlogin_reconfig,
.send = rlogin_send,
.sendbuffer = rlogin_sendbuffer,
.size = rlogin_size,
.special = rlogin_special,
.get_specials = rlogin_get_specials,
.connected = rlogin_connected,
.exitcode = rlogin_exitcode,
.sendok = rlogin_sendok,
.ldisc_option_state = rlogin_ldisc,
.provide_ldisc = rlogin_provide_ldisc,
.unthrottle = rlogin_unthrottle,
.cfg_info = rlogin_cfg_info,
.id = "rlogin",
.displayname_tc = "Rlogin",
.displayname_lc = "Rlogin", /* proper name, so capitalise it anyway */
.protocol = PROT_RLOGIN,
.default_port = 513,
};