1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00
putty-source/ssh/common.c

1278 lines
41 KiB
C
Raw Normal View History

/*
* Supporting routines used in common by all the various components of
* the SSH system.
*/
#include <assert.h>
#include <stdlib.h>
#include "putty.h"
Complete rewrite of PuTTY's bignum library. The old 'Bignum' data type is gone completely, and so is sshbn.c. In its place is a new thing called 'mp_int', handled by an entirely new library module mpint.c, with API differences both large and small. The main aim of this change is that the new library should be free of timing- and cache-related side channels. I've written the code so that it _should_ - assuming I haven't made any mistakes - do all of its work without either control flow or memory addressing depending on the data words of the input numbers. (Though, being an _arbitrary_ precision library, it does have to at least depend on the sizes of the numbers - but there's a 'formal' size that can vary separately from the actual magnitude of the represented integer, so if you want to keep it secret that your number is actually small, it should work fine to have a very long mp_int and just happen to store 23 in it.) So I've done all my conditionalisation by means of computing both answers and doing bit-masking to swap the right one into place, and all loops over the words of an mp_int go up to the formal size rather than the actual size. I haven't actually tested the constant-time property in any rigorous way yet (I'm still considering the best way to do it). But this code is surely at the very least a big improvement on the old version, even if I later find a few more things to fix. I've also completely rewritten the low-level elliptic curve arithmetic from sshecc.c; the new ecc.c is closer to being an adjunct of mpint.c than it is to the SSH end of the code. The new elliptic curve code keeps all coordinates in Montgomery-multiplication transformed form to speed up all the multiplications mod the same prime, and only converts them back when you ask for the affine coordinates. Also, I adopted extended coordinates for the Edwards curve implementation. sshecc.c has also had a near-total rewrite in the course of switching it over to the new system. While I was there, I've separated ECDSA and EdDSA more completely - they now have separate vtables, instead of a single vtable in which nearly every function had a big if statement in it - and also made the externally exposed types for an ECDSA key and an ECDH context different. A minor new feature: since the new arithmetic code includes a modular square root function, we can now support the compressed point representation for the NIST curves. We seem to have been getting along fine without that so far, but it seemed a shame not to put it in, since it was suddenly easy. In sshrsa.c, one major change is that I've removed the RSA blinding step in rsa_privkey_op, in which we randomise the ciphertext before doing the decryption. The purpose of that was to avoid timing leaks giving away the plaintext - but the new arithmetic code should take that in its stride in the course of also being careful enough to avoid leaking the _private key_, which RSA blinding had no way to do anything about in any case. Apart from those specific points, most of the rest of the changes are more or less mechanical, just changing type names and translating code into the new API.
2018-12-31 13:53:41 +00:00
#include "mpint.h"
#include "ssh.h"
Reorganise host key checking and confirmation. Previously, checking the host key against the persistent cache managed by the storage.h API was done as part of the seat_verify_ssh_host_key method, i.e. separately by each Seat. Now that check is done by verify_ssh_host_key(), which is a new function in ssh/common.c that centralises all the parts of host key checking that don't need an interactive prompt. It subsumes the previous verify_ssh_manual_host_key() that checked against the Conf, and it does the check against the storage API that each Seat was previously doing separately. If it can't confirm or definitively reject the host key by itself, _then_ it calls out to the Seat, once an interactive prompt is definitely needed. The main point of doing this is so that when SshProxy forwards a Seat call from the proxy SSH connection to the primary Seat, it won't print an announcement of which connection is involved unless it's actually going to do something interactive. (Not that we're printing those announcements _yet_ anyway, but this is a piece of groundwork that works towards doing so.) But while I'm at it, I've also taken the opportunity to clean things up a bit by renaming functions sensibly. Previously we had three very similarly named functions verify_ssh_manual_host_key(), SeatVtable's 'verify_ssh_host_key' method, and verify_host_key() in storage.h. Now the Seat method is called 'confirm' rather than 'verify' (since its job is now always to print an interactive prompt, so it looks more like the other confirm_foo methods), and the storage.h function is called check_stored_host_key(), which goes better with store_host_key and avoids having too many functions with similar names. And the 'manual' function is subsumed into the new centralised code, so there's now just *one* host key function with 'verify' in the name. Several functions are reindented in this commit. Best viewed with whitespace changes ignored.
2021-10-25 17:12:17 +00:00
#include "storage.h"
#include "bpp.h"
#include "ppl.h"
#include "channel.h"
/* ----------------------------------------------------------------------
* Implementation of PacketQueue.
*/
static void pq_ensure_unlinked(PacketQueueNode *node)
{
if (node->on_free_queue) {
node->next->prev = node->prev;
node->prev->next = node->next;
} else {
assert(!node->next);
assert(!node->prev);
}
}
void pq_base_push(PacketQueueBase *pqb, PacketQueueNode *node)
{
pq_ensure_unlinked(node);
node->next = &pqb->end;
node->prev = pqb->end.prev;
node->next->prev = node;
node->prev->next = node;
pqb->total_size += node->formal_size;
if (pqb->ic)
queue_idempotent_callback(pqb->ic);
}
void pq_base_push_front(PacketQueueBase *pqb, PacketQueueNode *node)
{
pq_ensure_unlinked(node);
node->prev = &pqb->end;
node->next = pqb->end.next;
node->next->prev = node;
node->prev->next = node;
pqb->total_size += node->formal_size;
if (pqb->ic)
queue_idempotent_callback(pqb->ic);
}
static PacketQueueNode pktin_freeq_head = {
&pktin_freeq_head, &pktin_freeq_head, true
};
static void pktin_free_queue_callback(void *vctx)
{
while (pktin_freeq_head.next != &pktin_freeq_head) {
PacketQueueNode *node = pktin_freeq_head.next;
PktIn *pktin = container_of(node, PktIn, qnode);
pktin_freeq_head.next = node->next;
sfree(pktin);
}
pktin_freeq_head.prev = &pktin_freeq_head;
}
static IdempotentCallback ic_pktin_free = {
pktin_free_queue_callback, NULL, false
};
static inline void pq_unlink_common(PacketQueueBase *pqb,
PacketQueueNode *node)
{
node->next->prev = node->prev;
node->prev->next = node->next;
/* Check total_size doesn't drift out of sync downwards, by
* ensuring it doesn't underflow when we do this subtraction */
assert(pqb->total_size >= node->formal_size);
pqb->total_size -= node->formal_size;
/* Check total_size doesn't drift out of sync upwards, by checking
* that it's returned to exactly zero whenever a queue is
* emptied */
assert(pqb->end.next != &pqb->end || pqb->total_size == 0);
}
static PktIn *pq_in_after(PacketQueueBase *pqb,
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
PacketQueueNode *prev, bool pop)
{
PacketQueueNode *node = prev->next;
if (node == &pqb->end)
return NULL;
if (pop) {
pq_unlink_common(pqb, node);
node->prev = pktin_freeq_head.prev;
node->next = &pktin_freeq_head;
node->next->prev = node;
node->prev->next = node;
node->on_free_queue = true;
queue_idempotent_callback(&ic_pktin_free);
}
return container_of(node, PktIn, qnode);
}
static PktOut *pq_out_after(PacketQueueBase *pqb,
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
PacketQueueNode *prev, bool pop)
{
PacketQueueNode *node = prev->next;
if (node == &pqb->end)
return NULL;
if (pop) {
pq_unlink_common(pqb, node);
node->prev = node->next = NULL;
}
return container_of(node, PktOut, qnode);
}
void pq_in_init(PktInQueue *pq)
{
pq->pqb.ic = NULL;
pq->pqb.end.next = pq->pqb.end.prev = &pq->pqb.end;
pq->after = pq_in_after;
pq->pqb.total_size = 0;
}
void pq_out_init(PktOutQueue *pq)
{
pq->pqb.ic = NULL;
pq->pqb.end.next = pq->pqb.end.prev = &pq->pqb.end;
pq->after = pq_out_after;
pq->pqb.total_size = 0;
}
void pq_in_clear(PktInQueue *pq)
{
PktIn *pkt;
pq->pqb.ic = NULL;
while ((pkt = pq_pop(pq)) != NULL) {
/* No need to actually free these packets: pq_pop on a
* PktInQueue will automatically move them to the free
* queue. */
}
}
void pq_out_clear(PktOutQueue *pq)
{
PktOut *pkt;
pq->pqb.ic = NULL;
while ((pkt = pq_pop(pq)) != NULL)
ssh_free_pktout(pkt);
}
/*
* Concatenate the contents of the two queues q1 and q2, and leave the
* result in qdest. qdest must be either empty, or one of the input
* queues.
*/
void pq_base_concatenate(PacketQueueBase *qdest,
PacketQueueBase *q1, PacketQueueBase *q2)
{
struct PacketQueueNode *head1, *tail1, *head2, *tail2;
size_t total_size = q1->total_size + q2->total_size;
/*
* Extract the contents from both input queues, and empty them.
*/
head1 = (q1->end.next == &q1->end ? NULL : q1->end.next);
tail1 = (q1->end.prev == &q1->end ? NULL : q1->end.prev);
head2 = (q2->end.next == &q2->end ? NULL : q2->end.next);
tail2 = (q2->end.prev == &q2->end ? NULL : q2->end.prev);
q1->end.next = q1->end.prev = &q1->end;
q2->end.next = q2->end.prev = &q2->end;
q1->total_size = q2->total_size = 0;
/*
* Link the two lists together, handling the case where one or
* both is empty.
*/
if (tail1)
tail1->next = head2;
else
head1 = head2;
if (head2)
head2->prev = tail1;
else
tail2 = tail1;
/*
* Check the destination queue is currently empty. (If it was one
* of the input queues, then it will be, because we emptied both
* of those just a moment ago.)
*/
assert(qdest->end.next == &qdest->end);
assert(qdest->end.prev == &qdest->end);
/*
* If our concatenated list has anything in it, then put it in
* dest.
*/
if (!head1) {
assert(!tail2);
} else {
assert(tail2);
qdest->end.next = head1;
qdest->end.prev = tail2;
head1->prev = &qdest->end;
tail2->next = &qdest->end;
if (qdest->ic)
queue_idempotent_callback(qdest->ic);
}
qdest->total_size = total_size;
}
/* ----------------------------------------------------------------------
* Low-level functions for the packet structures themselves.
*/
static void ssh_pkt_BinarySink_write(BinarySink *bs,
const void *data, size_t len);
PktOut *ssh_new_packet(void)
{
PktOut *pkt = snew(PktOut);
BinarySink_INIT(pkt, ssh_pkt_BinarySink_write);
pkt->data = NULL;
pkt->length = 0;
pkt->maxlen = 0;
pkt->downstream_id = 0;
pkt->additional_log_text = NULL;
pkt->qnode.next = pkt->qnode.prev = NULL;
pkt->qnode.on_free_queue = false;
return pkt;
}
static void ssh_pkt_adddata(PktOut *pkt, const void *data, int len)
{
sgrowarrayn_nm(pkt->data, pkt->maxlen, pkt->length, len);
memcpy(pkt->data + pkt->length, data, len);
pkt->length += len;
pkt->qnode.formal_size = pkt->length;
}
static void ssh_pkt_BinarySink_write(BinarySink *bs,
const void *data, size_t len)
{
PktOut *pkt = BinarySink_DOWNCAST(bs, PktOut);
ssh_pkt_adddata(pkt, data, len);
}
void ssh_free_pktout(PktOut *pkt)
{
sfree(pkt->data);
sfree(pkt);
}
/* ----------------------------------------------------------------------
* Implement zombiechan_new() and its trivial vtable.
*/
static void zombiechan_free(Channel *chan);
static size_t zombiechan_send(
Channel *chan, bool is_stderr, const void *, size_t);
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 void zombiechan_set_input_wanted(Channel *chan, bool wanted);
static void zombiechan_do_nothing(Channel *chan);
static void zombiechan_open_failure(Channel *chan, const char *);
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 zombiechan_want_close(Channel *chan, bool sent_eof, bool rcvd_eof);
static char *zombiechan_log_close_msg(Channel *chan) { return NULL; }
static const ChannelVtable zombiechan_channelvt = {
.free = zombiechan_free,
.open_confirmation = zombiechan_do_nothing,
.open_failed = zombiechan_open_failure,
.send = zombiechan_send,
.send_eof = zombiechan_do_nothing,
.set_input_wanted = zombiechan_set_input_wanted,
.log_close_msg = zombiechan_log_close_msg,
.want_close = zombiechan_want_close,
.rcvd_exit_status = chan_no_exit_status,
.rcvd_exit_signal = chan_no_exit_signal,
.rcvd_exit_signal_numeric = chan_no_exit_signal_numeric,
.run_shell = chan_no_run_shell,
.run_command = chan_no_run_command,
.run_subsystem = chan_no_run_subsystem,
.enable_x11_forwarding = chan_no_enable_x11_forwarding,
.enable_agent_forwarding = chan_no_enable_agent_forwarding,
.allocate_pty = chan_no_allocate_pty,
.set_env = chan_no_set_env,
.send_break = chan_no_send_break,
.send_signal = chan_no_send_signal,
.change_window_size = chan_no_change_window_size,
.request_response = chan_no_request_response,
};
Channel *zombiechan_new(void)
{
Channel *chan = snew(Channel);
chan->vt = &zombiechan_channelvt;
chan->initial_fixed_window_size = 0;
return chan;
}
static void zombiechan_free(Channel *chan)
{
assert(chan->vt == &zombiechan_channelvt);
sfree(chan);
}
static void zombiechan_do_nothing(Channel *chan)
{
assert(chan->vt == &zombiechan_channelvt);
}
static void zombiechan_open_failure(Channel *chan, const char *errtext)
{
assert(chan->vt == &zombiechan_channelvt);
}
static size_t zombiechan_send(Channel *chan, bool is_stderr,
const void *data, size_t length)
{
assert(chan->vt == &zombiechan_channelvt);
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 19:23:19 +00:00
static void zombiechan_set_input_wanted(Channel *chan, bool enable)
{
assert(chan->vt == &zombiechan_channelvt);
}
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 zombiechan_want_close(Channel *chan, bool sent_eof, bool rcvd_eof)
{
return true;
}
/* ----------------------------------------------------------------------
* Common routines for handling SSH tty modes.
*/
static unsigned real_ttymode_opcode(unsigned our_opcode, int ssh_version)
{
switch (our_opcode) {
case TTYMODE_ISPEED:
return ssh_version == 1 ? TTYMODE_ISPEED_SSH1 : TTYMODE_ISPEED_SSH2;
case TTYMODE_OSPEED:
return ssh_version == 1 ? TTYMODE_OSPEED_SSH1 : TTYMODE_OSPEED_SSH2;
default:
return our_opcode;
}
}
static unsigned our_ttymode_opcode(unsigned real_opcode, int ssh_version)
{
if (ssh_version == 1) {
switch (real_opcode) {
case TTYMODE_ISPEED_SSH1:
return TTYMODE_ISPEED;
case TTYMODE_OSPEED_SSH1:
return TTYMODE_OSPEED;
default:
return real_opcode;
}
} else {
switch (real_opcode) {
case TTYMODE_ISPEED_SSH2:
return TTYMODE_ISPEED;
case TTYMODE_OSPEED_SSH2:
return TTYMODE_OSPEED;
default:
return real_opcode;
}
}
}
struct ssh_ttymodes get_ttymodes_from_conf(Seat *seat, Conf *conf)
{
struct ssh_ttymodes modes;
size_t i;
static const struct mode_name_type {
const char *mode;
int opcode;
enum { TYPE_CHAR, TYPE_BOOL } type;
} modes_names_types[] = {
#define TTYMODE_CHAR(name, val, index) { #name, val, TYPE_CHAR },
#define TTYMODE_FLAG(name, val, field, mask) { #name, val, TYPE_BOOL },
#include "ttymode-list.h"
#undef TTYMODE_CHAR
#undef TTYMODE_FLAG
};
memset(&modes, 0, sizeof(modes));
for (i = 0; i < lenof(modes_names_types); i++) {
const struct mode_name_type *mode = &modes_names_types[i];
const char *sval = conf_get_str_str(conf, CONF_ttymodes, mode->mode);
char *to_free = NULL;
if (!sval)
sval = "N"; /* just in case */
/*
* sval[0] can be
* - 'V', indicating that an explicit value follows it;
* - 'A', indicating that we should pass the value through from
* the local environment via get_ttymode; or
* - 'N', indicating that we should explicitly not send this
* mode.
*/
if (sval[0] == 'A') {
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 18:58:42 +00:00
sval = to_free = seat_get_ttymode(seat, mode->mode);
} else if (sval[0] == 'V') {
sval++; /* skip the 'V' */
} else {
/* else 'N', or something from the future we don't understand */
continue;
}
if (sval) {
/*
* Parse the string representation of the tty mode
* into the integer value it will take on the wire.
*/
unsigned ival = 0;
switch (mode->type) {
case TYPE_CHAR:
if (*sval) {
char *next = NULL;
/* We know ctrlparse won't write to the string, so
* casting away const is ugly but allowable. */
ival = ctrlparse((char *)sval, &next);
if (!next)
ival = sval[0];
} else {
ival = 255; /* special value meaning "don't set" */
}
break;
case TYPE_BOOL:
if (stricmp(sval, "yes") == 0 ||
stricmp(sval, "on") == 0 ||
stricmp(sval, "true") == 0 ||
stricmp(sval, "+") == 0)
ival = 1; /* true */
else if (stricmp(sval, "no") == 0 ||
stricmp(sval, "off") == 0 ||
stricmp(sval, "false") == 0 ||
stricmp(sval, "-") == 0)
ival = 0; /* false */
else
ival = (atoi(sval) != 0);
break;
default:
unreachable("Bad mode->type");
}
modes.have_mode[mode->opcode] = true;
modes.mode_val[mode->opcode] = ival;
}
sfree(to_free);
}
{
unsigned ospeed, ispeed;
/* Unpick the terminal-speed config string. */
ospeed = ispeed = 38400; /* last-resort defaults */
sscanf(conf_get_str(conf, CONF_termspeed), "%u,%u", &ospeed, &ispeed);
/* Currently we unconditionally set these */
modes.have_mode[TTYMODE_ISPEED] = true;
modes.mode_val[TTYMODE_ISPEED] = ispeed;
modes.have_mode[TTYMODE_OSPEED] = true;
modes.mode_val[TTYMODE_OSPEED] = ospeed;
}
return modes;
}
struct ssh_ttymodes read_ttymodes_from_packet(
BinarySource *bs, int ssh_version)
{
struct ssh_ttymodes modes;
memset(&modes, 0, sizeof(modes));
while (1) {
unsigned real_opcode, our_opcode;
real_opcode = get_byte(bs);
if (real_opcode == TTYMODE_END_OF_LIST)
break;
if (real_opcode >= 160) {
/*
* RFC 4254 (and the SSH 1.5 spec): "Opcodes 160 to 255
* are not yet defined, and cause parsing to stop (they
* should only be used after any other data)."
*
* My interpretation of this is that if one of these
* opcodes appears, it's not a parse _error_, but it is
* something that we don't know how to parse even well
* enough to step over it to find the next opcode, so we
* stop parsing now and assume that the rest of the string
* is composed entirely of things we don't understand and
* (as usual for unsupported terminal modes) silently
* ignore.
*/
return modes;
}
our_opcode = our_ttymode_opcode(real_opcode, ssh_version);
assert(our_opcode < TTYMODE_LIMIT);
modes.have_mode[our_opcode] = true;
if (ssh_version == 1 && real_opcode >= 1 && real_opcode <= 127)
modes.mode_val[our_opcode] = get_byte(bs);
else
modes.mode_val[our_opcode] = get_uint32(bs);
}
return modes;
}
void write_ttymodes_to_packet(BinarySink *bs, int ssh_version,
struct ssh_ttymodes modes)
{
unsigned i;
for (i = 0; i < TTYMODE_LIMIT; i++) {
if (modes.have_mode[i]) {
unsigned val = modes.mode_val[i];
unsigned opcode = real_ttymode_opcode(i, ssh_version);
put_byte(bs, opcode);
if (ssh_version == 1 && opcode >= 1 && opcode <= 127)
put_byte(bs, val);
else
put_uint32(bs, val);
}
}
put_byte(bs, TTYMODE_END_OF_LIST);
}
/* ----------------------------------------------------------------------
* Routine for allocating a new channel ID, given a means of finding
* the index field in a given channel structure.
*/
unsigned alloc_channel_id_general(tree234 *channels, size_t localid_offset)
{
const unsigned CHANNEL_NUMBER_OFFSET = 256;
search234_state ss;
/*
* First-fit allocation of channel numbers: we always pick the
* lowest unused one.
*
* Every channel before that, and no channel after it, has an ID
* exactly equal to its tree index plus CHANNEL_NUMBER_OFFSET. So
* we can use the search234 system to identify the length of that
* initial sequence, in a single log-time pass down the channels
* tree.
*/
search234_start(&ss, channels);
while (ss.element) {
unsigned localid = *(unsigned *)((char *)ss.element + localid_offset);
if (localid == ss.index + CHANNEL_NUMBER_OFFSET)
search234_step(&ss, +1);
else
search234_step(&ss, -1);
}
/*
* Now ss.index gives exactly the number of channels in that
* initial sequence. So adding CHANNEL_NUMBER_OFFSET to it must
* give precisely the lowest unused channel number.
*/
return ss.index + CHANNEL_NUMBER_OFFSET;
}
/* ----------------------------------------------------------------------
* Functions for handling the comma-separated strings used to store
* lists of protocol identifiers in SSH-2.
*/
void add_to_commasep_pl(strbuf *buf, ptrlen data)
{
if (buf->len > 0)
put_byte(buf, ',');
put_datapl(buf, data);
}
void add_to_commasep(strbuf *buf, const char *data)
{
add_to_commasep_pl(buf, ptrlen_from_asciz(data));
}
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 get_commasep_word(ptrlen *list, ptrlen *word)
{
const char *comma;
/*
* Discard empty list elements, should there be any, because we
* never want to return one as if it was a real string. (This
* introduces a mild tolerance of badly formatted data in lists we
* receive, but I think that's acceptable.)
*/
while (list->len > 0 && *(const char *)list->ptr == ',') {
list->ptr = (const char *)list->ptr + 1;
list->len--;
}
if (!list->len)
return false;
comma = memchr(list->ptr, ',', list->len);
if (!comma) {
*word = *list;
list->len = 0;
} else {
size_t wordlen = comma - (const char *)list->ptr;
word->ptr = list->ptr;
word->len = wordlen;
list->ptr = (const char *)list->ptr + wordlen + 1;
list->len -= wordlen + 1;
}
return true;
}
/* ----------------------------------------------------------------------
* Functions for translating SSH packet type codes into their symbolic
* string names.
*/
#define TRANSLATE_UNIVERSAL(y, name, value) \
if (type == value) return #name;
#define TRANSLATE_KEX(y, name, value, ctx) \
if (type == value && pkt_kctx == ctx) return #name;
#define TRANSLATE_AUTH(y, name, value, ctx) \
if (type == value && pkt_actx == ctx) return #name;
const char *ssh1_pkt_type(int type)
{
SSH1_MESSAGE_TYPES(TRANSLATE_UNIVERSAL, y);
return "unknown";
}
const char *ssh2_pkt_type(Pkt_KCtx pkt_kctx, Pkt_ACtx pkt_actx, int type)
{
SSH2_MESSAGE_TYPES(TRANSLATE_UNIVERSAL, TRANSLATE_KEX, TRANSLATE_AUTH, y);
return "unknown";
}
#undef TRANSLATE_UNIVERSAL
#undef TRANSLATE_KEX
#undef TRANSLATE_AUTH
Move most of ssh.c out into separate source files. I've tried to separate out as many individually coherent changes from this work as I could into their own commits, but here's where I run out and have to commit the rest of this major refactoring as a big-bang change. Most of ssh.c is now no longer in ssh.c: all five of the main coroutines that handle layers of the SSH-1 and SSH-2 protocols now each have their own source file to live in, and a lot of the supporting functions have moved into the appropriate one of those too. The new abstraction is a vtable called 'PacketProtocolLayer', which has an input and output packet queue. Each layer's main coroutine is invoked from the method ssh_ppl_process_queue(), which is usually (though not exclusively) triggered automatically when things are pushed on the input queue. In SSH-2, the base layer is the transport protocol, and it contains a pair of subsidiary queues by which it passes some of its packets to the higher SSH-2 layers - first userauth and then connection, which are peers at the same level, with the former abdicating in favour of the latter at the appropriate moment. SSH-1 is simpler: the whole login phase of the protocol (crypto setup and authentication) is all in one module, and since SSH-1 has no repeat key exchange, that setup layer abdicates in favour of the connection phase when it's done. ssh.c itself is now about a tenth of its old size (which all by itself is cause for celebration!). Its main job is to set up all the layers, hook them up to each other and to the BPP, and to funnel data back and forth between that collection of modules and external things such as the network and the terminal. Once it's set up a collection of packet protocol layers, it communicates with them partly by calling methods of the base layer (and if that's ssh2transport then it will delegate some functionality to the corresponding methods of its higher layer), and partly by talking directly to the connection layer no matter where it is in the stack by means of the separate ConnectionLayer vtable which I introduced in commit 8001dd4cb, and to which I've now added quite a few extra methods replacing services that used to be internal function calls within ssh.c. (One effect of this is that the SSH-1 and SSH-2 channel storage is now no longer shared - there are distinct struct types ssh1_channel and ssh2_channel. That means a bit more code duplication, but on the plus side, a lot fewer confusing conditionals in the middle of half-shared functions, and less risk of a piece of SSH-1 escaping into SSH-2 or vice versa, which I remember has happened at least once in the past.) The bulk of this commit introduces the five new source files, their common header sshppl.h and some shared supporting routines in sshcommon.c, and rewrites nearly all of ssh.c itself. But it also includes a couple of other changes that I couldn't separate easily enough: Firstly, there's a new handling for socket EOF, in which ssh.c sets an 'input_eof' flag in the BPP, and that responds by checking a flag that tells it whether to report the EOF as an error or not. (This is the main reason for those new BPP_READ / BPP_WAITFOR macros - they can check the EOF flag every time the coroutine is resumed.) Secondly, the error reporting itself is changed around again. I'd expected to put some data fields in the public PacketProtocolLayer structure that it could set to report errors in the same way as the BPPs have been doing, but in the end, I decided propagating all those data fields around was a pain and that even the BPPs shouldn't have been doing it that way. So I've reverted to a system where everything calls back to functions in ssh.c itself to report any connection- ending condition. But there's a new family of those functions, categorising the possible such conditions by semantics, and each one has a different set of detailed effects (e.g. how rudely to close the network connection, what exit status should be passed back to the whole application, whether to send a disconnect message and/or display a GUI error box). I don't expect this to be immediately perfect: of course, the code has been through a big upheaval, new bugs are expected, and I haven't been able to do a full job of testing (e.g. I haven't tested every auth or kex method). But I've checked that it _basically_ works - both SSH protocols, all the different kinds of forwarding channel, more than one auth method, Windows and Linux, connection sharing - and I think it's now at the point where the easiest way to find further bugs is to let it out into the wild and see what users can spot.
2018-09-24 17:28:16 +00:00
/* ----------------------------------------------------------------------
* Common helper function for clients and implementations of
* PacketProtocolLayer.
*/
void ssh_ppl_replace(PacketProtocolLayer *old, PacketProtocolLayer *new)
{
new->bpp = old->bpp;
ssh_ppl_setup_queues(new, old->in_pq, old->out_pq);
new->selfptr = old->selfptr;
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 18:58:42 +00:00
new->seat = old->seat;
Move most of ssh.c out into separate source files. I've tried to separate out as many individually coherent changes from this work as I could into their own commits, but here's where I run out and have to commit the rest of this major refactoring as a big-bang change. Most of ssh.c is now no longer in ssh.c: all five of the main coroutines that handle layers of the SSH-1 and SSH-2 protocols now each have their own source file to live in, and a lot of the supporting functions have moved into the appropriate one of those too. The new abstraction is a vtable called 'PacketProtocolLayer', which has an input and output packet queue. Each layer's main coroutine is invoked from the method ssh_ppl_process_queue(), which is usually (though not exclusively) triggered automatically when things are pushed on the input queue. In SSH-2, the base layer is the transport protocol, and it contains a pair of subsidiary queues by which it passes some of its packets to the higher SSH-2 layers - first userauth and then connection, which are peers at the same level, with the former abdicating in favour of the latter at the appropriate moment. SSH-1 is simpler: the whole login phase of the protocol (crypto setup and authentication) is all in one module, and since SSH-1 has no repeat key exchange, that setup layer abdicates in favour of the connection phase when it's done. ssh.c itself is now about a tenth of its old size (which all by itself is cause for celebration!). Its main job is to set up all the layers, hook them up to each other and to the BPP, and to funnel data back and forth between that collection of modules and external things such as the network and the terminal. Once it's set up a collection of packet protocol layers, it communicates with them partly by calling methods of the base layer (and if that's ssh2transport then it will delegate some functionality to the corresponding methods of its higher layer), and partly by talking directly to the connection layer no matter where it is in the stack by means of the separate ConnectionLayer vtable which I introduced in commit 8001dd4cb, and to which I've now added quite a few extra methods replacing services that used to be internal function calls within ssh.c. (One effect of this is that the SSH-1 and SSH-2 channel storage is now no longer shared - there are distinct struct types ssh1_channel and ssh2_channel. That means a bit more code duplication, but on the plus side, a lot fewer confusing conditionals in the middle of half-shared functions, and less risk of a piece of SSH-1 escaping into SSH-2 or vice versa, which I remember has happened at least once in the past.) The bulk of this commit introduces the five new source files, their common header sshppl.h and some shared supporting routines in sshcommon.c, and rewrites nearly all of ssh.c itself. But it also includes a couple of other changes that I couldn't separate easily enough: Firstly, there's a new handling for socket EOF, in which ssh.c sets an 'input_eof' flag in the BPP, and that responds by checking a flag that tells it whether to report the EOF as an error or not. (This is the main reason for those new BPP_READ / BPP_WAITFOR macros - they can check the EOF flag every time the coroutine is resumed.) Secondly, the error reporting itself is changed around again. I'd expected to put some data fields in the public PacketProtocolLayer structure that it could set to report errors in the same way as the BPPs have been doing, but in the end, I decided propagating all those data fields around was a pain and that even the BPPs shouldn't have been doing it that way. So I've reverted to a system where everything calls back to functions in ssh.c itself to report any connection- ending condition. But there's a new family of those functions, categorising the possible such conditions by semantics, and each one has a different set of detailed effects (e.g. how rudely to close the network connection, what exit status should be passed back to the whole application, whether to send a disconnect message and/or display a GUI error box). I don't expect this to be immediately perfect: of course, the code has been through a big upheaval, new bugs are expected, and I haven't been able to do a full job of testing (e.g. I haven't tested every auth or kex method). But I've checked that it _basically_ works - both SSH protocols, all the different kinds of forwarding channel, more than one auth method, Windows and Linux, connection sharing - and I think it's now at the point where the easiest way to find further bugs is to let it out into the wild and see what users can spot.
2018-09-24 17:28:16 +00:00
new->ssh = old->ssh;
*new->selfptr = new;
ssh_ppl_free(old);
/* The new layer might need to be the first one that sends a
* packet, so trigger a call to its main coroutine immediately. If
* it doesn't need to go first, the worst that will do is return
* straight away. */
queue_idempotent_callback(&new->ic_process_queue);
}
void ssh_ppl_free(PacketProtocolLayer *ppl)
{
delete_callbacks_for_context(ppl);
ppl->vt->free(ppl);
}
static void ssh_ppl_ic_process_queue_callback(void *context)
{
PacketProtocolLayer *ppl = (PacketProtocolLayer *)context;
ssh_ppl_process_queue(ppl);
}
void ssh_ppl_setup_queues(PacketProtocolLayer *ppl,
PktInQueue *inq, PktOutQueue *outq)
{
ppl->in_pq = inq;
ppl->out_pq = outq;
ppl->in_pq->pqb.ic = &ppl->ic_process_queue;
ppl->ic_process_queue.fn = ssh_ppl_ic_process_queue_callback;
ppl->ic_process_queue.ctx = ppl;
/* If there's already something on the input queue, it will want
* handling immediately. */
if (pq_peek(ppl->in_pq))
queue_idempotent_callback(&ppl->ic_process_queue);
}
void ssh_ppl_user_output_string_and_free(PacketProtocolLayer *ppl, char *text)
{
/* Messages sent via this function are from the SSH layer, not
* from the server-side process, so they always have the stderr
* flag set. */
seat_stderr_pl(ppl->seat, ptrlen_from_asciz(text));
Move most of ssh.c out into separate source files. I've tried to separate out as many individually coherent changes from this work as I could into their own commits, but here's where I run out and have to commit the rest of this major refactoring as a big-bang change. Most of ssh.c is now no longer in ssh.c: all five of the main coroutines that handle layers of the SSH-1 and SSH-2 protocols now each have their own source file to live in, and a lot of the supporting functions have moved into the appropriate one of those too. The new abstraction is a vtable called 'PacketProtocolLayer', which has an input and output packet queue. Each layer's main coroutine is invoked from the method ssh_ppl_process_queue(), which is usually (though not exclusively) triggered automatically when things are pushed on the input queue. In SSH-2, the base layer is the transport protocol, and it contains a pair of subsidiary queues by which it passes some of its packets to the higher SSH-2 layers - first userauth and then connection, which are peers at the same level, with the former abdicating in favour of the latter at the appropriate moment. SSH-1 is simpler: the whole login phase of the protocol (crypto setup and authentication) is all in one module, and since SSH-1 has no repeat key exchange, that setup layer abdicates in favour of the connection phase when it's done. ssh.c itself is now about a tenth of its old size (which all by itself is cause for celebration!). Its main job is to set up all the layers, hook them up to each other and to the BPP, and to funnel data back and forth between that collection of modules and external things such as the network and the terminal. Once it's set up a collection of packet protocol layers, it communicates with them partly by calling methods of the base layer (and if that's ssh2transport then it will delegate some functionality to the corresponding methods of its higher layer), and partly by talking directly to the connection layer no matter where it is in the stack by means of the separate ConnectionLayer vtable which I introduced in commit 8001dd4cb, and to which I've now added quite a few extra methods replacing services that used to be internal function calls within ssh.c. (One effect of this is that the SSH-1 and SSH-2 channel storage is now no longer shared - there are distinct struct types ssh1_channel and ssh2_channel. That means a bit more code duplication, but on the plus side, a lot fewer confusing conditionals in the middle of half-shared functions, and less risk of a piece of SSH-1 escaping into SSH-2 or vice versa, which I remember has happened at least once in the past.) The bulk of this commit introduces the five new source files, their common header sshppl.h and some shared supporting routines in sshcommon.c, and rewrites nearly all of ssh.c itself. But it also includes a couple of other changes that I couldn't separate easily enough: Firstly, there's a new handling for socket EOF, in which ssh.c sets an 'input_eof' flag in the BPP, and that responds by checking a flag that tells it whether to report the EOF as an error or not. (This is the main reason for those new BPP_READ / BPP_WAITFOR macros - they can check the EOF flag every time the coroutine is resumed.) Secondly, the error reporting itself is changed around again. I'd expected to put some data fields in the public PacketProtocolLayer structure that it could set to report errors in the same way as the BPPs have been doing, but in the end, I decided propagating all those data fields around was a pain and that even the BPPs shouldn't have been doing it that way. So I've reverted to a system where everything calls back to functions in ssh.c itself to report any connection- ending condition. But there's a new family of those functions, categorising the possible such conditions by semantics, and each one has a different set of detailed effects (e.g. how rudely to close the network connection, what exit status should be passed back to the whole application, whether to send a disconnect message and/or display a GUI error box). I don't expect this to be immediately perfect: of course, the code has been through a big upheaval, new bugs are expected, and I haven't been able to do a full job of testing (e.g. I haven't tested every auth or kex method). But I've checked that it _basically_ works - both SSH protocols, all the different kinds of forwarding channel, more than one auth method, Windows and Linux, connection sharing - and I think it's now at the point where the easiest way to find further bugs is to let it out into the wild and see what users can spot.
2018-09-24 17:28:16 +00:00
sfree(text);
}
Account for packet queues in ssh_sendbuffer(). Ever since I reworked the SSH code to have multiple internal packet queues, there's been a long-standing FIXME in ssh_sendbuffer() saying that we ought to include the data buffered in those queues as part of reporting how much data is buffered on standard input. Recently a user reported that 'proftpd', or rather its 'mod_sftp' add-on that implements an SFTP-only SSH server, exposes a bug related to that missing piece of code. The xfer_upload system in sftp.c starts by pushing SFTP write messages into the SSH code for as long as sftp_sendbuffer() (which ends up at ssh_sendbuffer()) reports that not too much data is buffered locally. In fact what happens is that all those messages end up on the packet queues between SSH protocol layers, so they're not counted by sftp_sendbuffer(), so we just keep going until there's some other reason to stop. Usually the reason we stop is because we've filled up the SFTP channel's SSH-layer window, so we need the server to send us a WINDOW_ADJUST before we're allowed to send any more data. So we return to the main event loop and start waiting for reply packets. And when the window is moderate (e.g. OpenSSH currently seems to present about 2MB), this isn't really noticeable. But proftpd presents the maximum-size window of 2^32-1 bytes, and as a result we just keep shovelling more and more packets into the internal packet queues until PSFTP has grown to 4GB in size, and only then do we even return to the event loop and start actually sending them down the network. Moreover, this happens again at rekey time, because while a rekey is in progress, ssh2transport stops emptying the queue of outgoing packets sent by its higher layer - so, again, everything just keeps buffering up somewhere that sftp_sendbuffer can't see it. But this commit fixes it! Each PacketProtocolLayer now provides a vtable method for asking how much data it currently has queued. Most of them share a default implementation which just returns the newly added total_size field from their pq_out; the exception is ssh2transport, which also has to account for data queued in its higher layer. And ssh_sendbuffer() adds that on to the quantity it already knew about in other locations, to give a more realistic idea of the currently buffered data.
2020-02-05 19:34:29 +00:00
size_t ssh_ppl_default_queued_data_size(PacketProtocolLayer *ppl)
{
return ppl->out_pq->pqb.total_size;
}
void ssh_ppl_default_final_output(PacketProtocolLayer *ppl)
{
}
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 10:57:21 +00:00
static void ssh_ppl_prompts_callback(void *ctx)
{
ssh_ppl_process_queue((PacketProtocolLayer *)ctx);
}
prompts_t *ssh_ppl_new_prompts(PacketProtocolLayer *ppl)
{
prompts_t *p = new_prompts();
p->callback = ssh_ppl_prompts_callback;
p->callback_ctx = ppl;
return p;
}
/* ----------------------------------------------------------------------
* Common helper functions for clients and implementations of
* BinaryPacketProtocol.
*/
static void ssh_bpp_input_raw_data_callback(void *context)
{
BinaryPacketProtocol *bpp = (BinaryPacketProtocol *)context;
Ssh *ssh = bpp->ssh; /* in case bpp is about to get freed */
ssh_bpp_handle_input(bpp);
/* If we've now cleared enough backlog on the input connection, we
* may need to unfreeze it. */
ssh_conn_processed_data(ssh);
}
static void ssh_bpp_output_packet_callback(void *context)
{
BinaryPacketProtocol *bpp = (BinaryPacketProtocol *)context;
ssh_bpp_handle_output(bpp);
}
void ssh_bpp_common_setup(BinaryPacketProtocol *bpp)
{
pq_in_init(&bpp->in_pq);
pq_out_init(&bpp->out_pq);
bpp->input_eof = false;
bpp->ic_in_raw.fn = ssh_bpp_input_raw_data_callback;
bpp->ic_in_raw.ctx = bpp;
bpp->ic_out_pq.fn = ssh_bpp_output_packet_callback;
bpp->ic_out_pq.ctx = bpp;
bpp->out_pq.pqb.ic = &bpp->ic_out_pq;
}
void ssh_bpp_free(BinaryPacketProtocol *bpp)
{
delete_callbacks_for_context(bpp);
bpp->vt->free(bpp);
}
void ssh2_bpp_queue_disconnect(BinaryPacketProtocol *bpp,
const char *msg, int category)
{
PktOut *pkt = ssh_bpp_new_pktout(bpp, SSH2_MSG_DISCONNECT);
put_uint32(pkt, category);
put_stringz(pkt, msg);
put_stringz(pkt, "en"); /* language tag */
pq_push(&bpp->out_pq, pkt);
}
Squash shift warnings in ssh2_bpp_check_unimplemented. I did a horrible thing with a list macro which builds up a 256-bit bitmap of known SSH-2 message types at compile time, by means of evaluating a conditional expression per known message type and per bitmap word which boils down to (in pseudocode) (shift count in range ? 1 << shift count : 0) I think this is perfectly valid C. If the shift count is out of range, then the use of the << operator in the true branch of the ?: would have undefined behaviour if it were executed - but that's OK, because in that situation, the safe false branch is executed instead. But when the whole thing is a compile-time evaluated constant expression, the compiler can prove statically that the << in the true branch is an out-of-range shift, and at least some compilers will warn about it verbosely. The same compiler *could* also prove statically that that branch isn't taken, and use that to suppress the warning - but at least clang does not. The solution is the same one I used in shift_right_by_one_word and shift_left_by_one_word in mpint.c: inside the true branch, nest a second conditional expression which coerces the shift count to always be in range, by setting it to 0 if it's not. This doesn't affect the output, because the only cases in which the output of the true branch is altered by this transformation are the ones in which the true branch wasn't taken anyway. So this change should make no difference to the output of this macro construction, but it suppresses about 350 pointless warnings from clang.
2021-09-14 10:16:49 +00:00
#define BITMAP_UNIVERSAL(y, name, value) \
| (value >= y && value < y+32 \
? 1UL << (value >= y && value < y+32 ? (value-y) : 0) \
: 0)
#define BITMAP_CONDITIONAL(y, name, value, ctx) \
BITMAP_UNIVERSAL(y, name, value)
#define SSH2_BITMAP_WORD(y) \
(0 SSH2_MESSAGE_TYPES(BITMAP_UNIVERSAL, BITMAP_CONDITIONAL, \
BITMAP_CONDITIONAL, (32*y)))
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 ssh2_bpp_check_unimplemented(BinaryPacketProtocol *bpp, PktIn *pktin)
{
static const unsigned valid_bitmap[] = {
SSH2_BITMAP_WORD(0),
SSH2_BITMAP_WORD(1),
SSH2_BITMAP_WORD(2),
SSH2_BITMAP_WORD(3),
SSH2_BITMAP_WORD(4),
SSH2_BITMAP_WORD(5),
SSH2_BITMAP_WORD(6),
SSH2_BITMAP_WORD(7),
};
if (pktin->type < 0x100 &&
!((valid_bitmap[pktin->type >> 5] >> (pktin->type & 0x1F)) & 1)) {
PktOut *pkt = ssh_bpp_new_pktout(bpp, SSH2_MSG_UNIMPLEMENTED);
put_uint32(pkt, pktin->sequence);
pq_push(&bpp->out_pq, pkt);
return true;
}
return false;
}
#undef BITMAP_UNIVERSAL
#undef BITMAP_CONDITIONAL
#undef SSH2_BITMAP_WORD
/* ----------------------------------------------------------------------
Reorganise host key checking and confirmation. Previously, checking the host key against the persistent cache managed by the storage.h API was done as part of the seat_verify_ssh_host_key method, i.e. separately by each Seat. Now that check is done by verify_ssh_host_key(), which is a new function in ssh/common.c that centralises all the parts of host key checking that don't need an interactive prompt. It subsumes the previous verify_ssh_manual_host_key() that checked against the Conf, and it does the check against the storage API that each Seat was previously doing separately. If it can't confirm or definitively reject the host key by itself, _then_ it calls out to the Seat, once an interactive prompt is definitely needed. The main point of doing this is so that when SshProxy forwards a Seat call from the proxy SSH connection to the primary Seat, it won't print an announcement of which connection is involved unless it's actually going to do something interactive. (Not that we're printing those announcements _yet_ anyway, but this is a piece of groundwork that works towards doing so.) But while I'm at it, I've also taken the opportunity to clean things up a bit by renaming functions sensibly. Previously we had three very similarly named functions verify_ssh_manual_host_key(), SeatVtable's 'verify_ssh_host_key' method, and verify_host_key() in storage.h. Now the Seat method is called 'confirm' rather than 'verify' (since its job is now always to print an interactive prompt, so it looks more like the other confirm_foo methods), and the storage.h function is called check_stored_host_key(), which goes better with store_host_key and avoids having too many functions with similar names. And the 'manual' function is subsumed into the new centralised code, so there's now just *one* host key function with 'verify' in the name. Several functions are reindented in this commit. Best viewed with whitespace changes ignored.
2021-10-25 17:12:17 +00:00
* Centralised component of SSH host key verification.
*
* verify_ssh_host_key is called from both the SSH-1 and SSH-2
* transport layers, and does the initial work of checking whether the
* host key is already known. If so, it returns success on its own
* account; otherwise, it calls out to the Seat to give an interactive
* prompt (the nature of which varies depending on the Seat itself).
*/
Richer data type for interactive prompt results. All the seat functions that request an interactive prompt of some kind to the user - both the main seat_get_userpass_input and the various confirmation dialogs for things like host keys - were using a simple int return value, with the general semantics of 0 = "fail", 1 = "proceed" (and in the case of seat_get_userpass_input, answers to the prompts were provided), and -1 = "request in progress, wait for a callback". In this commit I change all those functions' return types to a new struct called SeatPromptResult, whose primary field is an enum replacing those simple integer values. The main purpose is that the enum has not three but _four_ values: the "fail" result has been split into 'user abort' and 'software abort'. The distinction is that a user abort occurs as a result of an interactive UI action, such as the user clicking 'cancel' in a dialog box or hitting ^D or ^C at a terminal password prompt - and therefore, there's no need to display an error message telling the user that the interactive operation has failed, because the user already knows, because they _did_ it. 'Software abort' is from any other cause, where PuTTY is the first to know there was a problem, and has to tell the user. We already had this 'user abort' vs 'software abort' distinction in other parts of the code - the SSH backend has separate termination functions which protocol layers can call. But we assumed that any failure from an interactive prompt request fell into the 'user abort' category, which is not true. A couple of examples: if you configure a host key fingerprint in your saved session via the SSH > Host keys pane, and the server presents a host key that doesn't match it, then verify_ssh_host_key would report that the user had aborted the connection, and feel no need to tell the user what had gone wrong! Similarly, if a password provided on the command line was not accepted, then (after I fixed the semantics of that in the previous commit) the same wrong handling would occur. So now, those Seat prompt functions too can communicate whether the user or the software originated a connection abort. And in the latter case, we also provide an error message to present to the user. Result: in those two example cases (and others), error messages should no longer go missing. Implementation note: to avoid the hassle of having the error message in a SeatPromptResult being a dynamically allocated string (and hence, every recipient of one must always check whether it's non-NULL and free it on every exit path, plus being careful about copying the struct around), I've instead arranged that the structure contains a function pointer and a couple of parameters, so that the string form of the message can be constructed on demand. That way, the only users who need to free it are the ones who actually _asked_ for it in the first place, which is a much smaller set. (This is one of the rare occasions that I regret not having C++'s extra features available in this code base - a unique_ptr or shared_ptr to a string would have been just the thing here, and the compiler would have done all the hard work for me of remembering where to insert the frees!)
2021-12-28 17:52:00 +00:00
SeatPromptResult verify_ssh_host_key(
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 17:05:36 +00:00
InteractionReadySeat iseat, Conf *conf, const char *host, int port,
ssh_key *key, const char *keytype, char *keystr, const char *keydisp,
Allow manually confirming and caching certified keys. In the case where a server presents a host key signed by a different certificate from the one you've configured, it need not _always_ be evidence of wrongdoing. I can imagine situations in which two CAs cover overlapping sets of things, and you don't want to blanket-trust one of them, but you do want to connect to a specific host signed by that one. Accordingly, PuTTY's previous policy of unconditionally aborting the connection if certificate validation fails (which was always intended as a stopgap until I thought through what I wanted to replace it with) is now replaced by fallback handling: we present the host key fingerprint to the user and give them the option to accept and/or cache it based on the public key itself. This means that the certified key types have to have a representation in the host key cache. So I've assigned each one a type id, and generate the cache string itself by simply falling back to the base key. (Rationale for the latter: re-signing a public key with a different certificate doesn't change the _private_ key, or the set of valid signatures generated with it. So if you've been convinced for reasons other than the certificate that a particular private key is in the possession of $host, then proof of ownership of that private key should be enough to convince you you're talking to $host no matter what CA has signed the public half this week.) We now offer to receive a given certified host key type if _either_ we have at least one CA configured to trust that host, _or_ we have a certified key of that type cached. (So once you've decided manually that you trust a particular key, we can still receive that key and authenticate the host with it, even if you later delete the CA record that it didn't match anyway.) One change from normal (uncertified) host key handling is that for certified key types _all_ the host key prompts use the stronger language, with "WARNING - POTENTIAL SECURITY BREACH!" rather than the mild 'hmm, we haven't seen this host before'. Rationale: if you expected this CA key and got that one, it _could_ be a bold-as-brass MITM attempt in which someone hoped you'd accept their entire CA key. The mild wording is only for the case where we had no previous expectations _at all_ for the host to violate: not a CA _or_ a cached key.
2022-07-16 10:23:13 +00:00
char **fingerprints, int ca_count,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
Reorganise host key checking and confirmation. Previously, checking the host key against the persistent cache managed by the storage.h API was done as part of the seat_verify_ssh_host_key method, i.e. separately by each Seat. Now that check is done by verify_ssh_host_key(), which is a new function in ssh/common.c that centralises all the parts of host key checking that don't need an interactive prompt. It subsumes the previous verify_ssh_manual_host_key() that checked against the Conf, and it does the check against the storage API that each Seat was previously doing separately. If it can't confirm or definitively reject the host key by itself, _then_ it calls out to the Seat, once an interactive prompt is definitely needed. The main point of doing this is so that when SshProxy forwards a Seat call from the proxy SSH connection to the primary Seat, it won't print an announcement of which connection is involved unless it's actually going to do something interactive. (Not that we're printing those announcements _yet_ anyway, but this is a piece of groundwork that works towards doing so.) But while I'm at it, I've also taken the opportunity to clean things up a bit by renaming functions sensibly. Previously we had three very similarly named functions verify_ssh_manual_host_key(), SeatVtable's 'verify_ssh_host_key' method, and verify_host_key() in storage.h. Now the Seat method is called 'confirm' rather than 'verify' (since its job is now always to print an interactive prompt, so it looks more like the other confirm_foo methods), and the storage.h function is called check_stored_host_key(), which goes better with store_host_key and avoids having too many functions with similar names. And the 'manual' function is subsumed into the new centralised code, so there's now just *one* host key function with 'verify' in the name. Several functions are reindented in this commit. Best viewed with whitespace changes ignored.
2021-10-25 17:12:17 +00:00
/*
* First, check if the Conf includes a manual specification of the
* expected host key. If so, that completely supersedes everything
* else, including the normal host key cache _and_ including
* manual overrides: we return success or failure immediately,
* entirely based on whether the key matches the Conf.
*/
if (conf_get_str_nthstrkey(conf, CONF_ssh_manual_hostkeys, 0)) {
if (fingerprints) {
for (size_t i = 0; i < SSH_N_FPTYPES; i++) {
/*
* Each fingerprint string we've been given will have
* things like 'ssh-rsa 2048' at the front of it. Strip
* those off and narrow down to just the hash at the end
* of the string.
*/
const char *fingerprint = fingerprints[i];
if (!fingerprint)
continue;
const char *p = strrchr(fingerprint, ' ');
fingerprint = p ? p+1 : fingerprint;
if (conf_get_str_str_opt(conf, CONF_ssh_manual_hostkeys,
fingerprint))
Richer data type for interactive prompt results. All the seat functions that request an interactive prompt of some kind to the user - both the main seat_get_userpass_input and the various confirmation dialogs for things like host keys - were using a simple int return value, with the general semantics of 0 = "fail", 1 = "proceed" (and in the case of seat_get_userpass_input, answers to the prompts were provided), and -1 = "request in progress, wait for a callback". In this commit I change all those functions' return types to a new struct called SeatPromptResult, whose primary field is an enum replacing those simple integer values. The main purpose is that the enum has not three but _four_ values: the "fail" result has been split into 'user abort' and 'software abort'. The distinction is that a user abort occurs as a result of an interactive UI action, such as the user clicking 'cancel' in a dialog box or hitting ^D or ^C at a terminal password prompt - and therefore, there's no need to display an error message telling the user that the interactive operation has failed, because the user already knows, because they _did_ it. 'Software abort' is from any other cause, where PuTTY is the first to know there was a problem, and has to tell the user. We already had this 'user abort' vs 'software abort' distinction in other parts of the code - the SSH backend has separate termination functions which protocol layers can call. But we assumed that any failure from an interactive prompt request fell into the 'user abort' category, which is not true. A couple of examples: if you configure a host key fingerprint in your saved session via the SSH > Host keys pane, and the server presents a host key that doesn't match it, then verify_ssh_host_key would report that the user had aborted the connection, and feel no need to tell the user what had gone wrong! Similarly, if a password provided on the command line was not accepted, then (after I fixed the semantics of that in the previous commit) the same wrong handling would occur. So now, those Seat prompt functions too can communicate whether the user or the software originated a connection abort. And in the latter case, we also provide an error message to present to the user. Result: in those two example cases (and others), error messages should no longer go missing. Implementation note: to avoid the hassle of having the error message in a SeatPromptResult being a dynamically allocated string (and hence, every recipient of one must always check whether it's non-NULL and free it on every exit path, plus being careful about copying the struct around), I've instead arranged that the structure contains a function pointer and a couple of parameters, so that the string form of the message can be constructed on demand. That way, the only users who need to free it are the ones who actually _asked_ for it in the first place, which is a much smaller set. (This is one of the rare occasions that I regret not having C++'s extra features available in this code base - a unique_ptr or shared_ptr to a string would have been just the thing here, and the compiler would have done all the hard work for me of remembering where to insert the frees!)
2021-12-28 17:52:00 +00:00
return SPR_OK;
Reorganise host key checking and confirmation. Previously, checking the host key against the persistent cache managed by the storage.h API was done as part of the seat_verify_ssh_host_key method, i.e. separately by each Seat. Now that check is done by verify_ssh_host_key(), which is a new function in ssh/common.c that centralises all the parts of host key checking that don't need an interactive prompt. It subsumes the previous verify_ssh_manual_host_key() that checked against the Conf, and it does the check against the storage API that each Seat was previously doing separately. If it can't confirm or definitively reject the host key by itself, _then_ it calls out to the Seat, once an interactive prompt is definitely needed. The main point of doing this is so that when SshProxy forwards a Seat call from the proxy SSH connection to the primary Seat, it won't print an announcement of which connection is involved unless it's actually going to do something interactive. (Not that we're printing those announcements _yet_ anyway, but this is a piece of groundwork that works towards doing so.) But while I'm at it, I've also taken the opportunity to clean things up a bit by renaming functions sensibly. Previously we had three very similarly named functions verify_ssh_manual_host_key(), SeatVtable's 'verify_ssh_host_key' method, and verify_host_key() in storage.h. Now the Seat method is called 'confirm' rather than 'verify' (since its job is now always to print an interactive prompt, so it looks more like the other confirm_foo methods), and the storage.h function is called check_stored_host_key(), which goes better with store_host_key and avoids having too many functions with similar names. And the 'manual' function is subsumed into the new centralised code, so there's now just *one* host key function with 'verify' in the name. Several functions are reindented in this commit. Best viewed with whitespace changes ignored.
2021-10-25 17:12:17 +00:00
}
}
Reorganise host key checking and confirmation. Previously, checking the host key against the persistent cache managed by the storage.h API was done as part of the seat_verify_ssh_host_key method, i.e. separately by each Seat. Now that check is done by verify_ssh_host_key(), which is a new function in ssh/common.c that centralises all the parts of host key checking that don't need an interactive prompt. It subsumes the previous verify_ssh_manual_host_key() that checked against the Conf, and it does the check against the storage API that each Seat was previously doing separately. If it can't confirm or definitively reject the host key by itself, _then_ it calls out to the Seat, once an interactive prompt is definitely needed. The main point of doing this is so that when SshProxy forwards a Seat call from the proxy SSH connection to the primary Seat, it won't print an announcement of which connection is involved unless it's actually going to do something interactive. (Not that we're printing those announcements _yet_ anyway, but this is a piece of groundwork that works towards doing so.) But while I'm at it, I've also taken the opportunity to clean things up a bit by renaming functions sensibly. Previously we had three very similarly named functions verify_ssh_manual_host_key(), SeatVtable's 'verify_ssh_host_key' method, and verify_host_key() in storage.h. Now the Seat method is called 'confirm' rather than 'verify' (since its job is now always to print an interactive prompt, so it looks more like the other confirm_foo methods), and the storage.h function is called check_stored_host_key(), which goes better with store_host_key and avoids having too many functions with similar names. And the 'manual' function is subsumed into the new centralised code, so there's now just *one* host key function with 'verify' in the name. Several functions are reindented in this commit. Best viewed with whitespace changes ignored.
2021-10-25 17:12:17 +00:00
if (key) {
/*
Reorganise host key checking and confirmation. Previously, checking the host key against the persistent cache managed by the storage.h API was done as part of the seat_verify_ssh_host_key method, i.e. separately by each Seat. Now that check is done by verify_ssh_host_key(), which is a new function in ssh/common.c that centralises all the parts of host key checking that don't need an interactive prompt. It subsumes the previous verify_ssh_manual_host_key() that checked against the Conf, and it does the check against the storage API that each Seat was previously doing separately. If it can't confirm or definitively reject the host key by itself, _then_ it calls out to the Seat, once an interactive prompt is definitely needed. The main point of doing this is so that when SshProxy forwards a Seat call from the proxy SSH connection to the primary Seat, it won't print an announcement of which connection is involved unless it's actually going to do something interactive. (Not that we're printing those announcements _yet_ anyway, but this is a piece of groundwork that works towards doing so.) But while I'm at it, I've also taken the opportunity to clean things up a bit by renaming functions sensibly. Previously we had three very similarly named functions verify_ssh_manual_host_key(), SeatVtable's 'verify_ssh_host_key' method, and verify_host_key() in storage.h. Now the Seat method is called 'confirm' rather than 'verify' (since its job is now always to print an interactive prompt, so it looks more like the other confirm_foo methods), and the storage.h function is called check_stored_host_key(), which goes better with store_host_key and avoids having too many functions with similar names. And the 'manual' function is subsumed into the new centralised code, so there's now just *one* host key function with 'verify' in the name. Several functions are reindented in this commit. Best viewed with whitespace changes ignored.
2021-10-25 17:12:17 +00:00
* Construct the base64-encoded public key blob and see if
* that's listed.
*/
Reorganise host key checking and confirmation. Previously, checking the host key against the persistent cache managed by the storage.h API was done as part of the seat_verify_ssh_host_key method, i.e. separately by each Seat. Now that check is done by verify_ssh_host_key(), which is a new function in ssh/common.c that centralises all the parts of host key checking that don't need an interactive prompt. It subsumes the previous verify_ssh_manual_host_key() that checked against the Conf, and it does the check against the storage API that each Seat was previously doing separately. If it can't confirm or definitively reject the host key by itself, _then_ it calls out to the Seat, once an interactive prompt is definitely needed. The main point of doing this is so that when SshProxy forwards a Seat call from the proxy SSH connection to the primary Seat, it won't print an announcement of which connection is involved unless it's actually going to do something interactive. (Not that we're printing those announcements _yet_ anyway, but this is a piece of groundwork that works towards doing so.) But while I'm at it, I've also taken the opportunity to clean things up a bit by renaming functions sensibly. Previously we had three very similarly named functions verify_ssh_manual_host_key(), SeatVtable's 'verify_ssh_host_key' method, and verify_host_key() in storage.h. Now the Seat method is called 'confirm' rather than 'verify' (since its job is now always to print an interactive prompt, so it looks more like the other confirm_foo methods), and the storage.h function is called check_stored_host_key(), which goes better with store_host_key and avoids having too many functions with similar names. And the 'manual' function is subsumed into the new centralised code, so there's now just *one* host key function with 'verify' in the name. Several functions are reindented in this commit. Best viewed with whitespace changes ignored.
2021-10-25 17:12:17 +00:00
strbuf *binblob;
char *base64blob;
int atoms, i;
binblob = strbuf_new();
ssh_key_public_blob(key, BinarySink_UPCAST(binblob));
atoms = (binblob->len + 2) / 3;
base64blob = snewn(atoms * 4 + 1, char);
for (i = 0; i < atoms; i++)
base64_encode_atom(binblob->u + 3*i,
binblob->len - 3*i, base64blob + 4*i);
base64blob[atoms * 4] = '\0';
strbuf_free(binblob);
if (conf_get_str_str_opt(conf, CONF_ssh_manual_hostkeys,
Reorganise host key checking and confirmation. Previously, checking the host key against the persistent cache managed by the storage.h API was done as part of the seat_verify_ssh_host_key method, i.e. separately by each Seat. Now that check is done by verify_ssh_host_key(), which is a new function in ssh/common.c that centralises all the parts of host key checking that don't need an interactive prompt. It subsumes the previous verify_ssh_manual_host_key() that checked against the Conf, and it does the check against the storage API that each Seat was previously doing separately. If it can't confirm or definitively reject the host key by itself, _then_ it calls out to the Seat, once an interactive prompt is definitely needed. The main point of doing this is so that when SshProxy forwards a Seat call from the proxy SSH connection to the primary Seat, it won't print an announcement of which connection is involved unless it's actually going to do something interactive. (Not that we're printing those announcements _yet_ anyway, but this is a piece of groundwork that works towards doing so.) But while I'm at it, I've also taken the opportunity to clean things up a bit by renaming functions sensibly. Previously we had three very similarly named functions verify_ssh_manual_host_key(), SeatVtable's 'verify_ssh_host_key' method, and verify_host_key() in storage.h. Now the Seat method is called 'confirm' rather than 'verify' (since its job is now always to print an interactive prompt, so it looks more like the other confirm_foo methods), and the storage.h function is called check_stored_host_key(), which goes better with store_host_key and avoids having too many functions with similar names. And the 'manual' function is subsumed into the new centralised code, so there's now just *one* host key function with 'verify' in the name. Several functions are reindented in this commit. Best viewed with whitespace changes ignored.
2021-10-25 17:12:17 +00:00
base64blob)) {
sfree(base64blob);
Richer data type for interactive prompt results. All the seat functions that request an interactive prompt of some kind to the user - both the main seat_get_userpass_input and the various confirmation dialogs for things like host keys - were using a simple int return value, with the general semantics of 0 = "fail", 1 = "proceed" (and in the case of seat_get_userpass_input, answers to the prompts were provided), and -1 = "request in progress, wait for a callback". In this commit I change all those functions' return types to a new struct called SeatPromptResult, whose primary field is an enum replacing those simple integer values. The main purpose is that the enum has not three but _four_ values: the "fail" result has been split into 'user abort' and 'software abort'. The distinction is that a user abort occurs as a result of an interactive UI action, such as the user clicking 'cancel' in a dialog box or hitting ^D or ^C at a terminal password prompt - and therefore, there's no need to display an error message telling the user that the interactive operation has failed, because the user already knows, because they _did_ it. 'Software abort' is from any other cause, where PuTTY is the first to know there was a problem, and has to tell the user. We already had this 'user abort' vs 'software abort' distinction in other parts of the code - the SSH backend has separate termination functions which protocol layers can call. But we assumed that any failure from an interactive prompt request fell into the 'user abort' category, which is not true. A couple of examples: if you configure a host key fingerprint in your saved session via the SSH > Host keys pane, and the server presents a host key that doesn't match it, then verify_ssh_host_key would report that the user had aborted the connection, and feel no need to tell the user what had gone wrong! Similarly, if a password provided on the command line was not accepted, then (after I fixed the semantics of that in the previous commit) the same wrong handling would occur. So now, those Seat prompt functions too can communicate whether the user or the software originated a connection abort. And in the latter case, we also provide an error message to present to the user. Result: in those two example cases (and others), error messages should no longer go missing. Implementation note: to avoid the hassle of having the error message in a SeatPromptResult being a dynamically allocated string (and hence, every recipient of one must always check whether it's non-NULL and free it on every exit path, plus being careful about copying the struct around), I've instead arranged that the structure contains a function pointer and a couple of parameters, so that the string form of the message can be constructed on demand. That way, the only users who need to free it are the ones who actually _asked_ for it in the first place, which is a much smaller set. (This is one of the rare occasions that I regret not having C++'s extra features available in this code base - a unique_ptr or shared_ptr to a string would have been just the thing here, and the compiler would have done all the hard work for me of remembering where to insert the frees!)
2021-12-28 17:52:00 +00:00
return SPR_OK;
Reorganise host key checking and confirmation. Previously, checking the host key against the persistent cache managed by the storage.h API was done as part of the seat_verify_ssh_host_key method, i.e. separately by each Seat. Now that check is done by verify_ssh_host_key(), which is a new function in ssh/common.c that centralises all the parts of host key checking that don't need an interactive prompt. It subsumes the previous verify_ssh_manual_host_key() that checked against the Conf, and it does the check against the storage API that each Seat was previously doing separately. If it can't confirm or definitively reject the host key by itself, _then_ it calls out to the Seat, once an interactive prompt is definitely needed. The main point of doing this is so that when SshProxy forwards a Seat call from the proxy SSH connection to the primary Seat, it won't print an announcement of which connection is involved unless it's actually going to do something interactive. (Not that we're printing those announcements _yet_ anyway, but this is a piece of groundwork that works towards doing so.) But while I'm at it, I've also taken the opportunity to clean things up a bit by renaming functions sensibly. Previously we had three very similarly named functions verify_ssh_manual_host_key(), SeatVtable's 'verify_ssh_host_key' method, and verify_host_key() in storage.h. Now the Seat method is called 'confirm' rather than 'verify' (since its job is now always to print an interactive prompt, so it looks more like the other confirm_foo methods), and the storage.h function is called check_stored_host_key(), which goes better with store_host_key and avoids having too many functions with similar names. And the 'manual' function is subsumed into the new centralised code, so there's now just *one* host key function with 'verify' in the name. Several functions are reindented in this commit. Best viewed with whitespace changes ignored.
2021-10-25 17:12:17 +00:00
}
sfree(base64blob);
}
Reorganise host key checking and confirmation. Previously, checking the host key against the persistent cache managed by the storage.h API was done as part of the seat_verify_ssh_host_key method, i.e. separately by each Seat. Now that check is done by verify_ssh_host_key(), which is a new function in ssh/common.c that centralises all the parts of host key checking that don't need an interactive prompt. It subsumes the previous verify_ssh_manual_host_key() that checked against the Conf, and it does the check against the storage API that each Seat was previously doing separately. If it can't confirm or definitively reject the host key by itself, _then_ it calls out to the Seat, once an interactive prompt is definitely needed. The main point of doing this is so that when SshProxy forwards a Seat call from the proxy SSH connection to the primary Seat, it won't print an announcement of which connection is involved unless it's actually going to do something interactive. (Not that we're printing those announcements _yet_ anyway, but this is a piece of groundwork that works towards doing so.) But while I'm at it, I've also taken the opportunity to clean things up a bit by renaming functions sensibly. Previously we had three very similarly named functions verify_ssh_manual_host_key(), SeatVtable's 'verify_ssh_host_key' method, and verify_host_key() in storage.h. Now the Seat method is called 'confirm' rather than 'verify' (since its job is now always to print an interactive prompt, so it looks more like the other confirm_foo methods), and the storage.h function is called check_stored_host_key(), which goes better with store_host_key and avoids having too many functions with similar names. And the 'manual' function is subsumed into the new centralised code, so there's now just *one* host key function with 'verify' in the name. Several functions are reindented in this commit. Best viewed with whitespace changes ignored.
2021-10-25 17:12:17 +00:00
Richer data type for interactive prompt results. All the seat functions that request an interactive prompt of some kind to the user - both the main seat_get_userpass_input and the various confirmation dialogs for things like host keys - were using a simple int return value, with the general semantics of 0 = "fail", 1 = "proceed" (and in the case of seat_get_userpass_input, answers to the prompts were provided), and -1 = "request in progress, wait for a callback". In this commit I change all those functions' return types to a new struct called SeatPromptResult, whose primary field is an enum replacing those simple integer values. The main purpose is that the enum has not three but _four_ values: the "fail" result has been split into 'user abort' and 'software abort'. The distinction is that a user abort occurs as a result of an interactive UI action, such as the user clicking 'cancel' in a dialog box or hitting ^D or ^C at a terminal password prompt - and therefore, there's no need to display an error message telling the user that the interactive operation has failed, because the user already knows, because they _did_ it. 'Software abort' is from any other cause, where PuTTY is the first to know there was a problem, and has to tell the user. We already had this 'user abort' vs 'software abort' distinction in other parts of the code - the SSH backend has separate termination functions which protocol layers can call. But we assumed that any failure from an interactive prompt request fell into the 'user abort' category, which is not true. A couple of examples: if you configure a host key fingerprint in your saved session via the SSH > Host keys pane, and the server presents a host key that doesn't match it, then verify_ssh_host_key would report that the user had aborted the connection, and feel no need to tell the user what had gone wrong! Similarly, if a password provided on the command line was not accepted, then (after I fixed the semantics of that in the previous commit) the same wrong handling would occur. So now, those Seat prompt functions too can communicate whether the user or the software originated a connection abort. And in the latter case, we also provide an error message to present to the user. Result: in those two example cases (and others), error messages should no longer go missing. Implementation note: to avoid the hassle of having the error message in a SeatPromptResult being a dynamically allocated string (and hence, every recipient of one must always check whether it's non-NULL and free it on every exit path, plus being careful about copying the struct around), I've instead arranged that the structure contains a function pointer and a couple of parameters, so that the string form of the message can be constructed on demand. That way, the only users who need to free it are the ones who actually _asked_ for it in the first place, which is a much smaller set. (This is one of the rare occasions that I regret not having C++'s extra features available in this code base - a unique_ptr or shared_ptr to a string would have been just the thing here, and the compiler would have done all the hard work for me of remembering where to insert the frees!)
2021-12-28 17:52:00 +00:00
return SPR_SW_ABORT("Host key not in manually configured list");
}
Reorganise host key checking and confirmation. Previously, checking the host key against the persistent cache managed by the storage.h API was done as part of the seat_verify_ssh_host_key method, i.e. separately by each Seat. Now that check is done by verify_ssh_host_key(), which is a new function in ssh/common.c that centralises all the parts of host key checking that don't need an interactive prompt. It subsumes the previous verify_ssh_manual_host_key() that checked against the Conf, and it does the check against the storage API that each Seat was previously doing separately. If it can't confirm or definitively reject the host key by itself, _then_ it calls out to the Seat, once an interactive prompt is definitely needed. The main point of doing this is so that when SshProxy forwards a Seat call from the proxy SSH connection to the primary Seat, it won't print an announcement of which connection is involved unless it's actually going to do something interactive. (Not that we're printing those announcements _yet_ anyway, but this is a piece of groundwork that works towards doing so.) But while I'm at it, I've also taken the opportunity to clean things up a bit by renaming functions sensibly. Previously we had three very similarly named functions verify_ssh_manual_host_key(), SeatVtable's 'verify_ssh_host_key' method, and verify_host_key() in storage.h. Now the Seat method is called 'confirm' rather than 'verify' (since its job is now always to print an interactive prompt, so it looks more like the other confirm_foo methods), and the storage.h function is called check_stored_host_key(), which goes better with store_host_key and avoids having too many functions with similar names. And the 'manual' function is subsumed into the new centralised code, so there's now just *one* host key function with 'verify' in the name. Several functions are reindented in this commit. Best viewed with whitespace changes ignored.
2021-10-25 17:12:17 +00:00
/*
* Next, check the host key cache.
*/
int storage_status = check_stored_host_key(host, port, keytype, keystr);
if (storage_status == 0) /* matching key was found in the cache */
Richer data type for interactive prompt results. All the seat functions that request an interactive prompt of some kind to the user - both the main seat_get_userpass_input and the various confirmation dialogs for things like host keys - were using a simple int return value, with the general semantics of 0 = "fail", 1 = "proceed" (and in the case of seat_get_userpass_input, answers to the prompts were provided), and -1 = "request in progress, wait for a callback". In this commit I change all those functions' return types to a new struct called SeatPromptResult, whose primary field is an enum replacing those simple integer values. The main purpose is that the enum has not three but _four_ values: the "fail" result has been split into 'user abort' and 'software abort'. The distinction is that a user abort occurs as a result of an interactive UI action, such as the user clicking 'cancel' in a dialog box or hitting ^D or ^C at a terminal password prompt - and therefore, there's no need to display an error message telling the user that the interactive operation has failed, because the user already knows, because they _did_ it. 'Software abort' is from any other cause, where PuTTY is the first to know there was a problem, and has to tell the user. We already had this 'user abort' vs 'software abort' distinction in other parts of the code - the SSH backend has separate termination functions which protocol layers can call. But we assumed that any failure from an interactive prompt request fell into the 'user abort' category, which is not true. A couple of examples: if you configure a host key fingerprint in your saved session via the SSH > Host keys pane, and the server presents a host key that doesn't match it, then verify_ssh_host_key would report that the user had aborted the connection, and feel no need to tell the user what had gone wrong! Similarly, if a password provided on the command line was not accepted, then (after I fixed the semantics of that in the previous commit) the same wrong handling would occur. So now, those Seat prompt functions too can communicate whether the user or the software originated a connection abort. And in the latter case, we also provide an error message to present to the user. Result: in those two example cases (and others), error messages should no longer go missing. Implementation note: to avoid the hassle of having the error message in a SeatPromptResult being a dynamically allocated string (and hence, every recipient of one must always check whether it's non-NULL and free it on every exit path, plus being careful about copying the struct around), I've instead arranged that the structure contains a function pointer and a couple of parameters, so that the string form of the message can be constructed on demand. That way, the only users who need to free it are the ones who actually _asked_ for it in the first place, which is a much smaller set. (This is one of the rare occasions that I regret not having C++'s extra features available in this code base - a unique_ptr or shared_ptr to a string would have been just the thing here, and the compiler would have done all the hard work for me of remembering where to insert the frees!)
2021-12-28 17:52:00 +00:00
return SPR_OK;
Reorganise host key checking and confirmation. Previously, checking the host key against the persistent cache managed by the storage.h API was done as part of the seat_verify_ssh_host_key method, i.e. separately by each Seat. Now that check is done by verify_ssh_host_key(), which is a new function in ssh/common.c that centralises all the parts of host key checking that don't need an interactive prompt. It subsumes the previous verify_ssh_manual_host_key() that checked against the Conf, and it does the check against the storage API that each Seat was previously doing separately. If it can't confirm or definitively reject the host key by itself, _then_ it calls out to the Seat, once an interactive prompt is definitely needed. The main point of doing this is so that when SshProxy forwards a Seat call from the proxy SSH connection to the primary Seat, it won't print an announcement of which connection is involved unless it's actually going to do something interactive. (Not that we're printing those announcements _yet_ anyway, but this is a piece of groundwork that works towards doing so.) But while I'm at it, I've also taken the opportunity to clean things up a bit by renaming functions sensibly. Previously we had three very similarly named functions verify_ssh_manual_host_key(), SeatVtable's 'verify_ssh_host_key' method, and verify_host_key() in storage.h. Now the Seat method is called 'confirm' rather than 'verify' (since its job is now always to print an interactive prompt, so it looks more like the other confirm_foo methods), and the storage.h function is called check_stored_host_key(), which goes better with store_host_key and avoids having too many functions with similar names. And the 'manual' function is subsumed into the new centralised code, so there's now just *one* host key function with 'verify' in the name. Several functions are reindented in this commit. Best viewed with whitespace changes ignored.
2021-10-25 17:12:17 +00:00
/*
* The key is either missing from the cache, or does not match.
* Either way, fall back to an interactive prompt from the Seat.
*/
Centralise most details of host-key prompting. The text of the host key warnings was replicated in three places: the Windows rc file, the GTK dialog setup function, and the console.c shared between both platforms' CLI tools. Now it lives in just one place, namely ssh/common.c where the rest of the centralised host-key checking is done, so it'll be easier to adjust the wording in future. This comes with some extra automation. Paragraph wrapping is no longer done by hand in any version of these prompts. (Previously we let GTK do the wrapping on GTK, but on Windows the resource file contained a bunch of pre-wrapped LTEXT lines, and console.c had pre-wrapped terminal messages.) And the dialog heights in Windows are determined automatically based on the amount of stuff in the window. The main idea of all this is that it'll be easier to set up more elaborate kinds of host key prompt that deal with certificates (if, e.g., a server sends us a certified host key which we don't trust the CA for). But there are side benefits of this refactoring too: each tool now reliably inserts its own appname in the prompts, and also, on Windows the entire prompt text is copy-pastable. Details of implementation: there's a new type SeatDialogText which holds a set of (type, string) pairs describing the contents of a prompt. Type codes distinguish ordinary text paragraphs, paragraphs to be displayed prominently (like key fingerprints), the extra-bold scary title at the top of the 'host key changed' version of the dialog, and the various information that lives in the subsidiary 'more info' box. ssh/common.c constructs this, and passes it to the Seat to present the actual prompt. In order to deal with the different UI for answering the prompt, I've added an extra Seat method 'prompt_descriptions' which returns some snippets of text to interpolate into the messages. ssh/common.c calls that while it's still constructing the text, and incorporates the resulting snippets into the SeatDialogText. For the moment, this refactoring only affects the host key prompts. The warnings about outmoded crypto are still done the old-fashioned way; they probably ought to be similarly refactored to use this new SeatDialogText system, but it's not immediately critical for the purpose I have right now.
2022-07-07 16:25:15 +00:00
SeatDialogText *text = seat_dialog_text_new();
const SeatDialogPromptDescriptions *pds =
seat_prompt_descriptions(iseat.seat);
FingerprintType fptype_default =
ssh2_pick_default_fingerprint(fingerprints);
seat_dialog_text_append(
text, SDT_TITLE, "%s Security Alert", appname);
HelpCtx helpctx;
Allow manually confirming and caching certified keys. In the case where a server presents a host key signed by a different certificate from the one you've configured, it need not _always_ be evidence of wrongdoing. I can imagine situations in which two CAs cover overlapping sets of things, and you don't want to blanket-trust one of them, but you do want to connect to a specific host signed by that one. Accordingly, PuTTY's previous policy of unconditionally aborting the connection if certificate validation fails (which was always intended as a stopgap until I thought through what I wanted to replace it with) is now replaced by fallback handling: we present the host key fingerprint to the user and give them the option to accept and/or cache it based on the public key itself. This means that the certified key types have to have a representation in the host key cache. So I've assigned each one a type id, and generate the cache string itself by simply falling back to the base key. (Rationale for the latter: re-signing a public key with a different certificate doesn't change the _private_ key, or the set of valid signatures generated with it. So if you've been convinced for reasons other than the certificate that a particular private key is in the possession of $host, then proof of ownership of that private key should be enough to convince you you're talking to $host no matter what CA has signed the public half this week.) We now offer to receive a given certified host key type if _either_ we have at least one CA configured to trust that host, _or_ we have a certified key of that type cached. (So once you've decided manually that you trust a particular key, we can still receive that key and authenticate the host with it, even if you later delete the CA record that it didn't match anyway.) One change from normal (uncertified) host key handling is that for certified key types _all_ the host key prompts use the stronger language, with "WARNING - POTENTIAL SECURITY BREACH!" rather than the mild 'hmm, we haven't seen this host before'. Rationale: if you expected this CA key and got that one, it _could_ be a bold-as-brass MITM attempt in which someone hoped you'd accept their entire CA key. The mild wording is only for the case where we had no previous expectations _at all_ for the host to violate: not a CA _or_ a cached key.
2022-07-16 10:23:13 +00:00
if (key && ssh_key_alg(key)->is_certificate) {
seat_dialog_text_append(
text, SDT_SCARY_HEADING, "WARNING - POTENTIAL SECURITY BREACH!");
seat_dialog_text_append(
text, SDT_PARA, "This server presented a certified host key:");
seat_dialog_text_append(
text, SDT_DISPLAY, "%s (port %d)", host, port);
if (ca_count) {
seat_dialog_text_append(
text, SDT_PARA, "which was signed by a different "
"certification authority from the %s %s is configured to "
"trust for this server.", ca_count > 1 ? "ones" : "one",
appname);
if (storage_status == 2) {
seat_dialog_text_append(
text, SDT_PARA, "ALSO, that key does not match the key "
"%s had previously cached for this server.", appname);
seat_dialog_text_append(
text, SDT_PARA, "This means that either another "
"certification authority is operating in this realm AND "
"the server administrator has changed the host key, or "
"you have actually connected to another computer "
"pretending to be the server.");
} else {
seat_dialog_text_append(
text, SDT_PARA, "This means that either another "
"certification authority is operating in this realm, or "
"you have actually connected to another computer "
"pretending to be the server.");
}
} else {
assert(storage_status == 2);
seat_dialog_text_append(
text, SDT_PARA, "which does not match the certified key %s "
"had previously cached for this server.", appname);
seat_dialog_text_append(
text, SDT_PARA, "This means that either the server "
"administrator has changed the host key, or you have actually "
"connected to another computer pretending to be the server.");
}
seat_dialog_text_append(
text, SDT_PARA, "The new %s key fingerprint is:", keytype);
seat_dialog_text_append(
text, SDT_DISPLAY, "%s", fingerprints[fptype_default]);
helpctx = HELPCTX(errors_cert_mismatch);
Allow manually confirming and caching certified keys. In the case where a server presents a host key signed by a different certificate from the one you've configured, it need not _always_ be evidence of wrongdoing. I can imagine situations in which two CAs cover overlapping sets of things, and you don't want to blanket-trust one of them, but you do want to connect to a specific host signed by that one. Accordingly, PuTTY's previous policy of unconditionally aborting the connection if certificate validation fails (which was always intended as a stopgap until I thought through what I wanted to replace it with) is now replaced by fallback handling: we present the host key fingerprint to the user and give them the option to accept and/or cache it based on the public key itself. This means that the certified key types have to have a representation in the host key cache. So I've assigned each one a type id, and generate the cache string itself by simply falling back to the base key. (Rationale for the latter: re-signing a public key with a different certificate doesn't change the _private_ key, or the set of valid signatures generated with it. So if you've been convinced for reasons other than the certificate that a particular private key is in the possession of $host, then proof of ownership of that private key should be enough to convince you you're talking to $host no matter what CA has signed the public half this week.) We now offer to receive a given certified host key type if _either_ we have at least one CA configured to trust that host, _or_ we have a certified key of that type cached. (So once you've decided manually that you trust a particular key, we can still receive that key and authenticate the host with it, even if you later delete the CA record that it didn't match anyway.) One change from normal (uncertified) host key handling is that for certified key types _all_ the host key prompts use the stronger language, with "WARNING - POTENTIAL SECURITY BREACH!" rather than the mild 'hmm, we haven't seen this host before'. Rationale: if you expected this CA key and got that one, it _could_ be a bold-as-brass MITM attempt in which someone hoped you'd accept their entire CA key. The mild wording is only for the case where we had no previous expectations _at all_ for the host to violate: not a CA _or_ a cached key.
2022-07-16 10:23:13 +00:00
} else if (storage_status == 1) {
Centralise most details of host-key prompting. The text of the host key warnings was replicated in three places: the Windows rc file, the GTK dialog setup function, and the console.c shared between both platforms' CLI tools. Now it lives in just one place, namely ssh/common.c where the rest of the centralised host-key checking is done, so it'll be easier to adjust the wording in future. This comes with some extra automation. Paragraph wrapping is no longer done by hand in any version of these prompts. (Previously we let GTK do the wrapping on GTK, but on Windows the resource file contained a bunch of pre-wrapped LTEXT lines, and console.c had pre-wrapped terminal messages.) And the dialog heights in Windows are determined automatically based on the amount of stuff in the window. The main idea of all this is that it'll be easier to set up more elaborate kinds of host key prompt that deal with certificates (if, e.g., a server sends us a certified host key which we don't trust the CA for). But there are side benefits of this refactoring too: each tool now reliably inserts its own appname in the prompts, and also, on Windows the entire prompt text is copy-pastable. Details of implementation: there's a new type SeatDialogText which holds a set of (type, string) pairs describing the contents of a prompt. Type codes distinguish ordinary text paragraphs, paragraphs to be displayed prominently (like key fingerprints), the extra-bold scary title at the top of the 'host key changed' version of the dialog, and the various information that lives in the subsidiary 'more info' box. ssh/common.c constructs this, and passes it to the Seat to present the actual prompt. In order to deal with the different UI for answering the prompt, I've added an extra Seat method 'prompt_descriptions' which returns some snippets of text to interpolate into the messages. ssh/common.c calls that while it's still constructing the text, and incorporates the resulting snippets into the SeatDialogText. For the moment, this refactoring only affects the host key prompts. The warnings about outmoded crypto are still done the old-fashioned way; they probably ought to be similarly refactored to use this new SeatDialogText system, but it's not immediately critical for the purpose I have right now.
2022-07-07 16:25:15 +00:00
seat_dialog_text_append(
text, SDT_PARA, "The host key is not cached for this server:");
seat_dialog_text_append(
text, SDT_DISPLAY, "%s (port %d)", host, port);
seat_dialog_text_append(
text, SDT_PARA, "You have no guarantee that the server is the "
"computer you think it is.");
seat_dialog_text_append(
text, SDT_PARA, "The server's %s key fingerprint is:", keytype);
seat_dialog_text_append(
text, SDT_DISPLAY, "%s", fingerprints[fptype_default]);
helpctx = HELPCTX(errors_hostkey_absent);
Centralise most details of host-key prompting. The text of the host key warnings was replicated in three places: the Windows rc file, the GTK dialog setup function, and the console.c shared between both platforms' CLI tools. Now it lives in just one place, namely ssh/common.c where the rest of the centralised host-key checking is done, so it'll be easier to adjust the wording in future. This comes with some extra automation. Paragraph wrapping is no longer done by hand in any version of these prompts. (Previously we let GTK do the wrapping on GTK, but on Windows the resource file contained a bunch of pre-wrapped LTEXT lines, and console.c had pre-wrapped terminal messages.) And the dialog heights in Windows are determined automatically based on the amount of stuff in the window. The main idea of all this is that it'll be easier to set up more elaborate kinds of host key prompt that deal with certificates (if, e.g., a server sends us a certified host key which we don't trust the CA for). But there are side benefits of this refactoring too: each tool now reliably inserts its own appname in the prompts, and also, on Windows the entire prompt text is copy-pastable. Details of implementation: there's a new type SeatDialogText which holds a set of (type, string) pairs describing the contents of a prompt. Type codes distinguish ordinary text paragraphs, paragraphs to be displayed prominently (like key fingerprints), the extra-bold scary title at the top of the 'host key changed' version of the dialog, and the various information that lives in the subsidiary 'more info' box. ssh/common.c constructs this, and passes it to the Seat to present the actual prompt. In order to deal with the different UI for answering the prompt, I've added an extra Seat method 'prompt_descriptions' which returns some snippets of text to interpolate into the messages. ssh/common.c calls that while it's still constructing the text, and incorporates the resulting snippets into the SeatDialogText. For the moment, this refactoring only affects the host key prompts. The warnings about outmoded crypto are still done the old-fashioned way; they probably ought to be similarly refactored to use this new SeatDialogText system, but it's not immediately critical for the purpose I have right now.
2022-07-07 16:25:15 +00:00
} else {
seat_dialog_text_append(
text, SDT_SCARY_HEADING, "WARNING - POTENTIAL SECURITY BREACH!");
seat_dialog_text_append(
text, SDT_PARA, "The host key does not match the one %s has "
"cached for this server:", appname);
seat_dialog_text_append(
text, SDT_DISPLAY, "%s (port %d)", host, port);
seat_dialog_text_append(
text, SDT_PARA, "This means that either the server administrator "
"has changed the host key, or you have actually connected to "
"another computer pretending to be the server.");
seat_dialog_text_append(
text, SDT_PARA, "The new %s key fingerprint is:", keytype);
seat_dialog_text_append(
text, SDT_DISPLAY, "%s", fingerprints[fptype_default]);
helpctx = HELPCTX(errors_hostkey_changed);
Centralise most details of host-key prompting. The text of the host key warnings was replicated in three places: the Windows rc file, the GTK dialog setup function, and the console.c shared between both platforms' CLI tools. Now it lives in just one place, namely ssh/common.c where the rest of the centralised host-key checking is done, so it'll be easier to adjust the wording in future. This comes with some extra automation. Paragraph wrapping is no longer done by hand in any version of these prompts. (Previously we let GTK do the wrapping on GTK, but on Windows the resource file contained a bunch of pre-wrapped LTEXT lines, and console.c had pre-wrapped terminal messages.) And the dialog heights in Windows are determined automatically based on the amount of stuff in the window. The main idea of all this is that it'll be easier to set up more elaborate kinds of host key prompt that deal with certificates (if, e.g., a server sends us a certified host key which we don't trust the CA for). But there are side benefits of this refactoring too: each tool now reliably inserts its own appname in the prompts, and also, on Windows the entire prompt text is copy-pastable. Details of implementation: there's a new type SeatDialogText which holds a set of (type, string) pairs describing the contents of a prompt. Type codes distinguish ordinary text paragraphs, paragraphs to be displayed prominently (like key fingerprints), the extra-bold scary title at the top of the 'host key changed' version of the dialog, and the various information that lives in the subsidiary 'more info' box. ssh/common.c constructs this, and passes it to the Seat to present the actual prompt. In order to deal with the different UI for answering the prompt, I've added an extra Seat method 'prompt_descriptions' which returns some snippets of text to interpolate into the messages. ssh/common.c calls that while it's still constructing the text, and incorporates the resulting snippets into the SeatDialogText. For the moment, this refactoring only affects the host key prompts. The warnings about outmoded crypto are still done the old-fashioned way; they probably ought to be similarly refactored to use this new SeatDialogText system, but it's not immediately critical for the purpose I have right now.
2022-07-07 16:25:15 +00:00
}
/* The above text is printed even in batch mode. Here's where we stop if
* we can't present interactive prompts. */
seat_dialog_text_append(
text, SDT_BATCH_ABORT, "Connection abandoned.");
if (storage_status == 1) {
seat_dialog_text_append(
text, SDT_PARA, "If you trust this host, %s to add the key to "
"%s's cache and carry on connecting.",
pds->hk_accept_action, appname);
if (key && ssh_key_alg(key)->is_certificate) {
seat_dialog_text_append(
text, SDT_PARA, "(Storing this certified key in the cache "
"will NOT cause its certification authority to be trusted "
"for any other key or host.)");
}
Centralise most details of host-key prompting. The text of the host key warnings was replicated in three places: the Windows rc file, the GTK dialog setup function, and the console.c shared between both platforms' CLI tools. Now it lives in just one place, namely ssh/common.c where the rest of the centralised host-key checking is done, so it'll be easier to adjust the wording in future. This comes with some extra automation. Paragraph wrapping is no longer done by hand in any version of these prompts. (Previously we let GTK do the wrapping on GTK, but on Windows the resource file contained a bunch of pre-wrapped LTEXT lines, and console.c had pre-wrapped terminal messages.) And the dialog heights in Windows are determined automatically based on the amount of stuff in the window. The main idea of all this is that it'll be easier to set up more elaborate kinds of host key prompt that deal with certificates (if, e.g., a server sends us a certified host key which we don't trust the CA for). But there are side benefits of this refactoring too: each tool now reliably inserts its own appname in the prompts, and also, on Windows the entire prompt text is copy-pastable. Details of implementation: there's a new type SeatDialogText which holds a set of (type, string) pairs describing the contents of a prompt. Type codes distinguish ordinary text paragraphs, paragraphs to be displayed prominently (like key fingerprints), the extra-bold scary title at the top of the 'host key changed' version of the dialog, and the various information that lives in the subsidiary 'more info' box. ssh/common.c constructs this, and passes it to the Seat to present the actual prompt. In order to deal with the different UI for answering the prompt, I've added an extra Seat method 'prompt_descriptions' which returns some snippets of text to interpolate into the messages. ssh/common.c calls that while it's still constructing the text, and incorporates the resulting snippets into the SeatDialogText. For the moment, this refactoring only affects the host key prompts. The warnings about outmoded crypto are still done the old-fashioned way; they probably ought to be similarly refactored to use this new SeatDialogText system, but it's not immediately critical for the purpose I have right now.
2022-07-07 16:25:15 +00:00
seat_dialog_text_append(
text, SDT_PARA, "If you want to carry on connecting just once, "
"without adding the key to the cache, %s.",
pds->hk_connect_once_action);
seat_dialog_text_append(
text, SDT_PARA, "If you do not trust this host, %s to abandon the "
"connection.", pds->hk_cancel_action);
seat_dialog_text_append(
text, SDT_PROMPT, "Store key in cache?");
} else {
seat_dialog_text_append(
text, SDT_PARA, "If you were expecting this change and trust the "
"new key, %s to update %s's cache and carry on connecting.",
pds->hk_accept_action, appname);
if (key && ssh_key_alg(key)->is_certificate) {
seat_dialog_text_append(
text, SDT_PARA, "(Storing this certified key in the cache "
"will NOT cause its certification authority to be trusted "
"for any other key or host.)");
}
Centralise most details of host-key prompting. The text of the host key warnings was replicated in three places: the Windows rc file, the GTK dialog setup function, and the console.c shared between both platforms' CLI tools. Now it lives in just one place, namely ssh/common.c where the rest of the centralised host-key checking is done, so it'll be easier to adjust the wording in future. This comes with some extra automation. Paragraph wrapping is no longer done by hand in any version of these prompts. (Previously we let GTK do the wrapping on GTK, but on Windows the resource file contained a bunch of pre-wrapped LTEXT lines, and console.c had pre-wrapped terminal messages.) And the dialog heights in Windows are determined automatically based on the amount of stuff in the window. The main idea of all this is that it'll be easier to set up more elaborate kinds of host key prompt that deal with certificates (if, e.g., a server sends us a certified host key which we don't trust the CA for). But there are side benefits of this refactoring too: each tool now reliably inserts its own appname in the prompts, and also, on Windows the entire prompt text is copy-pastable. Details of implementation: there's a new type SeatDialogText which holds a set of (type, string) pairs describing the contents of a prompt. Type codes distinguish ordinary text paragraphs, paragraphs to be displayed prominently (like key fingerprints), the extra-bold scary title at the top of the 'host key changed' version of the dialog, and the various information that lives in the subsidiary 'more info' box. ssh/common.c constructs this, and passes it to the Seat to present the actual prompt. In order to deal with the different UI for answering the prompt, I've added an extra Seat method 'prompt_descriptions' which returns some snippets of text to interpolate into the messages. ssh/common.c calls that while it's still constructing the text, and incorporates the resulting snippets into the SeatDialogText. For the moment, this refactoring only affects the host key prompts. The warnings about outmoded crypto are still done the old-fashioned way; they probably ought to be similarly refactored to use this new SeatDialogText system, but it's not immediately critical for the purpose I have right now.
2022-07-07 16:25:15 +00:00
seat_dialog_text_append(
text, SDT_PARA, "If you want to carry on connecting but without "
"updating the cache, %s.", pds->hk_connect_once_action);
seat_dialog_text_append(
text, SDT_PARA, "If you want to abandon the connection "
"completely, %s to cancel. %s is the ONLY guaranteed safe choice.",
pds->hk_cancel_action, pds->hk_cancel_action_Participle);
seat_dialog_text_append(
text, SDT_PROMPT, "Update cached key?");
}
seat_dialog_text_append(text, SDT_MORE_INFO_KEY,
"Full text of host's public key");
seat_dialog_text_append(text, SDT_MORE_INFO_VALUE_BLOB, "%s", keydisp);
if (fingerprints[SSH_FPTYPE_SHA256]) {
seat_dialog_text_append(text, SDT_MORE_INFO_KEY, "SHA256 fingerprint");
seat_dialog_text_append(text, SDT_MORE_INFO_VALUE_SHORT, "%s",
fingerprints[SSH_FPTYPE_SHA256]);
}
if (fingerprints[SSH_FPTYPE_MD5]) {
seat_dialog_text_append(text, SDT_MORE_INFO_KEY, "MD5 fingerprint");
seat_dialog_text_append(text, SDT_MORE_INFO_VALUE_SHORT, "%s",
fingerprints[SSH_FPTYPE_MD5]);
}
SeatPromptResult toret = seat_confirm_ssh_host_key(
iseat, host, port, keytype, keystr, text, helpctx, callback, ctx);
seat_dialog_text_free(text);
return toret;
}
Move most of ssh.c out into separate source files. I've tried to separate out as many individually coherent changes from this work as I could into their own commits, but here's where I run out and have to commit the rest of this major refactoring as a big-bang change. Most of ssh.c is now no longer in ssh.c: all five of the main coroutines that handle layers of the SSH-1 and SSH-2 protocols now each have their own source file to live in, and a lot of the supporting functions have moved into the appropriate one of those too. The new abstraction is a vtable called 'PacketProtocolLayer', which has an input and output packet queue. Each layer's main coroutine is invoked from the method ssh_ppl_process_queue(), which is usually (though not exclusively) triggered automatically when things are pushed on the input queue. In SSH-2, the base layer is the transport protocol, and it contains a pair of subsidiary queues by which it passes some of its packets to the higher SSH-2 layers - first userauth and then connection, which are peers at the same level, with the former abdicating in favour of the latter at the appropriate moment. SSH-1 is simpler: the whole login phase of the protocol (crypto setup and authentication) is all in one module, and since SSH-1 has no repeat key exchange, that setup layer abdicates in favour of the connection phase when it's done. ssh.c itself is now about a tenth of its old size (which all by itself is cause for celebration!). Its main job is to set up all the layers, hook them up to each other and to the BPP, and to funnel data back and forth between that collection of modules and external things such as the network and the terminal. Once it's set up a collection of packet protocol layers, it communicates with them partly by calling methods of the base layer (and if that's ssh2transport then it will delegate some functionality to the corresponding methods of its higher layer), and partly by talking directly to the connection layer no matter where it is in the stack by means of the separate ConnectionLayer vtable which I introduced in commit 8001dd4cb, and to which I've now added quite a few extra methods replacing services that used to be internal function calls within ssh.c. (One effect of this is that the SSH-1 and SSH-2 channel storage is now no longer shared - there are distinct struct types ssh1_channel and ssh2_channel. That means a bit more code duplication, but on the plus side, a lot fewer confusing conditionals in the middle of half-shared functions, and less risk of a piece of SSH-1 escaping into SSH-2 or vice versa, which I remember has happened at least once in the past.) The bulk of this commit introduces the five new source files, their common header sshppl.h and some shared supporting routines in sshcommon.c, and rewrites nearly all of ssh.c itself. But it also includes a couple of other changes that I couldn't separate easily enough: Firstly, there's a new handling for socket EOF, in which ssh.c sets an 'input_eof' flag in the BPP, and that responds by checking a flag that tells it whether to report the EOF as an error or not. (This is the main reason for those new BPP_READ / BPP_WAITFOR macros - they can check the EOF flag every time the coroutine is resumed.) Secondly, the error reporting itself is changed around again. I'd expected to put some data fields in the public PacketProtocolLayer structure that it could set to report errors in the same way as the BPPs have been doing, but in the end, I decided propagating all those data fields around was a pain and that even the BPPs shouldn't have been doing it that way. So I've reverted to a system where everything calls back to functions in ssh.c itself to report any connection- ending condition. But there's a new family of those functions, categorising the possible such conditions by semantics, and each one has a different set of detailed effects (e.g. how rudely to close the network connection, what exit status should be passed back to the whole application, whether to send a disconnect message and/or display a GUI error box). I don't expect this to be immediately perfect: of course, the code has been through a big upheaval, new bugs are expected, and I haven't been able to do a full job of testing (e.g. I haven't tested every auth or kex method). But I've checked that it _basically_ works - both SSH protocols, all the different kinds of forwarding channel, more than one auth method, Windows and Linux, connection sharing - and I think it's now at the point where the easiest way to find further bugs is to let it out into the wild and see what users can spot.
2018-09-24 17:28:16 +00:00
SeatPromptResult confirm_weak_crypto_primitive(
InteractionReadySeat iseat, const char *algtype, const char *algname,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx,
WeakCryptoReason wcr)
{
SeatDialogText *text = seat_dialog_text_new();
const SeatDialogPromptDescriptions *pds =
seat_prompt_descriptions(iseat.seat);
seat_dialog_text_append(text, SDT_TITLE, "%s Security Alert", appname);
switch (wcr) {
case WCR_BELOW_THRESHOLD:
seat_dialog_text_append(
text, SDT_PARA,
"The first %s supported by the server is %s, "
"which is below the configured warning threshold.",
algtype, algname);
break;
case WCR_TERRAPIN:
case WCR_TERRAPIN_AVOIDABLE:
seat_dialog_text_append(
text, SDT_PARA,
"The %s selected for this session is %s, "
"which, with this server, is vulnerable to the 'Terrapin' attack "
"CVE-2023-48795, potentially allowing an attacker to modify "
"the encrypted session.",
algtype, algname);
seat_dialog_text_append(
text, SDT_PARA,
"Upgrading, patching, or reconfiguring this SSH server is the "
"best way to avoid this vulnerability, if possible.");
if (wcr == WCR_TERRAPIN_AVOIDABLE) {
seat_dialog_text_append(
text, SDT_PARA,
"You can also avoid this vulnerability by abandoning "
"this connection, moving ChaCha20 to below the "
"'warn below here' line in PuTTY's SSH cipher "
"configuration (so that an algorithm without the "
"vulnerability will be selected), and starting a new "
"connection.");
}
break;
default:
unreachable("bad WeakCryptoReason");
}
/* In batch mode, we print the above information and then this
* abort message, and stop. */
seat_dialog_text_append(text, SDT_BATCH_ABORT, "Connection abandoned.");
seat_dialog_text_append(
text, SDT_PARA, "To accept the risk and continue, %s. "
"To abandon the connection, %s.",
pds->weak_accept_action, pds->weak_cancel_action);
seat_dialog_text_append(text, SDT_PROMPT, "Continue with connection?");
SeatPromptResult toret = seat_confirm_weak_crypto_primitive(
iseat, text, callback, ctx);
seat_dialog_text_free(text);
return toret;
}
SeatPromptResult confirm_weak_cached_hostkey(
InteractionReadySeat iseat, const char *algname, const char **betteralgs,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
SeatDialogText *text = seat_dialog_text_new();
const SeatDialogPromptDescriptions *pds =
seat_prompt_descriptions(iseat.seat);
seat_dialog_text_append(text, SDT_TITLE, "%s Security Alert", appname);
seat_dialog_text_append(
text, SDT_PARA,
"The first host key type we have stored for this server "
"is %s, which is below the configured warning threshold.", algname);
seat_dialog_text_append(
text, SDT_PARA,
"The server also provides the following types of host key "
"above the threshold, which we do not have stored:");
for (const char **p = betteralgs; *p; p++)
seat_dialog_text_append(text, SDT_DISPLAY, "%s", *p);
/* In batch mode, we print the above information and then this
* abort message, and stop. */
seat_dialog_text_append(text, SDT_BATCH_ABORT, "Connection abandoned.");
seat_dialog_text_append(
text, SDT_PARA, "To accept the risk and continue, %s. "
"To abandon the connection, %s.",
pds->weak_accept_action, pds->weak_cancel_action);
seat_dialog_text_append(text, SDT_PROMPT, "Continue with connection?");
SeatPromptResult toret = seat_confirm_weak_cached_hostkey(
iseat, text, callback, ctx);
seat_dialog_text_free(text);
return toret;
}
Move most of ssh.c out into separate source files. I've tried to separate out as many individually coherent changes from this work as I could into their own commits, but here's where I run out and have to commit the rest of this major refactoring as a big-bang change. Most of ssh.c is now no longer in ssh.c: all five of the main coroutines that handle layers of the SSH-1 and SSH-2 protocols now each have their own source file to live in, and a lot of the supporting functions have moved into the appropriate one of those too. The new abstraction is a vtable called 'PacketProtocolLayer', which has an input and output packet queue. Each layer's main coroutine is invoked from the method ssh_ppl_process_queue(), which is usually (though not exclusively) triggered automatically when things are pushed on the input queue. In SSH-2, the base layer is the transport protocol, and it contains a pair of subsidiary queues by which it passes some of its packets to the higher SSH-2 layers - first userauth and then connection, which are peers at the same level, with the former abdicating in favour of the latter at the appropriate moment. SSH-1 is simpler: the whole login phase of the protocol (crypto setup and authentication) is all in one module, and since SSH-1 has no repeat key exchange, that setup layer abdicates in favour of the connection phase when it's done. ssh.c itself is now about a tenth of its old size (which all by itself is cause for celebration!). Its main job is to set up all the layers, hook them up to each other and to the BPP, and to funnel data back and forth between that collection of modules and external things such as the network and the terminal. Once it's set up a collection of packet protocol layers, it communicates with them partly by calling methods of the base layer (and if that's ssh2transport then it will delegate some functionality to the corresponding methods of its higher layer), and partly by talking directly to the connection layer no matter where it is in the stack by means of the separate ConnectionLayer vtable which I introduced in commit 8001dd4cb, and to which I've now added quite a few extra methods replacing services that used to be internal function calls within ssh.c. (One effect of this is that the SSH-1 and SSH-2 channel storage is now no longer shared - there are distinct struct types ssh1_channel and ssh2_channel. That means a bit more code duplication, but on the plus side, a lot fewer confusing conditionals in the middle of half-shared functions, and less risk of a piece of SSH-1 escaping into SSH-2 or vice versa, which I remember has happened at least once in the past.) The bulk of this commit introduces the five new source files, their common header sshppl.h and some shared supporting routines in sshcommon.c, and rewrites nearly all of ssh.c itself. But it also includes a couple of other changes that I couldn't separate easily enough: Firstly, there's a new handling for socket EOF, in which ssh.c sets an 'input_eof' flag in the BPP, and that responds by checking a flag that tells it whether to report the EOF as an error or not. (This is the main reason for those new BPP_READ / BPP_WAITFOR macros - they can check the EOF flag every time the coroutine is resumed.) Secondly, the error reporting itself is changed around again. I'd expected to put some data fields in the public PacketProtocolLayer structure that it could set to report errors in the same way as the BPPs have been doing, but in the end, I decided propagating all those data fields around was a pain and that even the BPPs shouldn't have been doing it that way. So I've reverted to a system where everything calls back to functions in ssh.c itself to report any connection- ending condition. But there's a new family of those functions, categorising the possible such conditions by semantics, and each one has a different set of detailed effects (e.g. how rudely to close the network connection, what exit status should be passed back to the whole application, whether to send a disconnect message and/or display a GUI error box). I don't expect this to be immediately perfect: of course, the code has been through a big upheaval, new bugs are expected, and I haven't been able to do a full job of testing (e.g. I haven't tested every auth or kex method). But I've checked that it _basically_ works - both SSH protocols, all the different kinds of forwarding channel, more than one auth method, Windows and Linux, connection sharing - and I think it's now at the point where the easiest way to find further bugs is to let it out into the wild and see what users can spot.
2018-09-24 17:28:16 +00:00
/* ----------------------------------------------------------------------
Move client-specific SSH code into new files. This is a major code reorganisation in preparation for making this code base into one that can build an SSH server as well as a client. (Mostly for purposes of using the server as a regression test suite for the client, though I have some other possible uses in mind too. However, it's currently no part of my plan to harden the server to the point where it can sensibly be deployed in a hostile environment.) In this preparatory commit, I've broken up the SSH-2 transport and connection layers, and the SSH-1 connection layer, into multiple source files, with each layer having its own header file containing the shared type definitions. In each case, the new source file contains code that's specific to the client side of the protocol, so that a new file can be swapped in in its place when building the server. Mostly this is just a straightforward moving of code without changing it very much, but there are a couple of actual changes in the process: The parsing of SSH-2 global-request and channel open-messages is now done by a new pair of functions in the client module. For channel opens, I've invented a new union data type to be the return value from that function, representing either failure (plus error message), success (plus Channel instance to manage the new channel), or an instruction to hand the channel over to a sharing downstream (plus a pointer to the downstream in question). Also, the tree234 of remote port forwardings in ssh2connection is now initialised on first use by the client-specific code, so that's where its compare function lives. The shared ssh2connection_free() still takes responsibility for freeing it, but now has to check if it's non-null first. The outer shell of the ssh2_lportfwd_open method, for making a local-to-remote port forwarding, is still centralised in ssh2connection.c, but the part of it that actually constructs the outgoing channel-open message has moved into the client code, because that will have to change depending on whether the channel-open has to have type direct-tcpip or forwarded-tcpip. In the SSH-1 connection layer, half the filter_queue method has moved out into the new client-specific code, but not all of it - bidirectional channel maintenance messages are still handled centrally. One exception is SSH_MSG_PORT_OPEN, which can be sent in both directions, but with subtly different semantics - from server to client, it's referring to a previously established remote forwarding (and must be rejected if there isn't one that matches it), but from client to server it's just a "direct-tcpip" request with no prior context. So that one is in the client-specific module, and when I add the server code it will have its own different handler.
2018-10-20 16:57:37 +00:00
* Common functions shared between SSH-1 layers.
Move most of ssh.c out into separate source files. I've tried to separate out as many individually coherent changes from this work as I could into their own commits, but here's where I run out and have to commit the rest of this major refactoring as a big-bang change. Most of ssh.c is now no longer in ssh.c: all five of the main coroutines that handle layers of the SSH-1 and SSH-2 protocols now each have their own source file to live in, and a lot of the supporting functions have moved into the appropriate one of those too. The new abstraction is a vtable called 'PacketProtocolLayer', which has an input and output packet queue. Each layer's main coroutine is invoked from the method ssh_ppl_process_queue(), which is usually (though not exclusively) triggered automatically when things are pushed on the input queue. In SSH-2, the base layer is the transport protocol, and it contains a pair of subsidiary queues by which it passes some of its packets to the higher SSH-2 layers - first userauth and then connection, which are peers at the same level, with the former abdicating in favour of the latter at the appropriate moment. SSH-1 is simpler: the whole login phase of the protocol (crypto setup and authentication) is all in one module, and since SSH-1 has no repeat key exchange, that setup layer abdicates in favour of the connection phase when it's done. ssh.c itself is now about a tenth of its old size (which all by itself is cause for celebration!). Its main job is to set up all the layers, hook them up to each other and to the BPP, and to funnel data back and forth between that collection of modules and external things such as the network and the terminal. Once it's set up a collection of packet protocol layers, it communicates with them partly by calling methods of the base layer (and if that's ssh2transport then it will delegate some functionality to the corresponding methods of its higher layer), and partly by talking directly to the connection layer no matter where it is in the stack by means of the separate ConnectionLayer vtable which I introduced in commit 8001dd4cb, and to which I've now added quite a few extra methods replacing services that used to be internal function calls within ssh.c. (One effect of this is that the SSH-1 and SSH-2 channel storage is now no longer shared - there are distinct struct types ssh1_channel and ssh2_channel. That means a bit more code duplication, but on the plus side, a lot fewer confusing conditionals in the middle of half-shared functions, and less risk of a piece of SSH-1 escaping into SSH-2 or vice versa, which I remember has happened at least once in the past.) The bulk of this commit introduces the five new source files, their common header sshppl.h and some shared supporting routines in sshcommon.c, and rewrites nearly all of ssh.c itself. But it also includes a couple of other changes that I couldn't separate easily enough: Firstly, there's a new handling for socket EOF, in which ssh.c sets an 'input_eof' flag in the BPP, and that responds by checking a flag that tells it whether to report the EOF as an error or not. (This is the main reason for those new BPP_READ / BPP_WAITFOR macros - they can check the EOF flag every time the coroutine is resumed.) Secondly, the error reporting itself is changed around again. I'd expected to put some data fields in the public PacketProtocolLayer structure that it could set to report errors in the same way as the BPPs have been doing, but in the end, I decided propagating all those data fields around was a pain and that even the BPPs shouldn't have been doing it that way. So I've reverted to a system where everything calls back to functions in ssh.c itself to report any connection- ending condition. But there's a new family of those functions, categorising the possible such conditions by semantics, and each one has a different set of detailed effects (e.g. how rudely to close the network connection, what exit status should be passed back to the whole application, whether to send a disconnect message and/or display a GUI error box). I don't expect this to be immediately perfect: of course, the code has been through a big upheaval, new bugs are expected, and I haven't been able to do a full job of testing (e.g. I haven't tested every auth or kex method). But I've checked that it _basically_ works - both SSH protocols, all the different kinds of forwarding channel, more than one auth method, Windows and Linux, connection sharing - and I think it's now at the point where the easiest way to find further bugs is to let it out into the wild and see what users can spot.
2018-09-24 17:28:16 +00:00
*/
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 ssh1_common_get_specials(
Move most of ssh.c out into separate source files. I've tried to separate out as many individually coherent changes from this work as I could into their own commits, but here's where I run out and have to commit the rest of this major refactoring as a big-bang change. Most of ssh.c is now no longer in ssh.c: all five of the main coroutines that handle layers of the SSH-1 and SSH-2 protocols now each have their own source file to live in, and a lot of the supporting functions have moved into the appropriate one of those too. The new abstraction is a vtable called 'PacketProtocolLayer', which has an input and output packet queue. Each layer's main coroutine is invoked from the method ssh_ppl_process_queue(), which is usually (though not exclusively) triggered automatically when things are pushed on the input queue. In SSH-2, the base layer is the transport protocol, and it contains a pair of subsidiary queues by which it passes some of its packets to the higher SSH-2 layers - first userauth and then connection, which are peers at the same level, with the former abdicating in favour of the latter at the appropriate moment. SSH-1 is simpler: the whole login phase of the protocol (crypto setup and authentication) is all in one module, and since SSH-1 has no repeat key exchange, that setup layer abdicates in favour of the connection phase when it's done. ssh.c itself is now about a tenth of its old size (which all by itself is cause for celebration!). Its main job is to set up all the layers, hook them up to each other and to the BPP, and to funnel data back and forth between that collection of modules and external things such as the network and the terminal. Once it's set up a collection of packet protocol layers, it communicates with them partly by calling methods of the base layer (and if that's ssh2transport then it will delegate some functionality to the corresponding methods of its higher layer), and partly by talking directly to the connection layer no matter where it is in the stack by means of the separate ConnectionLayer vtable which I introduced in commit 8001dd4cb, and to which I've now added quite a few extra methods replacing services that used to be internal function calls within ssh.c. (One effect of this is that the SSH-1 and SSH-2 channel storage is now no longer shared - there are distinct struct types ssh1_channel and ssh2_channel. That means a bit more code duplication, but on the plus side, a lot fewer confusing conditionals in the middle of half-shared functions, and less risk of a piece of SSH-1 escaping into SSH-2 or vice versa, which I remember has happened at least once in the past.) The bulk of this commit introduces the five new source files, their common header sshppl.h and some shared supporting routines in sshcommon.c, and rewrites nearly all of ssh.c itself. But it also includes a couple of other changes that I couldn't separate easily enough: Firstly, there's a new handling for socket EOF, in which ssh.c sets an 'input_eof' flag in the BPP, and that responds by checking a flag that tells it whether to report the EOF as an error or not. (This is the main reason for those new BPP_READ / BPP_WAITFOR macros - they can check the EOF flag every time the coroutine is resumed.) Secondly, the error reporting itself is changed around again. I'd expected to put some data fields in the public PacketProtocolLayer structure that it could set to report errors in the same way as the BPPs have been doing, but in the end, I decided propagating all those data fields around was a pain and that even the BPPs shouldn't have been doing it that way. So I've reverted to a system where everything calls back to functions in ssh.c itself to report any connection- ending condition. But there's a new family of those functions, categorising the possible such conditions by semantics, and each one has a different set of detailed effects (e.g. how rudely to close the network connection, what exit status should be passed back to the whole application, whether to send a disconnect message and/or display a GUI error box). I don't expect this to be immediately perfect: of course, the code has been through a big upheaval, new bugs are expected, and I haven't been able to do a full job of testing (e.g. I haven't tested every auth or kex method). But I've checked that it _basically_ works - both SSH protocols, all the different kinds of forwarding channel, more than one auth method, Windows and Linux, connection sharing - and I think it's now at the point where the easiest way to find further bugs is to let it out into the wild and see what users can spot.
2018-09-24 17:28:16 +00:00
PacketProtocolLayer *ppl, add_special_fn_t add_special, void *ctx)
{
/*
* Don't bother offering IGNORE if we've decided the remote
* won't cope with it, since we wouldn't bother sending it if
* asked anyway.
*/
if (!(ppl->remote_bugs & BUG_CHOKES_ON_SSH1_IGNORE)) {
add_special(ctx, "IGNORE message", SS_NOP, 0);
return true;
Move most of ssh.c out into separate source files. I've tried to separate out as many individually coherent changes from this work as I could into their own commits, but here's where I run out and have to commit the rest of this major refactoring as a big-bang change. Most of ssh.c is now no longer in ssh.c: all five of the main coroutines that handle layers of the SSH-1 and SSH-2 protocols now each have their own source file to live in, and a lot of the supporting functions have moved into the appropriate one of those too. The new abstraction is a vtable called 'PacketProtocolLayer', which has an input and output packet queue. Each layer's main coroutine is invoked from the method ssh_ppl_process_queue(), which is usually (though not exclusively) triggered automatically when things are pushed on the input queue. In SSH-2, the base layer is the transport protocol, and it contains a pair of subsidiary queues by which it passes some of its packets to the higher SSH-2 layers - first userauth and then connection, which are peers at the same level, with the former abdicating in favour of the latter at the appropriate moment. SSH-1 is simpler: the whole login phase of the protocol (crypto setup and authentication) is all in one module, and since SSH-1 has no repeat key exchange, that setup layer abdicates in favour of the connection phase when it's done. ssh.c itself is now about a tenth of its old size (which all by itself is cause for celebration!). Its main job is to set up all the layers, hook them up to each other and to the BPP, and to funnel data back and forth between that collection of modules and external things such as the network and the terminal. Once it's set up a collection of packet protocol layers, it communicates with them partly by calling methods of the base layer (and if that's ssh2transport then it will delegate some functionality to the corresponding methods of its higher layer), and partly by talking directly to the connection layer no matter where it is in the stack by means of the separate ConnectionLayer vtable which I introduced in commit 8001dd4cb, and to which I've now added quite a few extra methods replacing services that used to be internal function calls within ssh.c. (One effect of this is that the SSH-1 and SSH-2 channel storage is now no longer shared - there are distinct struct types ssh1_channel and ssh2_channel. That means a bit more code duplication, but on the plus side, a lot fewer confusing conditionals in the middle of half-shared functions, and less risk of a piece of SSH-1 escaping into SSH-2 or vice versa, which I remember has happened at least once in the past.) The bulk of this commit introduces the five new source files, their common header sshppl.h and some shared supporting routines in sshcommon.c, and rewrites nearly all of ssh.c itself. But it also includes a couple of other changes that I couldn't separate easily enough: Firstly, there's a new handling for socket EOF, in which ssh.c sets an 'input_eof' flag in the BPP, and that responds by checking a flag that tells it whether to report the EOF as an error or not. (This is the main reason for those new BPP_READ / BPP_WAITFOR macros - they can check the EOF flag every time the coroutine is resumed.) Secondly, the error reporting itself is changed around again. I'd expected to put some data fields in the public PacketProtocolLayer structure that it could set to report errors in the same way as the BPPs have been doing, but in the end, I decided propagating all those data fields around was a pain and that even the BPPs shouldn't have been doing it that way. So I've reverted to a system where everything calls back to functions in ssh.c itself to report any connection- ending condition. But there's a new family of those functions, categorising the possible such conditions by semantics, and each one has a different set of detailed effects (e.g. how rudely to close the network connection, what exit status should be passed back to the whole application, whether to send a disconnect message and/or display a GUI error box). I don't expect this to be immediately perfect: of course, the code has been through a big upheaval, new bugs are expected, and I haven't been able to do a full job of testing (e.g. I haven't tested every auth or kex method). But I've checked that it _basically_ works - both SSH protocols, all the different kinds of forwarding channel, more than one auth method, Windows and Linux, connection sharing - and I think it's now at the point where the easiest way to find further bugs is to let it out into the wild and see what users can spot.
2018-09-24 17:28:16 +00:00
}
return false;
Move most of ssh.c out into separate source files. I've tried to separate out as many individually coherent changes from this work as I could into their own commits, but here's where I run out and have to commit the rest of this major refactoring as a big-bang change. Most of ssh.c is now no longer in ssh.c: all five of the main coroutines that handle layers of the SSH-1 and SSH-2 protocols now each have their own source file to live in, and a lot of the supporting functions have moved into the appropriate one of those too. The new abstraction is a vtable called 'PacketProtocolLayer', which has an input and output packet queue. Each layer's main coroutine is invoked from the method ssh_ppl_process_queue(), which is usually (though not exclusively) triggered automatically when things are pushed on the input queue. In SSH-2, the base layer is the transport protocol, and it contains a pair of subsidiary queues by which it passes some of its packets to the higher SSH-2 layers - first userauth and then connection, which are peers at the same level, with the former abdicating in favour of the latter at the appropriate moment. SSH-1 is simpler: the whole login phase of the protocol (crypto setup and authentication) is all in one module, and since SSH-1 has no repeat key exchange, that setup layer abdicates in favour of the connection phase when it's done. ssh.c itself is now about a tenth of its old size (which all by itself is cause for celebration!). Its main job is to set up all the layers, hook them up to each other and to the BPP, and to funnel data back and forth between that collection of modules and external things such as the network and the terminal. Once it's set up a collection of packet protocol layers, it communicates with them partly by calling methods of the base layer (and if that's ssh2transport then it will delegate some functionality to the corresponding methods of its higher layer), and partly by talking directly to the connection layer no matter where it is in the stack by means of the separate ConnectionLayer vtable which I introduced in commit 8001dd4cb, and to which I've now added quite a few extra methods replacing services that used to be internal function calls within ssh.c. (One effect of this is that the SSH-1 and SSH-2 channel storage is now no longer shared - there are distinct struct types ssh1_channel and ssh2_channel. That means a bit more code duplication, but on the plus side, a lot fewer confusing conditionals in the middle of half-shared functions, and less risk of a piece of SSH-1 escaping into SSH-2 or vice versa, which I remember has happened at least once in the past.) The bulk of this commit introduces the five new source files, their common header sshppl.h and some shared supporting routines in sshcommon.c, and rewrites nearly all of ssh.c itself. But it also includes a couple of other changes that I couldn't separate easily enough: Firstly, there's a new handling for socket EOF, in which ssh.c sets an 'input_eof' flag in the BPP, and that responds by checking a flag that tells it whether to report the EOF as an error or not. (This is the main reason for those new BPP_READ / BPP_WAITFOR macros - they can check the EOF flag every time the coroutine is resumed.) Secondly, the error reporting itself is changed around again. I'd expected to put some data fields in the public PacketProtocolLayer structure that it could set to report errors in the same way as the BPPs have been doing, but in the end, I decided propagating all those data fields around was a pain and that even the BPPs shouldn't have been doing it that way. So I've reverted to a system where everything calls back to functions in ssh.c itself to report any connection- ending condition. But there's a new family of those functions, categorising the possible such conditions by semantics, and each one has a different set of detailed effects (e.g. how rudely to close the network connection, what exit status should be passed back to the whole application, whether to send a disconnect message and/or display a GUI error box). I don't expect this to be immediately perfect: of course, the code has been through a big upheaval, new bugs are expected, and I haven't been able to do a full job of testing (e.g. I haven't tested every auth or kex method). But I've checked that it _basically_ works - both SSH protocols, all the different kinds of forwarding channel, more than one auth method, Windows and Linux, connection sharing - and I think it's now at the point where the easiest way to find further bugs is to let it out into the wild and see what users can spot.
2018-09-24 17:28:16 +00:00
}
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 ssh1_common_filter_queue(PacketProtocolLayer *ppl)
Move client-specific SSH code into new files. This is a major code reorganisation in preparation for making this code base into one that can build an SSH server as well as a client. (Mostly for purposes of using the server as a regression test suite for the client, though I have some other possible uses in mind too. However, it's currently no part of my plan to harden the server to the point where it can sensibly be deployed in a hostile environment.) In this preparatory commit, I've broken up the SSH-2 transport and connection layers, and the SSH-1 connection layer, into multiple source files, with each layer having its own header file containing the shared type definitions. In each case, the new source file contains code that's specific to the client side of the protocol, so that a new file can be swapped in in its place when building the server. Mostly this is just a straightforward moving of code without changing it very much, but there are a couple of actual changes in the process: The parsing of SSH-2 global-request and channel open-messages is now done by a new pair of functions in the client module. For channel opens, I've invented a new union data type to be the return value from that function, representing either failure (plus error message), success (plus Channel instance to manage the new channel), or an instruction to hand the channel over to a sharing downstream (plus a pointer to the downstream in question). Also, the tree234 of remote port forwardings in ssh2connection is now initialised on first use by the client-specific code, so that's where its compare function lives. The shared ssh2connection_free() still takes responsibility for freeing it, but now has to check if it's non-null first. The outer shell of the ssh2_lportfwd_open method, for making a local-to-remote port forwarding, is still centralised in ssh2connection.c, but the part of it that actually constructs the outgoing channel-open message has moved into the client code, because that will have to change depending on whether the channel-open has to have type direct-tcpip or forwarded-tcpip. In the SSH-1 connection layer, half the filter_queue method has moved out into the new client-specific code, but not all of it - bidirectional channel maintenance messages are still handled centrally. One exception is SSH_MSG_PORT_OPEN, which can be sent in both directions, but with subtly different semantics - from server to client, it's referring to a previously established remote forwarding (and must be rejected if there isn't one that matches it), but from client to server it's just a "direct-tcpip" request with no prior context. So that one is in the client-specific module, and when I add the server code it will have its own different handler.
2018-10-20 16:57:37 +00:00
{
PktIn *pktin;
ptrlen msg;
while ((pktin = pq_peek(ppl->in_pq)) != NULL) {
switch (pktin->type) {
case SSH1_MSG_DISCONNECT:
msg = get_string(pktin);
ssh_remote_error(ppl->ssh,
"Remote side sent disconnect message:\n\"%.*s\"",
PTRLEN_PRINTF(msg));
/* don't try to pop the queue, because we've been freed! */
return true; /* indicate that we've been freed */
Move client-specific SSH code into new files. This is a major code reorganisation in preparation for making this code base into one that can build an SSH server as well as a client. (Mostly for purposes of using the server as a regression test suite for the client, though I have some other possible uses in mind too. However, it's currently no part of my plan to harden the server to the point where it can sensibly be deployed in a hostile environment.) In this preparatory commit, I've broken up the SSH-2 transport and connection layers, and the SSH-1 connection layer, into multiple source files, with each layer having its own header file containing the shared type definitions. In each case, the new source file contains code that's specific to the client side of the protocol, so that a new file can be swapped in in its place when building the server. Mostly this is just a straightforward moving of code without changing it very much, but there are a couple of actual changes in the process: The parsing of SSH-2 global-request and channel open-messages is now done by a new pair of functions in the client module. For channel opens, I've invented a new union data type to be the return value from that function, representing either failure (plus error message), success (plus Channel instance to manage the new channel), or an instruction to hand the channel over to a sharing downstream (plus a pointer to the downstream in question). Also, the tree234 of remote port forwardings in ssh2connection is now initialised on first use by the client-specific code, so that's where its compare function lives. The shared ssh2connection_free() still takes responsibility for freeing it, but now has to check if it's non-null first. The outer shell of the ssh2_lportfwd_open method, for making a local-to-remote port forwarding, is still centralised in ssh2connection.c, but the part of it that actually constructs the outgoing channel-open message has moved into the client code, because that will have to change depending on whether the channel-open has to have type direct-tcpip or forwarded-tcpip. In the SSH-1 connection layer, half the filter_queue method has moved out into the new client-specific code, but not all of it - bidirectional channel maintenance messages are still handled centrally. One exception is SSH_MSG_PORT_OPEN, which can be sent in both directions, but with subtly different semantics - from server to client, it's referring to a previously established remote forwarding (and must be rejected if there isn't one that matches it), but from client to server it's just a "direct-tcpip" request with no prior context. So that one is in the client-specific module, and when I add the server code it will have its own different handler.
2018-10-20 16:57:37 +00:00
case SSH1_MSG_DEBUG:
msg = get_string(pktin);
ppl_logevent("Remote debug message: %.*s", PTRLEN_PRINTF(msg));
Move client-specific SSH code into new files. This is a major code reorganisation in preparation for making this code base into one that can build an SSH server as well as a client. (Mostly for purposes of using the server as a regression test suite for the client, though I have some other possible uses in mind too. However, it's currently no part of my plan to harden the server to the point where it can sensibly be deployed in a hostile environment.) In this preparatory commit, I've broken up the SSH-2 transport and connection layers, and the SSH-1 connection layer, into multiple source files, with each layer having its own header file containing the shared type definitions. In each case, the new source file contains code that's specific to the client side of the protocol, so that a new file can be swapped in in its place when building the server. Mostly this is just a straightforward moving of code without changing it very much, but there are a couple of actual changes in the process: The parsing of SSH-2 global-request and channel open-messages is now done by a new pair of functions in the client module. For channel opens, I've invented a new union data type to be the return value from that function, representing either failure (plus error message), success (plus Channel instance to manage the new channel), or an instruction to hand the channel over to a sharing downstream (plus a pointer to the downstream in question). Also, the tree234 of remote port forwardings in ssh2connection is now initialised on first use by the client-specific code, so that's where its compare function lives. The shared ssh2connection_free() still takes responsibility for freeing it, but now has to check if it's non-null first. The outer shell of the ssh2_lportfwd_open method, for making a local-to-remote port forwarding, is still centralised in ssh2connection.c, but the part of it that actually constructs the outgoing channel-open message has moved into the client code, because that will have to change depending on whether the channel-open has to have type direct-tcpip or forwarded-tcpip. In the SSH-1 connection layer, half the filter_queue method has moved out into the new client-specific code, but not all of it - bidirectional channel maintenance messages are still handled centrally. One exception is SSH_MSG_PORT_OPEN, which can be sent in both directions, but with subtly different semantics - from server to client, it's referring to a previously established remote forwarding (and must be rejected if there isn't one that matches it), but from client to server it's just a "direct-tcpip" request with no prior context. So that one is in the client-specific module, and when I add the server code it will have its own different handler.
2018-10-20 16:57:37 +00:00
pq_pop(ppl->in_pq);
break;
case SSH1_MSG_IGNORE:
/* Do nothing, because we're ignoring it! Duhh. */
pq_pop(ppl->in_pq);
break;
default:
return false;
Move client-specific SSH code into new files. This is a major code reorganisation in preparation for making this code base into one that can build an SSH server as well as a client. (Mostly for purposes of using the server as a regression test suite for the client, though I have some other possible uses in mind too. However, it's currently no part of my plan to harden the server to the point where it can sensibly be deployed in a hostile environment.) In this preparatory commit, I've broken up the SSH-2 transport and connection layers, and the SSH-1 connection layer, into multiple source files, with each layer having its own header file containing the shared type definitions. In each case, the new source file contains code that's specific to the client side of the protocol, so that a new file can be swapped in in its place when building the server. Mostly this is just a straightforward moving of code without changing it very much, but there are a couple of actual changes in the process: The parsing of SSH-2 global-request and channel open-messages is now done by a new pair of functions in the client module. For channel opens, I've invented a new union data type to be the return value from that function, representing either failure (plus error message), success (plus Channel instance to manage the new channel), or an instruction to hand the channel over to a sharing downstream (plus a pointer to the downstream in question). Also, the tree234 of remote port forwardings in ssh2connection is now initialised on first use by the client-specific code, so that's where its compare function lives. The shared ssh2connection_free() still takes responsibility for freeing it, but now has to check if it's non-null first. The outer shell of the ssh2_lportfwd_open method, for making a local-to-remote port forwarding, is still centralised in ssh2connection.c, but the part of it that actually constructs the outgoing channel-open message has moved into the client code, because that will have to change depending on whether the channel-open has to have type direct-tcpip or forwarded-tcpip. In the SSH-1 connection layer, half the filter_queue method has moved out into the new client-specific code, but not all of it - bidirectional channel maintenance messages are still handled centrally. One exception is SSH_MSG_PORT_OPEN, which can be sent in both directions, but with subtly different semantics - from server to client, it's referring to a previously established remote forwarding (and must be rejected if there isn't one that matches it), but from client to server it's just a "direct-tcpip" request with no prior context. So that one is in the client-specific module, and when I add the server code it will have its own different handler.
2018-10-20 16:57:37 +00:00
}
}
return false;
Move client-specific SSH code into new files. This is a major code reorganisation in preparation for making this code base into one that can build an SSH server as well as a client. (Mostly for purposes of using the server as a regression test suite for the client, though I have some other possible uses in mind too. However, it's currently no part of my plan to harden the server to the point where it can sensibly be deployed in a hostile environment.) In this preparatory commit, I've broken up the SSH-2 transport and connection layers, and the SSH-1 connection layer, into multiple source files, with each layer having its own header file containing the shared type definitions. In each case, the new source file contains code that's specific to the client side of the protocol, so that a new file can be swapped in in its place when building the server. Mostly this is just a straightforward moving of code without changing it very much, but there are a couple of actual changes in the process: The parsing of SSH-2 global-request and channel open-messages is now done by a new pair of functions in the client module. For channel opens, I've invented a new union data type to be the return value from that function, representing either failure (plus error message), success (plus Channel instance to manage the new channel), or an instruction to hand the channel over to a sharing downstream (plus a pointer to the downstream in question). Also, the tree234 of remote port forwardings in ssh2connection is now initialised on first use by the client-specific code, so that's where its compare function lives. The shared ssh2connection_free() still takes responsibility for freeing it, but now has to check if it's non-null first. The outer shell of the ssh2_lportfwd_open method, for making a local-to-remote port forwarding, is still centralised in ssh2connection.c, but the part of it that actually constructs the outgoing channel-open message has moved into the client code, because that will have to change depending on whether the channel-open has to have type direct-tcpip or forwarded-tcpip. In the SSH-1 connection layer, half the filter_queue method has moved out into the new client-specific code, but not all of it - bidirectional channel maintenance messages are still handled centrally. One exception is SSH_MSG_PORT_OPEN, which can be sent in both directions, but with subtly different semantics - from server to client, it's referring to a previously established remote forwarding (and must be rejected if there isn't one that matches it), but from client to server it's just a "direct-tcpip" request with no prior context. So that one is in the client-specific module, and when I add the server code it will have its own different handler.
2018-10-20 16:57:37 +00:00
}
void ssh1_compute_session_id(
unsigned char *session_id, const unsigned char *cookie,
RSAKey *hostkey, RSAKey *servkey)
Move client-specific SSH code into new files. This is a major code reorganisation in preparation for making this code base into one that can build an SSH server as well as a client. (Mostly for purposes of using the server as a regression test suite for the client, though I have some other possible uses in mind too. However, it's currently no part of my plan to harden the server to the point where it can sensibly be deployed in a hostile environment.) In this preparatory commit, I've broken up the SSH-2 transport and connection layers, and the SSH-1 connection layer, into multiple source files, with each layer having its own header file containing the shared type definitions. In each case, the new source file contains code that's specific to the client side of the protocol, so that a new file can be swapped in in its place when building the server. Mostly this is just a straightforward moving of code without changing it very much, but there are a couple of actual changes in the process: The parsing of SSH-2 global-request and channel open-messages is now done by a new pair of functions in the client module. For channel opens, I've invented a new union data type to be the return value from that function, representing either failure (plus error message), success (plus Channel instance to manage the new channel), or an instruction to hand the channel over to a sharing downstream (plus a pointer to the downstream in question). Also, the tree234 of remote port forwardings in ssh2connection is now initialised on first use by the client-specific code, so that's where its compare function lives. The shared ssh2connection_free() still takes responsibility for freeing it, but now has to check if it's non-null first. The outer shell of the ssh2_lportfwd_open method, for making a local-to-remote port forwarding, is still centralised in ssh2connection.c, but the part of it that actually constructs the outgoing channel-open message has moved into the client code, because that will have to change depending on whether the channel-open has to have type direct-tcpip or forwarded-tcpip. In the SSH-1 connection layer, half the filter_queue method has moved out into the new client-specific code, but not all of it - bidirectional channel maintenance messages are still handled centrally. One exception is SSH_MSG_PORT_OPEN, which can be sent in both directions, but with subtly different semantics - from server to client, it's referring to a previously established remote forwarding (and must be rejected if there isn't one that matches it), but from client to server it's just a "direct-tcpip" request with no prior context. So that one is in the client-specific module, and when I add the server code it will have its own different handler.
2018-10-20 16:57:37 +00:00
{
ssh_hash *hash = ssh_hash_new(&ssh_md5);
Move client-specific SSH code into new files. This is a major code reorganisation in preparation for making this code base into one that can build an SSH server as well as a client. (Mostly for purposes of using the server as a regression test suite for the client, though I have some other possible uses in mind too. However, it's currently no part of my plan to harden the server to the point where it can sensibly be deployed in a hostile environment.) In this preparatory commit, I've broken up the SSH-2 transport and connection layers, and the SSH-1 connection layer, into multiple source files, with each layer having its own header file containing the shared type definitions. In each case, the new source file contains code that's specific to the client side of the protocol, so that a new file can be swapped in in its place when building the server. Mostly this is just a straightforward moving of code without changing it very much, but there are a couple of actual changes in the process: The parsing of SSH-2 global-request and channel open-messages is now done by a new pair of functions in the client module. For channel opens, I've invented a new union data type to be the return value from that function, representing either failure (plus error message), success (plus Channel instance to manage the new channel), or an instruction to hand the channel over to a sharing downstream (plus a pointer to the downstream in question). Also, the tree234 of remote port forwardings in ssh2connection is now initialised on first use by the client-specific code, so that's where its compare function lives. The shared ssh2connection_free() still takes responsibility for freeing it, but now has to check if it's non-null first. The outer shell of the ssh2_lportfwd_open method, for making a local-to-remote port forwarding, is still centralised in ssh2connection.c, but the part of it that actually constructs the outgoing channel-open message has moved into the client code, because that will have to change depending on whether the channel-open has to have type direct-tcpip or forwarded-tcpip. In the SSH-1 connection layer, half the filter_queue method has moved out into the new client-specific code, but not all of it - bidirectional channel maintenance messages are still handled centrally. One exception is SSH_MSG_PORT_OPEN, which can be sent in both directions, but with subtly different semantics - from server to client, it's referring to a previously established remote forwarding (and must be rejected if there isn't one that matches it), but from client to server it's just a "direct-tcpip" request with no prior context. So that one is in the client-specific module, and when I add the server code it will have its own different handler.
2018-10-20 16:57:37 +00:00
Complete rewrite of PuTTY's bignum library. The old 'Bignum' data type is gone completely, and so is sshbn.c. In its place is a new thing called 'mp_int', handled by an entirely new library module mpint.c, with API differences both large and small. The main aim of this change is that the new library should be free of timing- and cache-related side channels. I've written the code so that it _should_ - assuming I haven't made any mistakes - do all of its work without either control flow or memory addressing depending on the data words of the input numbers. (Though, being an _arbitrary_ precision library, it does have to at least depend on the sizes of the numbers - but there's a 'formal' size that can vary separately from the actual magnitude of the represented integer, so if you want to keep it secret that your number is actually small, it should work fine to have a very long mp_int and just happen to store 23 in it.) So I've done all my conditionalisation by means of computing both answers and doing bit-masking to swap the right one into place, and all loops over the words of an mp_int go up to the formal size rather than the actual size. I haven't actually tested the constant-time property in any rigorous way yet (I'm still considering the best way to do it). But this code is surely at the very least a big improvement on the old version, even if I later find a few more things to fix. I've also completely rewritten the low-level elliptic curve arithmetic from sshecc.c; the new ecc.c is closer to being an adjunct of mpint.c than it is to the SSH end of the code. The new elliptic curve code keeps all coordinates in Montgomery-multiplication transformed form to speed up all the multiplications mod the same prime, and only converts them back when you ask for the affine coordinates. Also, I adopted extended coordinates for the Edwards curve implementation. sshecc.c has also had a near-total rewrite in the course of switching it over to the new system. While I was there, I've separated ECDSA and EdDSA more completely - they now have separate vtables, instead of a single vtable in which nearly every function had a big if statement in it - and also made the externally exposed types for an ECDSA key and an ECDH context different. A minor new feature: since the new arithmetic code includes a modular square root function, we can now support the compressed point representation for the NIST curves. We seem to have been getting along fine without that so far, but it seemed a shame not to put it in, since it was suddenly easy. In sshrsa.c, one major change is that I've removed the RSA blinding step in rsa_privkey_op, in which we randomise the ciphertext before doing the decryption. The purpose of that was to avoid timing leaks giving away the plaintext - but the new arithmetic code should take that in its stride in the course of also being careful enough to avoid leaking the _private key_, which RSA blinding had no way to do anything about in any case. Apart from those specific points, most of the rest of the changes are more or less mechanical, just changing type names and translating code into the new API.
2018-12-31 13:53:41 +00:00
for (size_t i = (mp_get_nbits(hostkey->modulus) + 7) / 8; i-- ;)
put_byte(hash, mp_get_byte(hostkey->modulus, i));
Complete rewrite of PuTTY's bignum library. The old 'Bignum' data type is gone completely, and so is sshbn.c. In its place is a new thing called 'mp_int', handled by an entirely new library module mpint.c, with API differences both large and small. The main aim of this change is that the new library should be free of timing- and cache-related side channels. I've written the code so that it _should_ - assuming I haven't made any mistakes - do all of its work without either control flow or memory addressing depending on the data words of the input numbers. (Though, being an _arbitrary_ precision library, it does have to at least depend on the sizes of the numbers - but there's a 'formal' size that can vary separately from the actual magnitude of the represented integer, so if you want to keep it secret that your number is actually small, it should work fine to have a very long mp_int and just happen to store 23 in it.) So I've done all my conditionalisation by means of computing both answers and doing bit-masking to swap the right one into place, and all loops over the words of an mp_int go up to the formal size rather than the actual size. I haven't actually tested the constant-time property in any rigorous way yet (I'm still considering the best way to do it). But this code is surely at the very least a big improvement on the old version, even if I later find a few more things to fix. I've also completely rewritten the low-level elliptic curve arithmetic from sshecc.c; the new ecc.c is closer to being an adjunct of mpint.c than it is to the SSH end of the code. The new elliptic curve code keeps all coordinates in Montgomery-multiplication transformed form to speed up all the multiplications mod the same prime, and only converts them back when you ask for the affine coordinates. Also, I adopted extended coordinates for the Edwards curve implementation. sshecc.c has also had a near-total rewrite in the course of switching it over to the new system. While I was there, I've separated ECDSA and EdDSA more completely - they now have separate vtables, instead of a single vtable in which nearly every function had a big if statement in it - and also made the externally exposed types for an ECDSA key and an ECDH context different. A minor new feature: since the new arithmetic code includes a modular square root function, we can now support the compressed point representation for the NIST curves. We seem to have been getting along fine without that so far, but it seemed a shame not to put it in, since it was suddenly easy. In sshrsa.c, one major change is that I've removed the RSA blinding step in rsa_privkey_op, in which we randomise the ciphertext before doing the decryption. The purpose of that was to avoid timing leaks giving away the plaintext - but the new arithmetic code should take that in its stride in the course of also being careful enough to avoid leaking the _private key_, which RSA blinding had no way to do anything about in any case. Apart from those specific points, most of the rest of the changes are more or less mechanical, just changing type names and translating code into the new API.
2018-12-31 13:53:41 +00:00
for (size_t i = (mp_get_nbits(servkey->modulus) + 7) / 8; i-- ;)
put_byte(hash, mp_get_byte(servkey->modulus, i));
put_data(hash, cookie, 8);
ssh_hash_final(hash, session_id);
Move client-specific SSH code into new files. This is a major code reorganisation in preparation for making this code base into one that can build an SSH server as well as a client. (Mostly for purposes of using the server as a regression test suite for the client, though I have some other possible uses in mind too. However, it's currently no part of my plan to harden the server to the point where it can sensibly be deployed in a hostile environment.) In this preparatory commit, I've broken up the SSH-2 transport and connection layers, and the SSH-1 connection layer, into multiple source files, with each layer having its own header file containing the shared type definitions. In each case, the new source file contains code that's specific to the client side of the protocol, so that a new file can be swapped in in its place when building the server. Mostly this is just a straightforward moving of code without changing it very much, but there are a couple of actual changes in the process: The parsing of SSH-2 global-request and channel open-messages is now done by a new pair of functions in the client module. For channel opens, I've invented a new union data type to be the return value from that function, representing either failure (plus error message), success (plus Channel instance to manage the new channel), or an instruction to hand the channel over to a sharing downstream (plus a pointer to the downstream in question). Also, the tree234 of remote port forwardings in ssh2connection is now initialised on first use by the client-specific code, so that's where its compare function lives. The shared ssh2connection_free() still takes responsibility for freeing it, but now has to check if it's non-null first. The outer shell of the ssh2_lportfwd_open method, for making a local-to-remote port forwarding, is still centralised in ssh2connection.c, but the part of it that actually constructs the outgoing channel-open message has moved into the client code, because that will have to change depending on whether the channel-open has to have type direct-tcpip or forwarded-tcpip. In the SSH-1 connection layer, half the filter_queue method has moved out into the new client-specific code, but not all of it - bidirectional channel maintenance messages are still handled centrally. One exception is SSH_MSG_PORT_OPEN, which can be sent in both directions, but with subtly different semantics - from server to client, it's referring to a previously established remote forwarding (and must be rejected if there isn't one that matches it), but from client to server it's just a "direct-tcpip" request with no prior context. So that one is in the client-specific module, and when I add the server code it will have its own different handler.
2018-10-20 16:57:37 +00:00
}
Richer data type for interactive prompt results. All the seat functions that request an interactive prompt of some kind to the user - both the main seat_get_userpass_input and the various confirmation dialogs for things like host keys - were using a simple int return value, with the general semantics of 0 = "fail", 1 = "proceed" (and in the case of seat_get_userpass_input, answers to the prompts were provided), and -1 = "request in progress, wait for a callback". In this commit I change all those functions' return types to a new struct called SeatPromptResult, whose primary field is an enum replacing those simple integer values. The main purpose is that the enum has not three but _four_ values: the "fail" result has been split into 'user abort' and 'software abort'. The distinction is that a user abort occurs as a result of an interactive UI action, such as the user clicking 'cancel' in a dialog box or hitting ^D or ^C at a terminal password prompt - and therefore, there's no need to display an error message telling the user that the interactive operation has failed, because the user already knows, because they _did_ it. 'Software abort' is from any other cause, where PuTTY is the first to know there was a problem, and has to tell the user. We already had this 'user abort' vs 'software abort' distinction in other parts of the code - the SSH backend has separate termination functions which protocol layers can call. But we assumed that any failure from an interactive prompt request fell into the 'user abort' category, which is not true. A couple of examples: if you configure a host key fingerprint in your saved session via the SSH > Host keys pane, and the server presents a host key that doesn't match it, then verify_ssh_host_key would report that the user had aborted the connection, and feel no need to tell the user what had gone wrong! Similarly, if a password provided on the command line was not accepted, then (after I fixed the semantics of that in the previous commit) the same wrong handling would occur. So now, those Seat prompt functions too can communicate whether the user or the software originated a connection abort. And in the latter case, we also provide an error message to present to the user. Result: in those two example cases (and others), error messages should no longer go missing. Implementation note: to avoid the hassle of having the error message in a SeatPromptResult being a dynamically allocated string (and hence, every recipient of one must always check whether it's non-NULL and free it on every exit path, plus being careful about copying the struct around), I've instead arranged that the structure contains a function pointer and a couple of parameters, so that the string form of the message can be constructed on demand. That way, the only users who need to free it are the ones who actually _asked_ for it in the first place, which is a much smaller set. (This is one of the rare occasions that I regret not having C++'s extra features available in this code base - a unique_ptr or shared_ptr to a string would have been just the thing here, and the compiler would have done all the hard work for me of remembering where to insert the frees!)
2021-12-28 17:52:00 +00:00
/* ----------------------------------------------------------------------
* Wrapper function to handle the abort-connection modes of a
* SeatPromptResult without a lot of verbiage at every call site.
*
* Can become ssh_sw_abort or ssh_user_close, depending on the kind of
* negative SeatPromptResult.
*/
void ssh_spr_close(Ssh *ssh, SeatPromptResult spr, const char *context)
{
if (spr.kind == SPRK_USER_ABORT) {
ssh_user_close(ssh, "User aborted at %s", context);
} else {
assert(spr.kind == SPRK_SW_ABORT);
char *err = spr_get_error_message(spr);
ssh_sw_abort(ssh, "%s", err);
sfree(err);
}
}