putty/unix/uxagentc.c

227 строки
5.7 KiB
C
Исходник Обычный вид История

/*
* SSH agent client code.
*/
#include <stdio.h>
#include <stdlib.h>
#include <assert.h>
#include <unistd.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <fcntl.h>
#include "putty.h"
#include "misc.h"
#include "tree234.h"
#include "puttymem.h"
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 22:23:19 +03:00
bool agent_exists(void)
{
const char *p = getenv("SSH_AUTH_SOCK");
if (p && *p)
return true;
return false;
}
static tree234 *agent_pending_queries;
struct agent_pending_query {
int fd;
char *retbuf;
char sizebuf[4];
int retsize, retlen;
void (*callback)(void *, void *, int);
void *callback_ctx;
};
static int agent_conncmp(void *av, void *bv)
{
agent_pending_query *a = (agent_pending_query *) av;
agent_pending_query *b = (agent_pending_query *) bv;
if (a->fd < b->fd)
return -1;
if (a->fd > b->fd)
return +1;
return 0;
}
static int agent_connfind(void *av, void *bv)
{
int afd = *(int *) av;
agent_pending_query *b = (agent_pending_query *) bv;
if (afd < b->fd)
return -1;
if (afd > b->fd)
return +1;
return 0;
}
/*
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 22:23:19 +03:00
* Attempt to read from an agent socket fd. Returns false if the
* expected response is as yet incomplete; returns true if it's either
* complete (conn->retbuf non-NULL and filled with something useful)
* or has failed totally (conn->retbuf is 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 22:23:19 +03:00
static bool agent_try_read(agent_pending_query *conn)
{
int ret;
ret = read(conn->fd, conn->retbuf+conn->retlen,
conn->retsize-conn->retlen);
if (ret <= 0) {
if (conn->retbuf != conn->sizebuf) sfree(conn->retbuf);
conn->retbuf = NULL;
conn->retlen = 0;
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 22:23:19 +03:00
return true;
}
conn->retlen += ret;
if (conn->retsize == 4 && conn->retlen == 4) {
conn->retsize = toint(GET_32BIT_MSB_FIRST(conn->retbuf) + 4);
if (conn->retsize <= 0) {
conn->retbuf = NULL;
conn->retlen = 0;
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 22:23:19 +03:00
return true; /* way too large */
}
assert(conn->retbuf == conn->sizebuf);
conn->retbuf = snewn(conn->retsize, char);
memcpy(conn->retbuf, conn->sizebuf, 4);
}
if (conn->retlen < conn->retsize)
return false; /* more data to come */
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 22:23:19 +03:00
return true;
}
void agent_cancel_query(agent_pending_query *conn)
{
uxsel_del(conn->fd);
close(conn->fd);
del234(agent_pending_queries, conn);
if (conn->retbuf && conn->retbuf != conn->sizebuf)
sfree(conn->retbuf);
sfree(conn);
}
static void agent_select_result(int fd, int event)
{
agent_pending_query *conn;
assert(event == SELECT_R); /* not selecting for anything but R */
conn = find234(agent_pending_queries, &fd, agent_connfind);
if (!conn) {
uxsel_del(fd);
return;
}
if (!agent_try_read(conn))
return; /* more data to come */
/*
* We have now completed the agent query. Do the callback.
*/
conn->callback(conn->callback_ctx, conn->retbuf, conn->retlen);
/* Null out conn->retbuf, since ownership of that buffer has
* passed to the callback. */
conn->retbuf = NULL;
agent_cancel_query(conn);
}
Stream-oriented agent forwarding on Unix. Historically, because of the way Windows Pageant's IPC works, PuTTY's agent forwarding has always been message-oriented. The channel implementation in agentf.c deals with receiving a data stream from the remote agent client and breaking it up into messages, and then it passes each message individually to agent_query(). On Unix, this is more work than is really needed, and I've always meant to get round to doing the more obvious thing: making an agent forwarding channel into simply a stream-oriented proxy, passing raw data back and forth between the SSH channel and the local AF_UNIX socket without having to know or care about the message boundaries in the stream. The portfwdmgr_connect_socket() facility introduced by the previous commit is the missing piece of infrastructure to make that possible. Now, the agent client module provides an API that includes a callback you can pass to portfwdmgr_connect_socket() to open a streamed agent connection, and the agent forwarding setup function tries to use that where possible, only falling back to the message-based agentf.c system if it can't be done. On Windows, the new piece of agent-client API returns failure, so we still fall back to agentf.c there. There are two benefits to doing it this way. One is that it's just simpler and more robust: if PuTTY isn't trying to parse the agent connection, then it has less work to do and fewer places to introduce bugs. The other is that it's futureproof against changes in the agent protocol: if any kind of extension is ever introduced that requires keeping state within a single agent connection, or that changes the protocol itself so that agentf's message-boundary detection stops working, then this forwarding system will still work.
2020-01-01 19:46:44 +03:00
static const char *agent_socket_path(void)
{
return getenv("SSH_AUTH_SOCK");
}
Merge the two low-level portfwd setup systems. In commit 09954a87c I introduced the portfwdmgr_connect_socket() system, which opened a port forwarding given a callback to create the Socket itself, with the aim of using it to make forwardings to Unix- domain sockets and Windows named pipes (both initially for agent forwarding). But I forgot that a year and a bit ago, in commit 834396170, I already introduced a similar low-level system for creating a PortForwarding around an unusual kind of socket: the portfwd_raw_new() system, which in place of a callback uses a two-phase setup protocol (you create the socket in between the two setup calls, and can roll it back if the socket can't be created). There's really no need to have _both_ these systems! So now I'm merging them, which is to say, I'm enhancing portfwd_raw_new to have the one new feature it needs, and throwing away the newer system completely. The new feature is to be able to control the initial state of the 'ready' flag: portfwd_raw_new was always used for initiating port forwardings in response to an incoming local connection, which means you need to start off with ready=false and set it true when the other end of the SSH connection sends back OPEN_CONFIRMATION. Now it's being used for initiating port forwardings in response to a CHANNEL_OPEN, we need to be able to start with ready=true. This commit reverts 09954a87c24e84dac133a9c29ffaef45f145eeca and its followup fix 12aa06ccc98cf8a912eb2ea54f02d234f2f8c173, and simplifies the agent_connect system down to a single trivial function that makes a Socket given a Plug.
2020-01-27 22:34:15 +03:00
Socket *agent_connect(Plug *plug)
Stream-oriented agent forwarding on Unix. Historically, because of the way Windows Pageant's IPC works, PuTTY's agent forwarding has always been message-oriented. The channel implementation in agentf.c deals with receiving a data stream from the remote agent client and breaking it up into messages, and then it passes each message individually to agent_query(). On Unix, this is more work than is really needed, and I've always meant to get round to doing the more obvious thing: making an agent forwarding channel into simply a stream-oriented proxy, passing raw data back and forth between the SSH channel and the local AF_UNIX socket without having to know or care about the message boundaries in the stream. The portfwdmgr_connect_socket() facility introduced by the previous commit is the missing piece of infrastructure to make that possible. Now, the agent client module provides an API that includes a callback you can pass to portfwdmgr_connect_socket() to open a streamed agent connection, and the agent forwarding setup function tries to use that where possible, only falling back to the message-based agentf.c system if it can't be done. On Windows, the new piece of agent-client API returns failure, so we still fall back to agentf.c there. There are two benefits to doing it this way. One is that it's just simpler and more robust: if PuTTY isn't trying to parse the agent connection, then it has less work to do and fewer places to introduce bugs. The other is that it's futureproof against changes in the agent protocol: if any kind of extension is ever introduced that requires keeping state within a single agent connection, or that changes the protocol itself so that agentf's message-boundary detection stops working, then this forwarding system will still work.
2020-01-01 19:46:44 +03:00
{
const char *path = agent_socket_path();
if (!path)
Merge the two low-level portfwd setup systems. In commit 09954a87c I introduced the portfwdmgr_connect_socket() system, which opened a port forwarding given a callback to create the Socket itself, with the aim of using it to make forwardings to Unix- domain sockets and Windows named pipes (both initially for agent forwarding). But I forgot that a year and a bit ago, in commit 834396170, I already introduced a similar low-level system for creating a PortForwarding around an unusual kind of socket: the portfwd_raw_new() system, which in place of a callback uses a two-phase setup protocol (you create the socket in between the two setup calls, and can roll it back if the socket can't be created). There's really no need to have _both_ these systems! So now I'm merging them, which is to say, I'm enhancing portfwd_raw_new to have the one new feature it needs, and throwing away the newer system completely. The new feature is to be able to control the initial state of the 'ready' flag: portfwd_raw_new was always used for initiating port forwardings in response to an incoming local connection, which means you need to start off with ready=false and set it true when the other end of the SSH connection sends back OPEN_CONFIRMATION. Now it's being used for initiating port forwardings in response to a CHANNEL_OPEN, we need to be able to start with ready=true. This commit reverts 09954a87c24e84dac133a9c29ffaef45f145eeca and its followup fix 12aa06ccc98cf8a912eb2ea54f02d234f2f8c173, and simplifies the agent_connect system down to a single trivial function that makes a Socket given a Plug.
2020-01-27 22:34:15 +03:00
return new_error_socket_fmt(plug, "SSH_AUTH_SOCK not set");
return sk_new(unix_sock_addr(path), 0, false, false, false, false, plug);
Stream-oriented agent forwarding on Unix. Historically, because of the way Windows Pageant's IPC works, PuTTY's agent forwarding has always been message-oriented. The channel implementation in agentf.c deals with receiving a data stream from the remote agent client and breaking it up into messages, and then it passes each message individually to agent_query(). On Unix, this is more work than is really needed, and I've always meant to get round to doing the more obvious thing: making an agent forwarding channel into simply a stream-oriented proxy, passing raw data back and forth between the SSH channel and the local AF_UNIX socket without having to know or care about the message boundaries in the stream. The portfwdmgr_connect_socket() facility introduced by the previous commit is the missing piece of infrastructure to make that possible. Now, the agent client module provides an API that includes a callback you can pass to portfwdmgr_connect_socket() to open a streamed agent connection, and the agent forwarding setup function tries to use that where possible, only falling back to the message-based agentf.c system if it can't be done. On Windows, the new piece of agent-client API returns failure, so we still fall back to agentf.c there. There are two benefits to doing it this way. One is that it's just simpler and more robust: if PuTTY isn't trying to parse the agent connection, then it has less work to do and fewer places to introduce bugs. The other is that it's futureproof against changes in the agent protocol: if any kind of extension is ever introduced that requires keeping state within a single agent connection, or that changes the protocol itself so that agentf's message-boundary detection stops working, then this forwarding system will still work.
2020-01-01 19:46:44 +03:00
}
agent_pending_query *agent_query(
strbuf *query, void **out, int *outlen,
void (*callback)(void *, void *, int), void *callback_ctx)
{
Stream-oriented agent forwarding on Unix. Historically, because of the way Windows Pageant's IPC works, PuTTY's agent forwarding has always been message-oriented. The channel implementation in agentf.c deals with receiving a data stream from the remote agent client and breaking it up into messages, and then it passes each message individually to agent_query(). On Unix, this is more work than is really needed, and I've always meant to get round to doing the more obvious thing: making an agent forwarding channel into simply a stream-oriented proxy, passing raw data back and forth between the SSH channel and the local AF_UNIX socket without having to know or care about the message boundaries in the stream. The portfwdmgr_connect_socket() facility introduced by the previous commit is the missing piece of infrastructure to make that possible. Now, the agent client module provides an API that includes a callback you can pass to portfwdmgr_connect_socket() to open a streamed agent connection, and the agent forwarding setup function tries to use that where possible, only falling back to the message-based agentf.c system if it can't be done. On Windows, the new piece of agent-client API returns failure, so we still fall back to agentf.c there. There are two benefits to doing it this way. One is that it's just simpler and more robust: if PuTTY isn't trying to parse the agent connection, then it has less work to do and fewer places to introduce bugs. The other is that it's futureproof against changes in the agent protocol: if any kind of extension is ever introduced that requires keeping state within a single agent connection, or that changes the protocol itself so that agentf's message-boundary detection stops working, then this forwarding system will still work.
2020-01-01 19:46:44 +03:00
const char *name;
int sock;
struct sockaddr_un addr;
int done;
agent_pending_query *conn;
Stream-oriented agent forwarding on Unix. Historically, because of the way Windows Pageant's IPC works, PuTTY's agent forwarding has always been message-oriented. The channel implementation in agentf.c deals with receiving a data stream from the remote agent client and breaking it up into messages, and then it passes each message individually to agent_query(). On Unix, this is more work than is really needed, and I've always meant to get round to doing the more obvious thing: making an agent forwarding channel into simply a stream-oriented proxy, passing raw data back and forth between the SSH channel and the local AF_UNIX socket without having to know or care about the message boundaries in the stream. The portfwdmgr_connect_socket() facility introduced by the previous commit is the missing piece of infrastructure to make that possible. Now, the agent client module provides an API that includes a callback you can pass to portfwdmgr_connect_socket() to open a streamed agent connection, and the agent forwarding setup function tries to use that where possible, only falling back to the message-based agentf.c system if it can't be done. On Windows, the new piece of agent-client API returns failure, so we still fall back to agentf.c there. There are two benefits to doing it this way. One is that it's just simpler and more robust: if PuTTY isn't trying to parse the agent connection, then it has less work to do and fewer places to introduce bugs. The other is that it's futureproof against changes in the agent protocol: if any kind of extension is ever introduced that requires keeping state within a single agent connection, or that changes the protocol itself so that agentf's message-boundary detection stops working, then this forwarding system will still work.
2020-01-01 19:46:44 +03:00
name = agent_socket_path();
if (!name || strlen(name) >= sizeof(addr.sun_path))
goto failure;
sock = socket(PF_UNIX, SOCK_STREAM, 0);
if (sock < 0) {
perror("socket(PF_UNIX)");
exit(1);
}
cloexec(sock);
addr.sun_family = AF_UNIX;
strcpy(addr.sun_path, name);
if (connect(sock, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
close(sock);
goto failure;
}
strbuf_finalise_agent_query(query);
for (done = 0; done < query->len ;) {
int ret = write(sock, query->s + done,
query->len - done);
if (ret <= 0) {
close(sock);
goto failure;
}
done += ret;
}
conn = snew(agent_pending_query);
conn->fd = sock;
conn->retbuf = conn->sizebuf;
conn->retsize = 4;
conn->retlen = 0;
conn->callback = callback;
conn->callback_ctx = callback_ctx;
if (!callback) {
/*
* Bodge to permit making deliberately synchronous agent
* requests. Used by Unix Pageant in command-line client mode,
* which is legit because it really is true that no other part
* of the program is trying to get anything useful done
* simultaneously. But this special case shouldn't be used in
* any more general program.
*/
no_nonblock(conn->fd);
while (!agent_try_read(conn))
/* empty loop body */;
*out = conn->retbuf;
*outlen = conn->retlen;
sfree(conn);
return NULL;
}
/*
* Otherwise do it properly: add conn to the tree of agent
* connections currently in flight, return 0 to indicate that the
* response hasn't been received yet, and call the callback when
* select_result comes back to us.
*/
if (!agent_pending_queries)
agent_pending_queries = newtree234(agent_conncmp);
add234(agent_pending_queries, conn);
uxsel_set(sock, SELECT_R, agent_select_result);
return conn;
failure:
*out = NULL;
*outlen = 0;
return NULL;
}