From b875edb6bda16cb63b3699f6644e3f1bab03e2a1 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sat, 26 Aug 2023 13:46:58 +0100
Subject: [PATCH 01/19] CHECKLST.txt: suggest writing Windows Store blurb ahead
of time.
That's two releases running I've got most of the way through the
mechanical upload processes and suddenly realised I still have a piece
of creative writing to do. A small one, but even so, there's no reason
it couldn't have been prepared a week in advance like the rest of the
announcements and changelogs. The only reason I didn't is that the
checklist didn't remind me to. Now it does.
(cherry picked from commit da550c3158a3d590ba19b2bd952e9e0111051571)
---
CHECKLST.txt | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/CHECKLST.txt b/CHECKLST.txt
index 98a3f61c..3d7093a7 100644
--- a/CHECKLST.txt
+++ b/CHECKLST.txt
@@ -162,6 +162,15 @@ Preparing to make the release
enabled), by editing prerel_version() in components/Base.mc to
return undef.
+ - Prepare some 'what's new in this release' blurb for the Windows
+ Store. This should be very brief - even briefer than the website
+ news item. Keep it to a couple of sentences in a single paragraph,
+ templated along the lines of 'X.YZ adds support for this, that and
+ the other, and fixes bugs including this and that', or 'X.YZ is a
+ bug-fix release, mostly in the area of Foo, with one important fix
+ to Bar'.
+ * Might as well check this into putty-aux too.
+
- Update the wishlist, in a local checkout:
* If there are any last-minute wishlist entries (e.g. security
vulnerabilities fixed in the new release), write entries for
@@ -255,7 +264,8 @@ locally, this is the procedure for putting it up on the web.
listing
* upload revised screenshots, if necessary
* update the URL in the "Applicable license terms" box
- + press Publish to submit that to the actual upload process
+ + press Publish or Submit (or whatever the button is called this
+ time) to submit that to the actual upload process
- Announce the release!
+ Construct a release announcement email whose message body is the
From dd2b5569cecd199ec0e0a7c9f8be110c5373c0e5 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sun, 27 Aug 2023 09:36:37 +0100
Subject: [PATCH 02/19] Remove a couple of double-typedefs.
Experimenting with different compile flags pointed out two instances
of typedefing the same name twice (though benignly, with the same
definition as well). PsocksDataSink was typedefed a couple of lines
above its struct definition and then again _with_ its struct
definition; cliloop_continue_t was typedefed in unix/platform.h and
didn't need defining again in unix/psocks.c.
(cherry picked from commit 3d3400788972183b58915e6d62ff37fe9d469083)
---
psocks.h | 4 ++--
unix/psocks.c | 3 ---
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/psocks.h b/psocks.h
index d1120a36..8ba3b2f2 100644
--- a/psocks.h
+++ b/psocks.h
@@ -6,10 +6,10 @@ typedef struct PsocksDataSink PsocksDataSink;
/* indices into PsocksDataSink arrays */
typedef enum PsocksDirection { UP, DN } PsocksDirection;
-typedef struct PsocksDataSink {
+struct PsocksDataSink {
void (*free)(PsocksDataSink *);
BinarySink *s[2];
-} PsocksDataSink;
+};
static inline void pds_free(PsocksDataSink *pds)
{ pds->free(pds); }
diff --git a/unix/psocks.c b/unix/psocks.c
index 748790b8..6b30dc16 100644
--- a/unix/psocks.c
+++ b/unix/psocks.c
@@ -160,9 +160,6 @@ static bool psocks_continue(void *ctx, bool found_any_fd,
return still_running;
}
-typedef bool (*cliloop_continue_t)(void *ctx, bool found_any_fd,
- bool ran_any_callback);
-
int main(int argc, char **argv)
{
psocks_state *ps = psocks_new(&platform);
From 6136ff821342a0dcdd8b46c0b1498a752ec5c6ad Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sun, 27 Aug 2023 09:36:42 +0100
Subject: [PATCH 03/19] CMakeLists.txt: explicitly ask for C99.
A user just reported that 0.79 doesn't build out of the box on Ubuntu
14.04 (trusty), because although its gcc (4.8.4) does _support_ C99,
it doesn't enable it without a non-default -std option. The user was
able to work around the problem by defining CMAKE_C_FLAGS=-std=gnu99,
but it would have been nicer if we'd done that automatically. Setting
CMAKE_C_STANDARD causes cmake to do so.
(This isn't a regression of 0.79 over 0.78 as far as I know; the user
in question said they had last built 0.76.)
I was surprised to find Ubuntu 14.04 still in use at all, but a quick
web search revealed that its support has been extended until next
year, so fair enough, I suppose. It's also running a cmake way older
than we support, but apparently that can be worked around via
Kitware's binary tarball downloads (which do still run on 14.04).
This is a bit unsatisfactory: I'd prefer to ask for C standards
support of _at least_ C99 level, and C11 if possible. Then I could
test for the presence of C11 features via check_c_source_compiles, and
use them opportunistically (e.g. in macro definitions). But as far as
I can see, cmake has no built-in support for asking for a standards
level of 'as new as you can get, but no older than 99'. Oh well.
(In any case, the thing I'd find most useful from C11 is _Generic, and
since that's in implementation namespace, compilers can - and do -
support it in C99 mode anyway. So it's probably fine, at least for now.)
(cherry picked from commit bd27962cd93a7ebb0098c0d6577b5d7340c7424a)
---
CMakeLists.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index b94d9b60..e6d884e3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,6 +1,8 @@
cmake_minimum_required(VERSION 3.7)
project(putty LANGUAGES C)
+set(CMAKE_C_STANDARD 99)
+
include(cmake/setup.cmake)
# Scan the docs directory first, so that when we start calling
From 73b41feba5af9980ee7318d09e744a3e749f8fd4 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sun, 3 Sep 2023 09:29:56 +0100
Subject: [PATCH 04/19] Rationalise the code that resets terminal scrollback.
Recently I encountered a CLI tool that took tens of seconds to run,
and produced no _visible_ output, but wrote ESC[0m to the terminal a
few times during its operation. (Probably by mistake. In other modes
it does print colourful messages, so I expect a 'reset colour' call
was accidentally outside the 'if' statement containing the rest of the
diagnostic it followed. Or something along those lines.)
I noticed this because every ESC[0m reset my pterm scrollback to the
bottom, which wasn't very helpful, and was unintentional on pterm's
part (as _well_ as on the part of the tool). But I can fix pterm!
At first glance the code _looked_ sensible: terminal.c contains calls
to seen_disp_event(term) whenever terminal output does something that
requires a redraw of the terminal window. Those are also the updates
that should count as 'reset scrollback on display activity'. And
ESC[0m, along with the rest of the SGR handler, correctly contained no
such call. So how did a display update happen at all?
The code was confusingly tangled up with the code that responds to
terminal activity by resetting the phase of the blinking cursor (if
any). term_reset_cblink() was calling seen_disp_event() (when surely
it should be the other way round!), and also, term_reset_cblink() was
called whenever _any_ terminal output data arrived. That combination
meant that any byte output to the terminal at all turned out to count
as display activity, whether or not it changed the screen contents.
Additionally, the other scrollback-reset flag, 'reset scrollback on
keypress', was handled by calling seen_disp_event() from the keyboard
handler. But display events and keyboard events are supposed to be
_independent_ potential causes of scrollback resets - it doesn't make
any sense to handle one by treating it as the other!
So I've reorganised the code completely:
- the seen_disp_event *flag* is now gone. Instead, the
seen_disp_event function tests the scroll_on_disp flag, and if set,
resets the scroll position immediately and sets the general
'scrollbar needs updating' flag.
- keyboard input is handled by doing exactly the same thing except
testing the scroll_on_key flag, so the two systems are properly
independent. That code calls term_schedule_update so that the
terminal will be redrawn as a result of the scroll, but doesn't
also call seen_disp_event() for the rest of the full treatment.
- the term_update code that does the scrollbar update is much
simpler, since now it only needs to test that one flag.
- I also had to set that flag explicitly in scroll() so that the
scrollbar would still be updated as a result of the scrollback size
changing. I think that must have been happening entirely by
accident before.
- term_reset_cblink is subsumed into seen_disp_event, so that only
_substantive_ display updates cause the cursor blink phase to reset
to the start of the solid period.
Result: if programs output no-op sequences like ESC[0m, or if you
press keys that don't echo, then the cursor will carry on blinking
normally, and (if you don't also have scroll_on_key set) the
scrollback won't be reset. And the code is slightly shorter than it
was before, and hopefully more sensible too.
(However, other classes of no-op activity _will_ still cause a cursor
blink phase change and a scrollback reset, such as sending a
cursor-positioning sequence that puts the cursor in the same place it
was already - even something as simple as ^M when already at the start
of the line. It might be nice to fix that, but it's much more
difficult: you'd have to either put a complicated and error-prone test
at every seen_disp_event call site, or else expensively diff the
entire visible terminal state against how it was before. And to avoid
a nondeterministic dependency on the terminal update cooldown, that
diff would have to be done at the granularity of individual control
sequences rather than a bounded number of times a second. I'd rather
not!)
(cherry picked from commit bdbd5f429c79a67cc1d5fee7f121b9fe642466ad)
(cherry-picker's note: I also had to remove the initialisation of
the removed field term->seen_disp_event from term_init(), which didn't
need doing on main because commit 74aa3cb7fb088d8 had already replaced
all the boring parts of term_init with a big memset)
---
terminal/terminal.c | 54 +++++++++++++++++++++------------------------
terminal/terminal.h | 1 -
2 files changed, 25 insertions(+), 30 deletions(-)
diff --git a/terminal/terminal.c b/terminal/terminal.c
index c5af4dc6..5c375562 100644
--- a/terminal/terminal.c
+++ b/terminal/terminal.c
@@ -1314,12 +1314,21 @@ static void term_schedule_update(Terminal *term)
}
/*
- * Call this whenever the terminal window state changes, to queue
- * an update.
+ * Call this whenever the terminal window state changes, to queue an
+ * update. This also resets the phase of cursor blinking, so that the
+ * cursor remains visible as it moves with the output, and sets a flag
+ * to indicate that if we have the 'reset scrollback on display
+ * activity' setting enabled, then we should activate it.
*/
static void seen_disp_event(Terminal *term)
{
- term->seen_disp_event = true; /* for scrollback-reset-on-activity */
+ if (term->scroll_on_disp) {
+ term->disptop = 0;
+ term->win_scrollbar_update_pending = true;
+ }
+ term->cblinker = true;
+ term->cblink_pending = false;
+ term_schedule_cblink(term);
term_schedule_update(term);
}
@@ -1354,17 +1363,6 @@ static void term_schedule_cblink(Terminal *term)
}
}
-/*
- * Call to reset cursor blinking on new output.
- */
-static void term_reset_cblink(Terminal *term)
-{
- seen_disp_event(term);
- term->cblinker = true;
- term->cblink_pending = false;
- term_schedule_cblink(term);
-}
-
/*
* Call to begin a visual bell.
*/
@@ -1531,17 +1529,10 @@ void term_update(Terminal *term)
}
if (win_setup_draw_ctx(term->win)) {
- bool need_sbar_update = term->seen_disp_event ||
- term->win_scrollbar_update_pending;
- term->win_scrollbar_update_pending = false;
- if (term->seen_disp_event && term->scroll_on_disp) {
- term->disptop = 0; /* return to main screen */
- term->seen_disp_event = false;
- need_sbar_update = true;
- }
-
- if (need_sbar_update)
+ if (term->win_scrollbar_update_pending) {
+ term->win_scrollbar_update_pending = false;
update_sbar(term);
+ }
do_paint(term);
win_set_cursor_pos(
term->win, term->curs.x, term->curs.y - term->disptop);
@@ -1574,9 +1565,10 @@ void term_seen_key_event(Terminal *term)
/*
* Reset the scrollback on keypress, if we're doing that.
*/
- if (term->scroll_on_key) {
- term->disptop = 0; /* return to main screen */
- seen_disp_event(term);
+ if (term->scroll_on_key && term->disptop != 0) {
+ term->disptop = 0;
+ term->win_scrollbar_update_pending = true;
+ term_schedule_update(term);
}
}
@@ -2042,7 +2034,6 @@ Terminal *term_init(Conf *myconf, struct unicode_data *ucsdata, TermWin *win)
term->print_job = NULL;
term->vt52_mode = false;
term->cr_lf_return = false;
- term->seen_disp_event = false;
term->mouse_is_down = 0;
term->reset_132 = false;
term->cblinker = false;
@@ -2694,6 +2685,12 @@ static void scroll(Terminal *term, int topline, int botline,
*/
if (term->disptop > -term->savelines && term->disptop < 0)
term->disptop--;
+
+ /*
+ * We've just modified the data that the terminal's
+ * scrollbar is based on, so remember to update it.
+ */
+ term->win_scrollbar_update_pending = true;
}
resizeline(term, line, term->cols);
clear_line(term, line);
@@ -7815,7 +7812,6 @@ static void term_added_data(Terminal *term, bool called_from_term_data)
{
if (!term->in_term_out) {
term->in_term_out = true;
- term_reset_cblink(term);
term_out(term, called_from_term_data);
term->in_term_out = false;
}
diff --git a/terminal/terminal.h b/terminal/terminal.h
index 87441855..d00133b9 100644
--- a/terminal/terminal.h
+++ b/terminal/terminal.h
@@ -147,7 +147,6 @@ struct terminal_tag {
long vbell_end;
bool app_cursor_keys, app_keypad_keys, vt52_mode;
bool repeat_off, srm_echo, cr_lf_return;
- bool seen_disp_event;
bool big_cursor;
bool xterm_mouse_forbidden;
From 53e7d2c024d5e760979a66649e59c9b0aadbaafa Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Fri, 22 Sep 2023 12:56:47 +0100
Subject: [PATCH 05/19] settings.c: missing 'const' in gppfont().
(cherry picked from commit cfdff822c4e41895a866c0a77f43668737ab95f4)
---
settings.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/settings.c b/settings.c
index bd7ca609..4f569377 100644
--- a/settings.c
+++ b/settings.c
@@ -146,7 +146,7 @@ static void gpps(settings_r *sesskey, const char *name, const char *def,
* format of a Filename or FontSpec is platform-dependent. So the
* platform-dependent functions MUST return some sort of value.
*/
-static void gppfont(settings_r *sesskey, char *name,
+static void gppfont(settings_r *sesskey, const char *name,
Conf *conf, int primary)
{
FontSpec *result = read_setting_fontspec(sesskey, name);
From 963aebc260e3a9ef11d44f600cde4f13142114a8 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sat, 23 Sep 2023 13:28:29 +0100
Subject: [PATCH 06/19] Tiny fixes in the SOCKS proxy code.
Just happened to jump out at me in an eyeball inspection just now. I
carefully moved all the protocol byte-value constants into a header
file with mnemonic names, but I still hard-coded SOCKS4_REPLY_VERSION
in the text of one diagnostic, and I got the wrong one of
SOCKS5_REQUEST_VERSION and SOCKS5_REPLY_VERSION at one point in the
code. Both benign (the right value was there, juste called by the
wrong name).
Also fixed some missing whitespace, in passing. (Probably the line it
was missing from had once been squashed up closer to the right margin.)
(cherry picked from commit 1cd0f1787f6c1c7d9ec0fc168ed36e54fea2c756)
---
proxy/socks4.c | 4 ++--
proxy/socks5.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/proxy/socks4.c b/proxy/socks4.c
index ac85ec05..8cd08d84 100644
--- a/proxy/socks4.c
+++ b/proxy/socks4.c
@@ -95,8 +95,8 @@ static void proxy_socks4_process_queue(ProxyNegotiator *pn)
if (data[0] != SOCKS4_REPLY_VERSION) {
pn->error = dupprintf("SOCKS proxy response contained reply "
- "version number %d (expected 0)",
- (int)data[0]);
+ "version number %d (expected %d)",
+ (int)data[0], SOCKS4_REPLY_VERSION);
crStopV;
}
diff --git a/proxy/socks5.c b/proxy/socks5.c
index 87a0bbc8..49331476 100644
--- a/proxy/socks5.c
+++ b/proxy/socks5.c
@@ -353,7 +353,7 @@ static void proxy_socks5_process_queue(ProxyNegotiator *pn)
"SOCKS 5 CHAP authentication failed");
crStopV;
}
- } else if (s->chap_attr==SOCKS5_AUTH_CHAP_ATTR_CHALLENGE) {
+ } else if (s->chap_attr == SOCKS5_AUTH_CHAP_ATTR_CHALLENGE) {
/* The CHAP challenge string. Send the response */
strbuf *response = chap_response(
make_ptrlen(s->chap_buf, s->chap_attr_len),
@@ -387,7 +387,7 @@ static void proxy_socks5_process_queue(ProxyNegotiator *pn)
* byte[] address, with variable size (see below)
* uint16 port
*/
- put_byte(pn->output, SOCKS5_REPLY_VERSION);
+ put_byte(pn->output, SOCKS5_REQUEST_VERSION);
put_byte(pn->output, SOCKS_CMD_CONNECT);
put_byte(pn->output, 0); /* reserved byte */
From 45033143769d2d3b720a1f9f1bdc8aada028fefb Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Mon, 25 Sep 2023 19:31:11 +0100
Subject: [PATCH 07/19] Add a missing seen_disp_event for ESC # 3 and friends.
These escape sequences immediately change the display of the line
they're invoked on, so they need to trigger a display update. But they
weren't, and I suppose we must have never noticed before due to the
complete confusion fixed in commit bdbd5f429c79a67.
(cherry picked from commit aa1552bc8227f70674ccf20838b141abfd170434)
---
terminal/terminal.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/terminal/terminal.c b/terminal/terminal.c
index 5c375562..af4dc5d1 100644
--- a/terminal/terminal.c
+++ b/terminal/terminal.c
@@ -4251,6 +4251,7 @@ static void term_out(Terminal *term, bool called_from_term_data)
check_line_size(term, ldata);
check_trust_status(term, ldata);
ldata->lattr = nlattr;
+ seen_disp_event(term);
break;
}
/* GZD4: G0 designate 94-set */
From bb453dd27cf1cb537a4993abff70906bc1ba3b50 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Mon, 25 Sep 2023 20:43:55 +0100
Subject: [PATCH 08/19] Further reorganisations of seen_disp_event().
Shortly after the previous commit I spotted another definitely missing
display update: if you send the byte 0x7F, aka 'destructive
backspace', then the display didn't update immediately.
That was two in a row, so I did an eyeball review of the whole
terminal state machine to the best of my ability. Found a couple more
borderline ones, but also, found that the entire VT52 sub-state-
machine had a blanket seen_disp_event which really _shouldn't_ have
been there, because half the VT52 sequences aren't actually display-
modifying updates.
To make this _slightly_ less error-prone, I've sunk a number of
seen_disp_update calls into subroutines that aren't the top-level
term_out(). For example, erase_lots(), scroll(), move() and
swap_screen() now all call seen_disp_update within themselves, so
their call sites don't all have to remember to.
There are probably further bugs after this upheaval, but I think it's
moving in generally the right direction.
(cherry picked from commit 6a6efd36aa846c581446afab8c48210cfa4300db)
---
terminal/terminal.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/terminal/terminal.c b/terminal/terminal.c
index af4dc5d1..3cdfc0d4 100644
--- a/terminal/terminal.c
+++ b/terminal/terminal.c
@@ -2556,6 +2556,8 @@ static void swap_screen(Terminal *term, int which,
*/
erase_lots(term, false, true, true);
}
+
+ seen_disp_event(term);
}
/*
@@ -2737,6 +2739,8 @@ static void scroll(Terminal *term, int topline, int botline,
}
}
}
+
+ seen_disp_event(term);
}
/*
@@ -2766,6 +2770,7 @@ static void move(Terminal *term, int x, int y, int marg_clip)
term->curs.x = x;
term->curs.y = y;
term->wrapnext = false;
+ seen_disp_event(term);
}
/*
@@ -2804,6 +2809,7 @@ static void save_cursor(Terminal *term, bool save)
term->cset_attr[term->cset] = term->save_csattr;
term->sco_acs = term->save_sco_acs;
set_erase_char(term);
+ seen_disp_event(term);
}
}
@@ -2962,6 +2968,8 @@ static void erase_lots(Terminal *term,
* application has explicitly thrown them away). */
if (erasing_lines_from_top && !(term->alt_which))
term->tempsblines = 0;
+
+ seen_disp_event(term);
}
/*
@@ -3872,6 +3880,7 @@ static void term_out(Terminal *term, bool called_from_term_data)
copy_termchar(scrlineptr(term->curs.y),
term->curs.x, &term->erase_char);
}
+ seen_disp_event(term);
} else
/* Or normal C0 controls. */
if ((c & ~0x1F) == 0 && term->termstate < DO_CTRLS) {
@@ -4136,7 +4145,6 @@ static void term_out(Terminal *term, bool called_from_term_data)
case '8': /* DECRC: restore cursor */
compatibility(VT100);
save_cursor(term, false);
- seen_disp_event(term);
break;
case '=': /* DECKPAM: Keypad application mode */
compatibility(VT100);
@@ -4810,7 +4818,6 @@ static void term_out(Terminal *term, bool called_from_term_data)
break;
case 'u': /* restore cursor */
save_cursor(term, false);
- seen_disp_event(term);
break;
case 't': /* DECSLPP: set page size - ie window height */
/*
@@ -4983,7 +4990,6 @@ static void term_out(Terminal *term, bool called_from_term_data)
scroll(term, term->marg_t, term->marg_b,
def(term->esc_args[0], 1), true);
term->wrapnext = false;
- seen_disp_event(term);
break;
case 'T': /* SD: Scroll down */
CLAMP(term->esc_args[0], term->rows);
@@ -4991,7 +4997,6 @@ static void term_out(Terminal *term, bool called_from_term_data)
scroll(term, term->marg_t, term->marg_b,
-def(term->esc_args[0], 1), true);
term->wrapnext = false;
- seen_disp_event(term);
break;
case ANSI('|', '*'): /* DECSNLS */
/*
@@ -5467,7 +5472,6 @@ static void term_out(Terminal *term, bool called_from_term_data)
break;
case VT52_ESC:
term->termstate = TOPLEVEL;
- seen_disp_event(term);
switch (c) {
case 'A':
move(term, term->curs.x, term->curs.y - 1, 1);
@@ -5531,10 +5535,12 @@ static void term_out(Terminal *term, bool called_from_term_data)
move(term, 0, 0, 0);
break;
case 'I':
- if (term->curs.y == 0)
+ if (term->curs.y == 0) {
scroll(term, 0, term->rows - 1, -1, true);
- else if (term->curs.y > 0)
+ } else if (term->curs.y > 0) {
term->curs.y--;
+ seen_disp_event(term);
+ }
term->wrapnext = false;
break;
case 'J':
@@ -5625,10 +5631,12 @@ static void term_out(Terminal *term, bool called_from_term_data)
case 'e':
/* compatibility(ATARI) */
term->cursor_on = true;
+ seen_disp_event(term);
break;
case 'f':
/* compatibility(ATARI) */
term->cursor_on = false;
+ seen_disp_event(term);
break;
/* case 'j': Save cursor position - broken on ST */
/* case 'k': Restore cursor position */
From c6013e2969ba88cb5a54b15a14615ff359bd411f Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Thu, 19 Oct 2023 18:55:04 +0100
Subject: [PATCH 09/19] Recognise and discard the APC terminal escape sequence.
I encountered an instance of this sequence in the log files from a
clang CI build. The payload text inside the wrapper was
"bk;t=1697630539879"; I don't know what the "bk" stood for, but the
second half appears to be a timestamp in milliseconds since the Unix
epoch.
I don't think there's anything we can (or should) actually _do_ with
this sequence, but I think it's useful to at least recognise it, so
that it can be conveniently discarded.
(cherry picked from commit 7b10e34b8f636008243df1a1add61c0763befb38)
---
terminal/terminal.c | 22 ++++++++++++++++++++++
terminal/terminal.h | 1 +
2 files changed, 23 insertions(+)
diff --git a/terminal/terminal.c b/terminal/terminal.c
index 3cdfc0d4..90c2880d 100644
--- a/terminal/terminal.c
+++ b/terminal/terminal.c
@@ -3206,6 +3206,13 @@ static void toggle_mode(Terminal *term, int mode, int query, bool state)
*/
static void do_osc(Terminal *term)
{
+ if (term->osc_is_apc) {
+ /* This OSC was really an APC, and we don't support that
+ * sequence at all. We only recognise it in order to ignore it
+ * and filter it out of input. */
+ return;
+ }
+
if (term->osc_w) {
while (term->osc_strlen--)
term->wordness[(unsigned char)term->osc_string[term->osc_strlen]] =
@@ -4134,6 +4141,21 @@ static void term_out(Terminal *term, bool called_from_term_data)
/* Compatibility is nasty here, xterm, linux, decterm yuk! */
compatibility(OTHER);
term->termstate = SEEN_OSC;
+ term->osc_is_apc = false;
+ term->osc_strlen = 0;
+ term->esc_args[0] = 0;
+ term->esc_nargs = 1;
+ break;
+ case '_': /* APC: application program command */
+ /* APC sequences are just a string, terminated by
+ * ST or (I've observed in practice) ^G. That is,
+ * they have the same termination convention as
+ * OSC. So we handle them by going straight into
+ * OSC_STRING state and setting a flag indicating
+ * that it's not really an OSC. */
+ compatibility(OTHER);
+ term->termstate = SEEN_OSC;
+ term->osc_is_apc = true;
term->osc_strlen = 0;
term->esc_args[0] = 0;
term->esc_nargs = 1;
diff --git a/terminal/terminal.h b/terminal/terminal.h
index d00133b9..e0361313 100644
--- a/terminal/terminal.h
+++ b/terminal/terminal.h
@@ -183,6 +183,7 @@ struct terminal_tag {
#define ANSI_QUE(x) ANSI(x,1)
#define OSC_STR_MAX 2048
+ bool osc_is_apc;
int osc_strlen;
char osc_string[OSC_STR_MAX + 1];
bool osc_w;
From 9e099151574885f3c717ac10a633a9218db8e7bb Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Fri, 24 Nov 2023 19:20:43 +0000
Subject: [PATCH 10/19] Fix check for "ext-info-s".
ssh2_scan_kexinits must check to see whether it's behaving as an SSH
client or server, in order to decide whether to look for "ext-info-s"
in the server's KEXINIT or "ext-info-c" in the client's, respectively.
This check was done by testing the pointer 'server_hostkeys' to see if
it was non-NULL. I think I must have imagined that a variable of that
name meant "the host keys we have available to offer the client, if we
are the server", as the similarly named parameter 'our_hostkeys' in
write_kexinit_lists in fact does mean. So I expected it to be non-NULL
for the server and NULL for the client, and coded accordingly.
But in fact it's used by the client: it collects host key types the
client has _seen_ from the server, in order to offer them as cross-
certification actions in the specials menu. Moreover, it's _always_
non-NULL, because in the server, it's easier to leave it present but
empty than to get rid of it.
So this code was always behaving as if it was the server, i.e. it was
looking for "ext-info-c" in the client KEXINIT. When it was in fact
the client, that test would always succeed, because we _sent_ that
KEXINIT ourselves!
But nobody ever noticed, because when we're the client, it doesn't
matter whether we saw "ext-info-c", because we don't have any reason
to send EXT_INFO from client to server. We're only concerned with
server-to-client EXT_INFO. So this embarrassing bug had no actual
effect.
---
ssh/transport2.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/ssh/transport2.c b/ssh/transport2.c
index d1c255fc..3fe358f8 100644
--- a/ssh/transport2.c
+++ b/ssh/transport2.c
@@ -956,7 +956,7 @@ struct server_hostkeys {
};
static bool ssh2_scan_kexinits(
- ptrlen client_kexinit, ptrlen server_kexinit,
+ ptrlen client_kexinit, ptrlen server_kexinit, bool we_are_server,
struct kexinit_algorithm_list kexlists[NKEXLIST],
const ssh_kex **kex_alg, const ssh_keyalg **hostkey_alg,
transport_direction *cs, transport_direction *sc,
@@ -1167,9 +1167,9 @@ static bool ssh2_scan_kexinits(
*/
{
ptrlen extinfo_advert =
- (server_hostkeys ? PTRLEN_LITERAL("ext-info-c") :
+ (we_are_server ? PTRLEN_LITERAL("ext-info-c") :
PTRLEN_LITERAL("ext-info-s"));
- ptrlen list = (server_hostkeys ? clists[KEXLIST_KEX] :
+ ptrlen list = (we_are_server ? clists[KEXLIST_KEX] :
slists[KEXLIST_KEX]);
for (ptrlen word; get_commasep_word(&list, &word) ;)
if (ptrlen_eq_ptrlen(word, extinfo_advert))
@@ -1463,7 +1463,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
if (!ssh2_scan_kexinits(
ptrlen_from_strbuf(s->client_kexinit),
- ptrlen_from_strbuf(s->server_kexinit),
+ ptrlen_from_strbuf(s->server_kexinit), s->ssc != NULL,
s->kexlists, &s->kex_alg, &s->hostkey_alg, s->cstrans,
s->sctrans, &s->warn_kex, &s->warn_hk, &s->warn_cscipher,
&s->warn_sccipher, s->ppl.ssh, NULL, &s->ignorepkt, &hks,
From f2e7086902b3605c96e54ef9c956ca7ab000010e Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sat, 18 Nov 2023 08:50:58 +0000
Subject: [PATCH 11/19] Factor out the check for ext-info-* keyword.
I'm about to want to use the same code to check for something else.
It's only a handful of lines, but even so.
Also, since the string constants are mentioned several times, this
seems like a good moment to lift them out into reusable static const
ptrlens.
---
ssh/transport2.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/ssh/transport2.c b/ssh/transport2.c
index 3fe358f8..69346b4f 100644
--- a/ssh/transport2.c
+++ b/ssh/transport2.c
@@ -27,6 +27,9 @@ const static ssh2_macalg *const buggymacs[] = {
&ssh_hmac_sha1_buggy, &ssh_hmac_sha1_96_buggy, &ssh_hmac_md5
};
+const static ptrlen ext_info_c = PTRLEN_DECL_LITERAL("ext-info-c");
+const static ptrlen ext_info_s = PTRLEN_DECL_LITERAL("ext-info-s");
+
static ssh_compressor *ssh_comp_none_init(void)
{
return NULL;
@@ -938,9 +941,9 @@ static void ssh2_write_kexinit_lists(
}
if (i == KEXLIST_KEX && first_time) {
if (our_hostkeys) /* we're the server */
- add_to_commasep(list, "ext-info-s");
+ add_to_commasep_pl(list, ext_info_s);
else /* we're the client */
- add_to_commasep(list, "ext-info-c");
+ add_to_commasep_pl(list, ext_info_c);
}
put_stringsb(pktout, list);
}
@@ -955,6 +958,14 @@ struct server_hostkeys {
size_t n, size;
};
+static bool kexinit_keyword_found(ptrlen list, ptrlen keyword)
+{
+ for (ptrlen word; get_commasep_word(&list, &word) ;)
+ if (ptrlen_eq_ptrlen(word, keyword))
+ return true;
+ return false;
+}
+
static bool ssh2_scan_kexinits(
ptrlen client_kexinit, ptrlen server_kexinit, bool we_are_server,
struct kexinit_algorithm_list kexlists[NKEXLIST],
@@ -1165,16 +1176,10 @@ static bool ssh2_scan_kexinits(
/*
* Check whether the other side advertised support for EXT_INFO.
*/
- {
- ptrlen extinfo_advert =
- (we_are_server ? PTRLEN_LITERAL("ext-info-c") :
- PTRLEN_LITERAL("ext-info-s"));
- ptrlen list = (we_are_server ? clists[KEXLIST_KEX] :
- slists[KEXLIST_KEX]);
- for (ptrlen word; get_commasep_word(&list, &word) ;)
- if (ptrlen_eq_ptrlen(word, extinfo_advert))
- *can_send_ext_info = true;
- }
+ if (kexinit_keyword_found(
+ we_are_server ? clists[KEXLIST_KEX] : slists[KEXLIST_KEX],
+ we_are_server ? ext_info_c : ext_info_s))
+ *can_send_ext_info = true;
if (server_hostkeys) {
/*
From 9fcbb86f715bc03e58921482efe663aa0c662d62 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Wed, 22 Nov 2023 08:57:54 +0000
Subject: [PATCH 12/19] Refactor confirm_weak to use SeatDialogText.
This centralises the messages for weak crypto algorithms (general, and
host keys in particular, the latter including a list of all the other
available host key types) into ssh/common.c, in much the same way as
we previously did for ordinary host key warnings.
The reason is the same too: I'm about to want to vary the text in one
of those dialog boxes, so it's convenient to start by putting it
somewhere that I can modify just once.
---
console.c | 14 +-----
proxy/sshproxy.c | 48 ++++++++++++++----
putty.h | 24 ++++-----
ssh.h | 6 +++
ssh/common.c | 73 ++++++++++++++++++++++++++++
ssh/login1.c | 2 +-
ssh/server.c | 4 +-
ssh/transport2.c | 21 ++++----
stubs/null-seat.c | 6 ++-
unix/console.c | 56 +++++++++++++--------
unix/dialog.c | 118 ++++++++++++++++++++++++---------------------
unix/platform.h | 4 +-
utils/tempseat.c | 4 +-
windows/console.c | 64 +++++++++++++-----------
windows/dialog.c | 112 +++++++++++++++++++++---------------------
windows/platform.h | 4 +-
16 files changed, 342 insertions(+), 218 deletions(-)
diff --git a/console.c b/console.c
index 62c64aff..0ecfb265 100644
--- a/console.c
+++ b/console.c
@@ -9,18 +9,6 @@
#include "misc.h"
#include "console.h"
-const char weakcrypto_msg_common_fmt[] =
- "The first %s supported by the server is\n"
- "%s, which is below the configured warning threshold.\n";
-
-const char weakhk_msg_common_fmt[] =
- "The first host key type we have stored for this server\n"
- "is %s, which is below the configured warning threshold.\n"
- "The server also provides the following types of host key\n"
- "above the threshold, which we do not have stored:\n"
- "%s\n";
-
-const char console_continue_prompt[] = "Continue with connection? (y/n) ";
const char console_abandoned_msg[] = "Connection abandoned.\n";
const SeatDialogPromptDescriptions *console_prompt_descriptions(Seat *seat)
@@ -30,6 +18,8 @@ const SeatDialogPromptDescriptions *console_prompt_descriptions(Seat *seat)
.hk_connect_once_action = "enter \"n\"",
.hk_cancel_action = "press Return",
.hk_cancel_action_Participle = "Pressing Return",
+ .weak_accept_action = "enter \"y\"",
+ .weak_cancel_action = "enter \"n\"",
};
return &descs;
}
diff --git a/proxy/sshproxy.c b/proxy/sshproxy.c
index 165c6c0a..4a797d38 100644
--- a/proxy/sshproxy.c
+++ b/proxy/sshproxy.c
@@ -430,8 +430,32 @@ static SeatPromptResult sshproxy_confirm_ssh_host_key(
return SPR_SW_ABORT("Noninteractive SSH proxy cannot confirm host key");
}
+static void sshproxy_format_seatdialogtext(strbuf *sb, SeatDialogText *text)
+{
+ for (SeatDialogTextItem *item = text->items,
+ *end = item+text->nitems; item < end; item++) {
+ switch (item->type) {
+ case SDT_SCARY_HEADING:
+ case SDT_PARA:
+ case SDT_DISPLAY:
+ put_stringz(sb, item->text);
+ put_byte(sb, '\n');
+ break;
+ case SDT_BATCH_ABORT:
+ put_stringz(sb, item->text);
+ put_byte(sb, '\n');
+ goto endloop;
+ default:
+ break;
+ }
+ }
+
+ endloop:
+ while (strbuf_chomp(sb, '\n'));
+}
+
static SeatPromptResult sshproxy_confirm_weak_crypto_primitive(
- Seat *seat, const char *algtype, const char *algname,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
SshProxy *sp = container_of(seat, SshProxy, seat);
@@ -442,22 +466,24 @@ static SeatPromptResult sshproxy_confirm_weak_crypto_primitive(
* request on to it.
*/
return seat_confirm_weak_crypto_primitive(
- wrap(sp->clientseat), algtype, algname, callback, ctx);
+ wrap(sp->clientseat), text, callback, ctx);
}
/*
* Otherwise, behave as if we're in batch mode: take the safest
* option.
*/
- sshproxy_error(sp, "First %s supported by server is %s, below warning "
- "threshold. Abandoning proxy SSH connection.",
- algtype, algname);
+ strbuf *sb = strbuf_new();
+ sshproxy_format_seatdialogtext(sb, text);
+ sshproxy_error(sp, sb->s);
+ strbuf_free(sb);
+
return SPR_SW_ABORT("Noninteractive SSH proxy cannot confirm "
"weak crypto primitive");
}
static SeatPromptResult sshproxy_confirm_weak_cached_hostkey(
- Seat *seat, const char *algname, const char *betteralgs,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
SshProxy *sp = container_of(seat, SshProxy, seat);
@@ -468,16 +494,18 @@ static SeatPromptResult sshproxy_confirm_weak_cached_hostkey(
* request on to it.
*/
return seat_confirm_weak_cached_hostkey(
- wrap(sp->clientseat), algname, betteralgs, callback, ctx);
+ wrap(sp->clientseat), text, callback, ctx);
}
/*
* Otherwise, behave as if we're in batch mode: take the safest
* option.
*/
- sshproxy_error(sp, "First host key type stored for server is %s, below "
- "warning threshold. Abandoning proxy SSH connection.",
- algname);
+ strbuf *sb = strbuf_new();
+ sshproxy_format_seatdialogtext(sb, text);
+ sshproxy_error(sp, sb->s);
+ strbuf_free(sb);
+
return SPR_SW_ABORT("Noninteractive SSH proxy cannot confirm "
"weak cached host key");
}
diff --git a/putty.h b/putty.h
index 6c5ed992..e4444c0e 100644
--- a/putty.h
+++ b/putty.h
@@ -1293,7 +1293,7 @@ struct SeatVtable {
* confirm_ssh_host_key above.
*/
SeatPromptResult (*confirm_weak_crypto_primitive)(
- Seat *seat, const char *algtype, const char *algname,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
/*
@@ -1304,11 +1304,10 @@ struct SeatVtable {
* This form is used in the case where we're using a host key
* below the warning threshold because that's the best one we have
* cached, but at least one host key algorithm *above* the
- * threshold is available that we don't have cached. 'betteralgs'
- * lists the better algorithm(s).
+ * threshold is available that we don't have cached.
*/
SeatPromptResult (*confirm_weak_cached_hostkey)(
- Seat *seat, const char *algname, const char *betteralgs,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
/*
@@ -1444,15 +1443,15 @@ static inline SeatPromptResult seat_confirm_ssh_host_key(
{ return iseat.seat->vt->confirm_ssh_host_key(
iseat.seat, h, p, ktyp, kstr, text, helpctx, cb, ctx); }
static inline SeatPromptResult seat_confirm_weak_crypto_primitive(
- InteractionReadySeat iseat, const char *atyp, const char *aname,
+ InteractionReadySeat iseat, SeatDialogText *text,
void (*cb)(void *ctx, SeatPromptResult result), void *ctx)
{ return iseat.seat->vt->confirm_weak_crypto_primitive(
- iseat.seat, atyp, aname, cb, ctx); }
+ iseat.seat, text, cb, ctx); }
static inline SeatPromptResult seat_confirm_weak_cached_hostkey(
- InteractionReadySeat iseat, const char *aname, const char *better,
+ InteractionReadySeat iseat, SeatDialogText *text,
void (*cb)(void *ctx, SeatPromptResult result), void *ctx)
{ return iseat.seat->vt->confirm_weak_cached_hostkey(
- iseat.seat, aname, better, cb, ctx); }
+ iseat.seat, text, cb, ctx); }
static inline const SeatDialogPromptDescriptions *seat_prompt_descriptions(
Seat *seat)
{ return seat->vt->prompt_descriptions(seat); }
@@ -1505,6 +1504,7 @@ struct SeatDialogPromptDescriptions {
const char *hk_accept_action;
const char *hk_connect_once_action;
const char *hk_cancel_action, *hk_cancel_action_Participle;
+ const char *weak_accept_action, *weak_cancel_action;
};
/* In the utils subdir: print a message to the Seat which can't be
@@ -1537,10 +1537,10 @@ SeatPromptResult nullseat_confirm_ssh_host_key(
char *keystr, SeatDialogText *text, HelpCtx helpctx,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
SeatPromptResult nullseat_confirm_weak_crypto_primitive(
- Seat *seat, const char *algtype, const char *algname,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
SeatPromptResult nullseat_confirm_weak_cached_hostkey(
- Seat *seat, const char *algname, const char *betteralgs,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
const SeatDialogPromptDescriptions *nullseat_prompt_descriptions(Seat *seat);
bool nullseat_is_never_utf8(Seat *seat);
@@ -1573,10 +1573,10 @@ SeatPromptResult console_confirm_ssh_host_key(
char *keystr, SeatDialogText *text, HelpCtx helpctx,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
SeatPromptResult console_confirm_weak_crypto_primitive(
- Seat *seat, const char *algtype, const char *algname,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
SeatPromptResult console_confirm_weak_cached_hostkey(
- Seat *seat, const char *algname, const char *betteralgs,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
StripCtrlChars *console_stripctrl_new(
Seat *seat, BinarySink *bs_out, SeatInteractionContext sic);
diff --git a/ssh.h b/ssh.h
index 2df0e3ed..b871ce8b 100644
--- a/ssh.h
+++ b/ssh.h
@@ -1908,6 +1908,12 @@ SeatPromptResult verify_ssh_host_key(
ssh_key *key, const char *keytype, char *keystr, const char *keydisp,
char **fingerprints, int ca_count,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
+SeatPromptResult confirm_weak_crypto_primitive(
+ InteractionReadySeat iseat, const char *algtype, const char *algname,
+ void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
+SeatPromptResult confirm_weak_cached_hostkey(
+ InteractionReadySeat iseat, const char *algname, const char **betteralgs,
+ void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
typedef struct ssh_transient_hostkey_cache ssh_transient_hostkey_cache;
ssh_transient_hostkey_cache *ssh_transient_hostkey_cache_new(void);
diff --git a/ssh/common.c b/ssh/common.c
index ad2d4632..2a1297c0 100644
--- a/ssh/common.c
+++ b/ssh/common.c
@@ -1085,6 +1085,79 @@ SeatPromptResult verify_ssh_host_key(
return toret;
}
+SeatPromptResult confirm_weak_crypto_primitive(
+ InteractionReadySeat iseat, const char *algtype, const char *algname,
+ 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 %s supported by the server is %s, "
+ "which is below the configured warning threshold.",
+ algtype, algname);
+
+ /* 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;
+}
+
/* ----------------------------------------------------------------------
* Common functions shared between SSH-1 layers.
*/
diff --git a/ssh/login1.c b/ssh/login1.c
index aa731848..88d1dd04 100644
--- a/ssh/login1.c
+++ b/ssh/login1.c
@@ -323,7 +323,7 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
/* Warn about chosen cipher if necessary. */
if (warn) {
- s->spr = seat_confirm_weak_crypto_primitive(
+ s->spr = confirm_weak_crypto_primitive(
ppl_get_iseat(&s->ppl), "cipher", cipher_string,
ssh1_login_dialog_callback, s);
crMaybeWaitUntilV(s->spr.kind != SPRK_INCOMPLETE);
diff --git a/ssh/server.c b/ssh/server.c
index 188426b3..a0aa277d 100644
--- a/ssh/server.c
+++ b/ssh/server.c
@@ -98,11 +98,11 @@ void mainchan_terminal_size(mainchan *mc, int width, int height) {}
/* Seat functions to ensure we don't get choosy about crypto - as the
* server, it's not up to us to give user warnings */
static SeatPromptResult server_confirm_weak_crypto_primitive(
- Seat *seat, const char *algtype, const char *algname,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{ return SPR_OK; }
static SeatPromptResult server_confirm_weak_cached_hostkey(
- Seat *seat, const char *algname, const char *betteralgs,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{ return SPR_OK; }
diff --git a/ssh/transport2.c b/ssh/transport2.c
index 69346b4f..053ca570 100644
--- a/ssh/transport2.c
+++ b/ssh/transport2.c
@@ -1514,7 +1514,8 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
if (s->warn_hk) {
int j, k;
- char *betteralgs;
+ const char **betteralgs = NULL;
+ size_t nbetter = 0, bettersize = 0;
/*
* Change warning box wording depending on why we chose a
@@ -1523,7 +1524,6 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
* could usefully cross-certify. Otherwise, use the same
* standard wording as any other weak crypto primitive.
*/
- betteralgs = NULL;
for (j = 0; j < s->n_uncert_hostkeys; j++) {
const struct ssh_signkey_with_user_pref_id *hktype =
&ssh2_hostkey_algs[s->uncert_hostkeys[j]];
@@ -1538,19 +1538,16 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
}
}
if (better) {
- if (betteralgs) {
- char *old_ba = betteralgs;
- betteralgs = dupcat(betteralgs, ",", hktype->alg->ssh_id);
- sfree(old_ba);
- } else {
- betteralgs = dupstr(hktype->alg->ssh_id);
- }
+ sgrowarray(betteralgs, bettersize, nbetter);
+ betteralgs[nbetter++] = hktype->alg->ssh_id;
}
}
if (betteralgs) {
/* Use the special warning prompt that lets us provide
* a list of better algorithms */
- s->spr = seat_confirm_weak_cached_hostkey(
+ sgrowarray(betteralgs, bettersize, nbetter);
+ betteralgs[nbetter] = NULL;
+ s->spr = confirm_weak_cached_hostkey(
ppl_get_iseat(&s->ppl), s->hostkey_alg->ssh_id, betteralgs,
ssh2_transport_dialog_callback, s);
sfree(betteralgs);
@@ -2389,7 +2386,7 @@ static int ca_blob_compare(void *av, void *bv)
}
/*
- * Wrapper on seat_confirm_weak_crypto_primitive(), which uses the
+ * Wrapper on confirm_weak_crypto_primitive(), which uses the
* tree234 s->weak_algorithms_consented_to to ensure we ask at most
* once about any given crypto primitive.
*/
@@ -2401,7 +2398,7 @@ static SeatPromptResult ssh2_transport_confirm_weak_crypto_primitive(
return SPR_OK;
add234(s->weak_algorithms_consented_to, (void *)alg);
- return seat_confirm_weak_crypto_primitive(
+ return confirm_weak_crypto_primitive(
ppl_get_iseat(&s->ppl), type, name, ssh2_transport_dialog_callback, s);
}
diff --git a/stubs/null-seat.c b/stubs/null-seat.c
index 37cb0f4c..2db45127 100644
--- a/stubs/null-seat.c
+++ b/stubs/null-seat.c
@@ -26,11 +26,11 @@ SeatPromptResult nullseat_confirm_ssh_host_key(
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{ return SPR_SW_ABORT("this seat can't handle interactive prompts"); }
SeatPromptResult nullseat_confirm_weak_crypto_primitive(
- Seat *seat, const char *algtype, const char *algname,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{ return SPR_SW_ABORT("this seat can't handle interactive prompts"); }
SeatPromptResult nullseat_confirm_weak_cached_hostkey(
- Seat *seat, const char *algname, const char *betteralgs,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{ return SPR_SW_ABORT("this seat can't handle interactive prompts"); }
bool nullseat_is_never_utf8(Seat *seat) { return false; }
@@ -60,6 +60,8 @@ const SeatDialogPromptDescriptions *nullseat_prompt_descriptions(Seat *seat)
.hk_connect_once_action = "",
.hk_cancel_action = "",
.hk_cancel_action_Participle = "",
+ .weak_accept_action = "",
+ .weak_cancel_action = "",
};
return &descs;
}
diff --git a/unix/console.c b/unix/console.c
index 286ecf29..ba35ccea 100644
--- a/unix/console.c
+++ b/unix/console.c
@@ -102,20 +102,18 @@ static int block_and_read(int fd, void *buf, size_t len)
return ret;
}
-SeatPromptResult console_confirm_ssh_host_key(
- Seat *seat, const char *host, int port, const char *keytype,
- char *keystr, SeatDialogText *text, HelpCtx helpctx,
- void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
+/*
+ * Helper function to print the message from a SeatDialogText. Returns
+ * the final prompt to print on the input line, or NULL if a
+ * batch-mode abort is needed. In the latter case it will have printed
+ * the abort text already.
+ */
+static const char *console_print_seatdialogtext(SeatDialogText *text)
{
- char line[32];
- struct termios cf;
const char *prompt = NULL;
-
stdio_sink errsink[1];
stdio_sink_init(errsink, stderr);
- premsg(&cf);
-
for (SeatDialogTextItem *item = text->items,
*end = item+text->nitems; item < end; item++) {
switch (item->type) {
@@ -135,8 +133,7 @@ SeatPromptResult console_confirm_ssh_host_key(
if (console_batch_mode) {
fprintf(stderr, "%s\n", item->text);
fflush(stderr);
- postmsg(&cf);
- return SPR_SW_ABORT("Cannot confirm a host key in batch mode");
+ return NULL;
}
break;
case SDT_PROMPT:
@@ -146,7 +143,26 @@ SeatPromptResult console_confirm_ssh_host_key(
break;
}
}
+
assert(prompt); /* something in the SeatDialogText should have set this */
+ return prompt;
+}
+
+SeatPromptResult console_confirm_ssh_host_key(
+ Seat *seat, const char *host, int port, const char *keytype,
+ char *keystr, SeatDialogText *text, HelpCtx helpctx,
+ void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
+{
+ char line[32];
+ struct termios cf;
+
+ premsg(&cf);
+
+ const char *prompt = console_print_seatdialogtext(text);
+ if (!prompt) {
+ postmsg(&cf);
+ return SPR_SW_ABORT("Cannot confirm a host key in batch mode");
+ }
while (true) {
fprintf(stderr,
@@ -202,23 +218,22 @@ SeatPromptResult console_confirm_ssh_host_key(
}
SeatPromptResult console_confirm_weak_crypto_primitive(
- Seat *seat, const char *algtype, const char *algname,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
char line[32];
struct termios cf;
premsg(&cf);
- fprintf(stderr, weakcrypto_msg_common_fmt, algtype, algname);
- if (console_batch_mode) {
- fputs(console_abandoned_msg, stderr);
+ const char *prompt = console_print_seatdialogtext(text);
+ if (!prompt) {
postmsg(&cf);
return SPR_SW_ABORT("Cannot confirm a weak crypto primitive "
"in batch mode");
}
- fputs(console_continue_prompt, stderr);
+ fprintf(stderr, "%s (y/n) ", prompt);
fflush(stderr);
{
@@ -244,23 +259,22 @@ SeatPromptResult console_confirm_weak_crypto_primitive(
}
SeatPromptResult console_confirm_weak_cached_hostkey(
- Seat *seat, const char *algname, const char *betteralgs,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
char line[32];
struct termios cf;
premsg(&cf);
- fprintf(stderr, weakhk_msg_common_fmt, algname, betteralgs);
- if (console_batch_mode) {
- fputs(console_abandoned_msg, stderr);
+ const char *prompt = console_print_seatdialogtext(text);
+ if (!prompt) {
postmsg(&cf);
return SPR_SW_ABORT("Cannot confirm a weak cached host key "
"in batch mode");
}
- fputs(console_continue_prompt, stderr);
+ fprintf(stderr, "%s (y/n) ", prompt);
fflush(stderr);
{
diff --git a/unix/dialog.c b/unix/dialog.c
index 5663ca92..63a94545 100644
--- a/unix/dialog.c
+++ b/unix/dialog.c
@@ -3609,10 +3609,54 @@ const SeatDialogPromptDescriptions *gtk_seat_prompt_descriptions(Seat *seat)
.hk_connect_once_action = "press \"Connect Once\"",
.hk_cancel_action = "press \"Cancel\"",
.hk_cancel_action_Participle = "Pressing \"Cancel\"",
+ .weak_accept_action = "press \"Yes\"",
+ .weak_cancel_action = "press \"No\"",
};
return &descs;
}
+/*
+ * Format a SeatDialogText into a strbuf, also adjusting the box width
+ * to cope with displayed text. Returns the dialog box title.
+ */
+static const char *gtk_format_seatdialogtext(
+ SeatDialogText *text, strbuf *dlg_text, int *width)
+{
+ const char *dlg_title = NULL;
+
+ for (SeatDialogTextItem *item = text->items,
+ *end = item + text->nitems; item < end; item++) {
+ switch (item->type) {
+ case SDT_PARA:
+ put_fmt(dlg_text, "%s\n\n", item->text);
+ break;
+ case SDT_DISPLAY: {
+ put_fmt(dlg_text, "%s\n\n", item->text);
+ int thiswidth = string_width(item->text);
+ if (*width < thiswidth)
+ *width = thiswidth;
+ break;
+ }
+ case SDT_SCARY_HEADING:
+ /* Can't change font size or weight in this context */
+ put_fmt(dlg_text, "%s\n\n", item->text);
+ break;
+ case SDT_TITLE:
+ dlg_title = item->text;
+ break;
+ default:
+ break;
+ }
+ }
+
+ /*
+ * Trim trailing newlines.
+ */
+ while (strbuf_chomp(dlg_text, '\n'));
+
+ return dlg_title;
+}
+
SeatPromptResult gtk_seat_confirm_ssh_host_key(
Seat *seat, const char *host, int port, const char *keytype,
char *keystr, SeatDialogText *text, HelpCtx helpctx,
@@ -3627,35 +3671,9 @@ SeatPromptResult gtk_seat_confirm_ssh_host_key(
button_array_hostkey, lenof(button_array_hostkey),
};
- const char *dlg_title = NULL;
- strbuf *dlg_text = strbuf_new();
int width = string_width("default dialog width determination string");
-
- for (SeatDialogTextItem *item = text->items,
- *end = item + text->nitems; item < end; item++) {
- switch (item->type) {
- case SDT_PARA:
- put_fmt(dlg_text, "%s\n\n", item->text);
- break;
- case SDT_DISPLAY: {
- put_fmt(dlg_text, "%s\n\n", item->text);
- int thiswidth = string_width(item->text);
- if (width < thiswidth)
- width = thiswidth;
- break;
- }
- case SDT_SCARY_HEADING:
- /* Can't change font size or weight in this context */
- put_fmt(dlg_text, "%s\n\n", item->text);
- break;
- case SDT_TITLE:
- dlg_title = item->text;
- break;
- default:
- break;
- }
- }
- while (strbuf_chomp(dlg_text, '\n'));
+ strbuf *dlg_text = strbuf_new();
+ const char *dlg_title = gtk_format_seatdialogtext(text, dlg_text, &width);
GtkWidget *mainwin, *msgbox;
@@ -3752,19 +3770,16 @@ static void simple_prompt_result_spr_callback(void *vctx, int result)
* below the configured 'warn' threshold).
*/
SeatPromptResult gtk_seat_confirm_weak_crypto_primitive(
- Seat *seat, const char *algtype, const char *algname,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
- static const char msg[] =
- "The first %s supported by the server is "
- "%s, which is below the configured warning threshold.\n"
- "Continue with connection?";
-
- char *text;
struct simple_prompt_result_spr_ctx *result_ctx;
GtkWidget *mainwin, *msgbox;
- text = dupprintf(msg, algtype, algname);
+ int width = string_width("Reasonably long line of text "
+ "as a width template");
+ strbuf *dlg_text = strbuf_new();
+ const char *dlg_title = gtk_format_seatdialogtext(text, dlg_text, &width);
result_ctx = snew(struct simple_prompt_result_spr_ctx);
result_ctx->callback = callback;
@@ -3774,33 +3789,26 @@ SeatPromptResult gtk_seat_confirm_weak_crypto_primitive(
mainwin = GTK_WIDGET(gtk_seat_get_window(seat));
msgbox = create_message_box(
- mainwin, "PuTTY Security Alert", text,
- string_width("Reasonably long line of text as a width template"),
- false, &buttons_yn, simple_prompt_result_spr_callback, result_ctx);
+ mainwin, dlg_title, dlg_text->s, width, false,
+ &buttons_yn, simple_prompt_result_spr_callback, result_ctx);
register_dialog(seat, result_ctx->dialog_slot, msgbox);
- sfree(text);
+ strbuf_free(dlg_text);
return SPR_INCOMPLETE;
}
SeatPromptResult gtk_seat_confirm_weak_cached_hostkey(
- Seat *seat, const char *algname, const char *betteralgs,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
- static const char msg[] =
- "The first host key type we have stored for this server\n"
- "is %s, which is below the configured warning threshold.\n"
- "The server also provides the following types of host key\n"
- "above the threshold, which we do not have stored:\n"
- "%s\n"
- "Continue with connection?";
-
- char *text;
struct simple_prompt_result_spr_ctx *result_ctx;
GtkWidget *mainwin, *msgbox;
- text = dupprintf(msg, algname, betteralgs);
+ int width = string_width("is ecdsa-nistp521, which is below the configured"
+ " warning threshold.");
+ strbuf *dlg_text = strbuf_new();
+ const char *dlg_title = gtk_format_seatdialogtext(text, dlg_text, &width);
result_ctx = snew(struct simple_prompt_result_spr_ctx);
result_ctx->callback = callback;
@@ -3810,13 +3818,11 @@ SeatPromptResult gtk_seat_confirm_weak_cached_hostkey(
mainwin = GTK_WIDGET(gtk_seat_get_window(seat));
msgbox = create_message_box(
- mainwin, "PuTTY Security Alert", text,
- string_width("is ecdsa-nistp521, which is below the configured"
- " warning threshold."),
- false, &buttons_yn, simple_prompt_result_spr_callback, result_ctx);
+ mainwin, dlg_title, dlg_text->s, width, false,
+ &buttons_yn, simple_prompt_result_spr_callback, result_ctx);
register_dialog(seat, result_ctx->dialog_slot, msgbox);
- sfree(text);
+ strbuf_free(dlg_text);
return SPR_INCOMPLETE;
}
diff --git a/unix/platform.h b/unix/platform.h
index 3b1db9ba..a6bc7ad1 100644
--- a/unix/platform.h
+++ b/unix/platform.h
@@ -225,10 +225,10 @@ SeatPromptResult gtk_seat_confirm_ssh_host_key(
char *keystr, SeatDialogText *text, HelpCtx helpctx,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
SeatPromptResult gtk_seat_confirm_weak_crypto_primitive(
- Seat *seat, const char *algtype, const char *algname,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
SeatPromptResult gtk_seat_confirm_weak_cached_hostkey(
- Seat *seat, const char *algname, const char *betteralgs,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
const SeatDialogPromptDescriptions *gtk_seat_prompt_descriptions(Seat *seat);
#ifdef MAY_REFER_TO_GTK_IN_HEADERS
diff --git a/utils/tempseat.c b/utils/tempseat.c
index ad37b0e3..39e641f9 100644
--- a/utils/tempseat.c
+++ b/utils/tempseat.c
@@ -255,7 +255,7 @@ static SeatPromptResult tempseat_confirm_ssh_host_key(
}
static SeatPromptResult tempseat_confirm_weak_crypto_primitive(
- Seat *seat, const char *algtype, const char *algname,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
unreachable("confirm_weak_crypto_primitive "
@@ -263,7 +263,7 @@ static SeatPromptResult tempseat_confirm_weak_crypto_primitive(
}
static SeatPromptResult tempseat_confirm_weak_cached_hostkey(
- Seat *seat, const char *algname, const char *betteralgs,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
unreachable("confirm_weak_cached_hostkey "
diff --git a/windows/console.c b/windows/console.c
index 2fd572b4..df10ae51 100644
--- a/windows/console.c
+++ b/windows/console.c
@@ -32,20 +32,18 @@ void console_print_error_msg(const char *prefix, const char *msg)
fflush(stderr);
}
-SeatPromptResult console_confirm_ssh_host_key(
- Seat *seat, const char *host, int port, const char *keytype,
- char *keystr, SeatDialogText *text, HelpCtx helpctx,
- void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
+/*
+ * Helper function to print the message from a SeatDialogText. Returns
+ * the final prompt to print on the input line, or NULL if a
+ * batch-mode abort is needed. In the latter case it will have printed
+ * the abort text already.
+ */
+static const char *console_print_seatdialogtext(SeatDialogText *text)
{
- HANDLE hin;
- DWORD savemode, i;
const char *prompt = NULL;
-
stdio_sink errsink[1];
stdio_sink_init(errsink, stderr);
- char line[32];
-
for (SeatDialogTextItem *item = text->items,
*end = item+text->nitems; item < end; item++) {
switch (item->type) {
@@ -65,7 +63,7 @@ SeatPromptResult console_confirm_ssh_host_key(
if (console_batch_mode) {
fprintf(stderr, "%s\n", item->text);
fflush(stderr);
- return SPR_SW_ABORT("Cannot confirm a host key in batch mode");
+ return NULL;
}
break;
case SDT_PROMPT:
@@ -76,6 +74,22 @@ SeatPromptResult console_confirm_ssh_host_key(
}
}
assert(prompt); /* something in the SeatDialogText should have set this */
+ return prompt;
+}
+
+SeatPromptResult console_confirm_ssh_host_key(
+ Seat *seat, const char *host, int port, const char *keytype,
+ char *keystr, SeatDialogText *text, HelpCtx helpctx,
+ void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
+{
+ HANDLE hin;
+ DWORD savemode, i;
+
+ const char *prompt = console_print_seatdialogtext(text);
+ if (!prompt)
+ return SPR_SW_ABORT("Cannot confirm a host key in batch mode");
+
+ char line[32];
while (true) {
fprintf(stderr,
@@ -128,23 +142,20 @@ SeatPromptResult console_confirm_ssh_host_key(
}
SeatPromptResult console_confirm_weak_crypto_primitive(
- Seat *seat, const char *algtype, const char *algname,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
HANDLE hin;
DWORD savemode, i;
- char line[32];
-
- fprintf(stderr, weakcrypto_msg_common_fmt, algtype, algname);
-
- if (console_batch_mode) {
- fputs(console_abandoned_msg, stderr);
+ const char *prompt = console_print_seatdialogtext(text);
+ if (!prompt)
return SPR_SW_ABORT("Cannot confirm a weak crypto primitive "
"in batch mode");
- }
- fputs(console_continue_prompt, stderr);
+ char line[32];
+
+ fprintf(stderr, "%s (y/n) ", prompt);
fflush(stderr);
hin = GetStdHandle(STD_INPUT_HANDLE);
@@ -163,23 +174,20 @@ SeatPromptResult console_confirm_weak_crypto_primitive(
}
SeatPromptResult console_confirm_weak_cached_hostkey(
- Seat *seat, const char *algname, const char *betteralgs,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
HANDLE hin;
DWORD savemode, i;
- char line[32];
-
- fprintf(stderr, weakhk_msg_common_fmt, algname, betteralgs);
-
- if (console_batch_mode) {
- fputs(console_abandoned_msg, stderr);
+ const char *prompt = console_print_seatdialogtext(text);
+ if (!prompt)
return SPR_SW_ABORT("Cannot confirm a weak cached host key "
"in batch mode");
- }
- fputs(console_continue_prompt, stderr);
+ char line[32];
+
+ fprintf(stderr, "%s (y/n) ", prompt);
fflush(stderr);
hin = GetStdHandle(STD_INPUT_HANDLE);
diff --git a/windows/dialog.c b/windows/dialog.c
index 5e49cca9..03d63806 100644
--- a/windows/dialog.c
+++ b/windows/dialog.c
@@ -963,6 +963,39 @@ static INT_PTR HostKeyMoreInfoProc(HWND hwnd, UINT msg, WPARAM wParam,
return 0;
}
+static const char *process_seatdialogtext(
+ strbuf *dlg_text, const char **scary_heading, SeatDialogText *text)
+{
+ const char *dlg_title = "";
+
+ for (SeatDialogTextItem *item = text->items,
+ *end = item + text->nitems; item < end; item++) {
+ switch (item->type) {
+ case SDT_PARA:
+ put_fmt(dlg_text, "%s\r\n\r\n", item->text);
+ break;
+ case SDT_DISPLAY:
+ put_fmt(dlg_text, "%s\r\n\r\n", item->text);
+ break;
+ case SDT_SCARY_HEADING:
+ assert(scary_heading != NULL && "only expect a scary heading if "
+ "the dialog has somewhere to put it");
+ *scary_heading = item->text;
+ break;
+ case SDT_TITLE:
+ dlg_title = item->text;
+ break;
+ default:
+ break;
+ }
+ }
+
+ /* Trim any trailing newlines */
+ while (strbuf_chomp(dlg_text, '\r') || strbuf_chomp(dlg_text, '\n'));
+
+ return dlg_title;
+}
+
static INT_PTR HostKeyDialogProc(HWND hwnd, UINT msg,
WPARAM wParam, LPARAM lParam, void *vctx)
{
@@ -971,32 +1004,15 @@ static INT_PTR HostKeyDialogProc(HWND hwnd, UINT msg,
switch (msg) {
case WM_INITDIALOG: {
strbuf *dlg_text = strbuf_new();
- const char *dlg_title = "";
- ctx->has_title = false;
- LPCTSTR iconid = IDI_QUESTION;
+ const char *scary_heading = NULL;
+ const char *dlg_title = process_seatdialogtext(
+ dlg_text, &scary_heading, ctx->text);
- for (SeatDialogTextItem *item = ctx->text->items,
- *end = item + ctx->text->nitems; item < end; item++) {
- switch (item->type) {
- case SDT_PARA:
- put_fmt(dlg_text, "%s\r\n\r\n", item->text);
- break;
- case SDT_DISPLAY:
- put_fmt(dlg_text, "%s\r\n\r\n", item->text);
- break;
- case SDT_SCARY_HEADING:
- SetDlgItemText(hwnd, IDC_HK_TITLE, item->text);
- iconid = IDI_WARNING;
- ctx->has_title = true;
- break;
- case SDT_TITLE:
- dlg_title = item->text;
- break;
- default:
- break;
- }
+ LPCTSTR iconid = IDI_QUESTION;
+ if (scary_heading) {
+ SetDlgItemText(hwnd, IDC_HK_TITLE, scary_heading);
+ iconid = IDI_WARNING;
}
- while (strbuf_chomp(dlg_text, '\r') || strbuf_chomp(dlg_text, '\n'));
SetDlgItemText(hwnd, IDC_HK_TEXT, dlg_text->s);
MakeDlgItemBorderless(hwnd, IDC_HK_TEXT);
@@ -1135,6 +1151,8 @@ const SeatDialogPromptDescriptions *win_seat_prompt_descriptions(Seat *seat)
.hk_connect_once_action = "press \"Connect Once\"",
.hk_cancel_action = "press \"Cancel\"",
.hk_cancel_action_Participle = "Pressing \"Cancel\"",
+ .weak_accept_action = "press \"Yes\"",
+ .weak_cancel_action = "press \"No\"",
};
return &descs;
}
@@ -1169,25 +1187,17 @@ SeatPromptResult win_seat_confirm_ssh_host_key(
* below the configured 'warn' threshold).
*/
SeatPromptResult win_seat_confirm_weak_crypto_primitive(
- Seat *seat, const char *algtype, const char *algname,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
- static const char mbtitle[] = "%s Security Alert";
- static const char msg[] =
- "The first %s supported by the server\n"
- "is %s, which is below the configured\n"
- "warning threshold.\n"
- "Do you want to continue with this connection?\n";
- char *message, *title;
- int mbret;
+ strbuf *dlg_text = strbuf_new();
+ const char *dlg_title = process_seatdialogtext(dlg_text, NULL, text);
- message = dupprintf(msg, algtype, algname);
- title = dupprintf(mbtitle, appname);
- mbret = MessageBox(NULL, message, title,
- MB_ICONWARNING | MB_YESNO | MB_DEFBUTTON2);
+ int mbret = MessageBox(NULL, dlg_text->s, dlg_title,
+ MB_ICONWARNING | MB_YESNO | MB_DEFBUTTON2);
socket_reselect_all();
- sfree(message);
- sfree(title);
+ strbuf_free(dlg_text);
+
if (mbret == IDYES)
return SPR_OK;
else
@@ -1195,27 +1205,17 @@ SeatPromptResult win_seat_confirm_weak_crypto_primitive(
}
SeatPromptResult win_seat_confirm_weak_cached_hostkey(
- Seat *seat, const char *algname, const char *betteralgs,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
{
- static const char mbtitle[] = "%s Security Alert";
- static const char msg[] =
- "The first host key type we have stored for this server\n"
- "is %s, which is below the configured warning threshold.\n"
- "The server also provides the following types of host key\n"
- "above the threshold, which we do not have stored:\n"
- "%s\n"
- "Do you want to continue with this connection?\n";
- char *message, *title;
- int mbret;
+ strbuf *dlg_text = strbuf_new();
+ const char *dlg_title = process_seatdialogtext(dlg_text, NULL, text);
- message = dupprintf(msg, algname, betteralgs);
- title = dupprintf(mbtitle, appname);
- mbret = MessageBox(NULL, message, title,
- MB_ICONWARNING | MB_YESNO | MB_DEFBUTTON2);
+ int mbret = MessageBox(NULL, dlg_text->s, dlg_title,
+ MB_ICONWARNING | MB_YESNO | MB_DEFBUTTON2);
socket_reselect_all();
- sfree(message);
- sfree(title);
+ strbuf_free(dlg_text);
+
if (mbret == IDYES)
return SPR_OK;
else
diff --git a/windows/platform.h b/windows/platform.h
index 959a207c..cd9ef989 100644
--- a/windows/platform.h
+++ b/windows/platform.h
@@ -237,10 +237,10 @@ SeatPromptResult win_seat_confirm_ssh_host_key(
char *keystr, SeatDialogText *text, HelpCtx helpctx,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
SeatPromptResult win_seat_confirm_weak_crypto_primitive(
- Seat *seat, const char *algtype, const char *algname,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
SeatPromptResult win_seat_confirm_weak_cached_hostkey(
- Seat *seat, const char *algname, const char *betteralgs,
+ Seat *seat, SeatDialogText *text,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
const SeatDialogPromptDescriptions *win_seat_prompt_descriptions(Seat *seat);
From 244be5412728a7334a2d457fbac4e0a2597165e5 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Wed, 29 Nov 2023 07:29:13 +0000
Subject: [PATCH 13/19] Support OpenSSH's new strict kex feature.
This is enabled via magic signalling keywords in the kex algorithms
list, similarly to ext-info-{c,s}. If both sides announce the
appropriate keyword, then this signals two changes to the standard SSH
protocol:
1. NEWKEYS resets packet sequence numbers: following any NEWKEYS, the
next packet sent in the same direction has sequence number zero.
2. No extraneous packets such as SSH_MSG_IGNORE are permitted during
the initial cleartext phase of the SSH protocol.
These two changes between them defeat the 'Terrapin' vulnerability,
aka CVE-2023-48795: a protocol-level exploit in which, for example, a
MITM injects a server-to-client SSH_MSG_IGNORE during the cleartext
phase, and deletes an initial segment of the server-to-client
encrypted data stream that it guesses is the right size to be the
server's SSH_MSG_EXT_INFO, so that both sides agree on the sequence
number of the _following_ server-to-client packet. In OpenSSH's
modified binary packet protocol modes this attack can go completely
undetected, and force a downgrade to (for example) SHA-1 based RSA.
(The ChaCha20/Poly1305 binary packet protocol is most vulnerable,
because it reinitialises the IV for each packet from scratch based on
the sequence number, so the keystream doesn't get out of sync.
Exploiting this in OpenSSH's ETM modes requires additional faff to
resync the keystream, and even then, the client likely sees a
corrupted SSH message at the start of the stream - but it will just
send SSH_MSG_UNIMPLEMENTED in response to that and proceed anyway. CBC
modes and standard AES SDCTR aren't vulnerable, because their MACs are
based on the plaintext rather than the ciphertext, so faking a correct
MAC on the corrupted packet requires the attacker to know what it
would decrypt to.)
---
ssh/bpp.h | 6 ++--
ssh/bpp2.c | 12 +++++--
ssh/transport2.c | 88 ++++++++++++++++++++++++++++++++++++++++++++----
ssh/transport2.h | 2 ++
4 files changed, 97 insertions(+), 11 deletions(-)
diff --git a/ssh/bpp.h b/ssh/bpp.h
index 87e7d7e7..23af5236 100644
--- a/ssh/bpp.h
+++ b/ssh/bpp.h
@@ -138,12 +138,14 @@ void ssh2_bpp_new_outgoing_crypto(
BinaryPacketProtocol *bpp,
const ssh_cipheralg *cipher, const void *ckey, const void *iv,
const ssh2_macalg *mac, bool etm_mode, const void *mac_key,
- const ssh_compression_alg *compression, bool delayed_compression);
+ const ssh_compression_alg *compression, bool delayed_compression,
+ bool reset_sequence_number);
void ssh2_bpp_new_incoming_crypto(
BinaryPacketProtocol *bpp,
const ssh_cipheralg *cipher, const void *ckey, const void *iv,
const ssh2_macalg *mac, bool etm_mode, const void *mac_key,
- const ssh_compression_alg *compression, bool delayed_compression);
+ const ssh_compression_alg *compression, bool delayed_compression,
+ bool reset_sequence_number);
/*
* A query method specific to the interface between ssh2transport and
diff --git a/ssh/bpp2.c b/ssh/bpp2.c
index e019dd2e..88003e82 100644
--- a/ssh/bpp2.c
+++ b/ssh/bpp2.c
@@ -106,7 +106,8 @@ void ssh2_bpp_new_outgoing_crypto(
BinaryPacketProtocol *bpp,
const ssh_cipheralg *cipher, const void *ckey, const void *iv,
const ssh2_macalg *mac, bool etm_mode, const void *mac_key,
- const ssh_compression_alg *compression, bool delayed_compression)
+ const ssh_compression_alg *compression, bool delayed_compression,
+ bool reset_sequence_number)
{
struct ssh2_bpp_state *s;
assert(bpp->vt == &ssh2_bpp_vtable);
@@ -150,6 +151,9 @@ void ssh2_bpp_new_outgoing_crypto(
s->out.mac = NULL;
}
+ if (reset_sequence_number)
+ s->out.sequence = 0;
+
if (delayed_compression && !s->seen_userauth_success) {
s->out.pending_compression = compression;
s->out_comp = NULL;
@@ -174,7 +178,8 @@ void ssh2_bpp_new_incoming_crypto(
BinaryPacketProtocol *bpp,
const ssh_cipheralg *cipher, const void *ckey, const void *iv,
const ssh2_macalg *mac, bool etm_mode, const void *mac_key,
- const ssh_compression_alg *compression, bool delayed_compression)
+ const ssh_compression_alg *compression, bool delayed_compression,
+ bool reset_sequence_number)
{
struct ssh2_bpp_state *s;
assert(bpp->vt == &ssh2_bpp_vtable);
@@ -231,6 +236,9 @@ void ssh2_bpp_new_incoming_crypto(
* start consuming the input data again. */
s->pending_newkeys = false;
+ if (reset_sequence_number)
+ s->in.sequence = 0;
+
/* And schedule a run of handle_input, in case there's already
* input data in the queue. */
queue_idempotent_callback(&s->bpp.ic_in_raw);
diff --git a/ssh/transport2.c b/ssh/transport2.c
index 053ca570..e60c1b30 100644
--- a/ssh/transport2.c
+++ b/ssh/transport2.c
@@ -29,6 +29,10 @@ const static ssh2_macalg *const buggymacs[] = {
const static ptrlen ext_info_c = PTRLEN_DECL_LITERAL("ext-info-c");
const static ptrlen ext_info_s = PTRLEN_DECL_LITERAL("ext-info-s");
+const static ptrlen kex_strict_c =
+ PTRLEN_DECL_LITERAL("kex-strict-c-v00@openssh.com");
+const static ptrlen kex_strict_s =
+ PTRLEN_DECL_LITERAL("kex-strict-s-v00@openssh.com");
static ssh_compressor *ssh_comp_none_init(void)
{
@@ -465,6 +469,31 @@ static bool ssh2_transport_filter_queue(struct ssh2_transport_state *s)
{
PktIn *pktin;
+ if (!s->enabled_incoming_crypto) {
+ /*
+ * Record the fact that we've seen any non-KEXINIT packet at
+ * the head of our queue.
+ *
+ * This enables us to check later that the initial incoming
+ * KEXINIT was the very first packet, if scanning the KEXINITs
+ * turns out to enable strict-kex mode.
+ */
+ PktIn *pktin = pq_peek(s->ppl.in_pq);
+ if (pktin && pktin->type != SSH2_MSG_KEXINIT)
+ s->seen_non_kexinit = true;
+
+ if (s->strict_kex) {
+ /*
+ * Also, if we're already in strict-KEX mode and haven't
+ * turned on crypto yet, don't do any actual filtering.
+ * This ensures that extraneous packets _after_ the
+ * KEXINIT will go to the main coroutine, which will
+ * complain about them.
+ */
+ return false;
+ }
+ }
+
while (1) {
if (ssh2_common_filter_queue(&s->ppl))
return true;
@@ -940,10 +969,13 @@ static void ssh2_write_kexinit_lists(
add_to_commasep_pl(list, kexlists[i].algs[j].name);
}
if (i == KEXLIST_KEX && first_time) {
- if (our_hostkeys) /* we're the server */
+ if (our_hostkeys) { /* we're the server */
add_to_commasep_pl(list, ext_info_s);
- else /* we're the client */
+ add_to_commasep_pl(list, kex_strict_s);
+ } else { /* we're the client */
add_to_commasep_pl(list, ext_info_c);
+ add_to_commasep_pl(list, kex_strict_c);
+ }
}
put_stringsb(pktout, list);
}
@@ -974,7 +1006,7 @@ static bool ssh2_scan_kexinits(
bool *warn_kex, bool *warn_hk, bool *warn_cscipher, bool *warn_sccipher,
Ssh *ssh, bool *ignore_guess_cs_packet, bool *ignore_guess_sc_packet,
struct server_hostkeys *server_hostkeys, unsigned *hkflags,
- bool *can_send_ext_info)
+ bool *can_send_ext_info, bool first_time, bool *strict_kex)
{
BinarySource client[1], server[1];
int i;
@@ -1181,6 +1213,14 @@ static bool ssh2_scan_kexinits(
we_are_server ? ext_info_c : ext_info_s))
*can_send_ext_info = true;
+ /*
+ * Check whether the other side advertised support for kex-strict.
+ */
+ if (first_time && kexinit_keyword_found(
+ we_are_server ? clists[KEXLIST_KEX] : slists[KEXLIST_KEX],
+ we_are_server ? kex_strict_c : kex_strict_s))
+ *strict_kex = true;
+
if (server_hostkeys) {
/*
* Finally, make an auxiliary pass over the server's host key
@@ -1244,10 +1284,26 @@ static void filter_outgoing_kexinit(struct ssh2_transport_state *s)
strbuf_clear(out);
ptrlen olist = get_string(osrc), ilist = get_string(isrc);
for (ptrlen oword; get_commasep_word(&olist, &oword) ;) {
+ ptrlen searchword = oword;
ptrlen ilist_copy = ilist;
+
+ /*
+ * Special case: the kex_strict keywords are
+ * asymmetrically named, so if we're contemplating
+ * including one of them in our filtered KEXINIT, we
+ * should search the other side's KEXINIT for the _other_
+ * one, not the same one.
+ */
+ if (i == KEXLIST_KEX) {
+ if (ptrlen_eq_ptrlen(oword, kex_strict_c))
+ searchword = kex_strict_s;
+ else if (ptrlen_eq_ptrlen(oword, kex_strict_s))
+ searchword = kex_strict_c;
+ }
+
bool add = false;
for (ptrlen iword; get_commasep_word(&ilist_copy, &iword) ;) {
- if (ptrlen_eq_ptrlen(oword, iword)) {
+ if (ptrlen_eq_ptrlen(searchword, iword)) {
/* Found this word in the incoming list. */
add = true;
break;
@@ -1472,11 +1528,25 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
s->kexlists, &s->kex_alg, &s->hostkey_alg, s->cstrans,
s->sctrans, &s->warn_kex, &s->warn_hk, &s->warn_cscipher,
&s->warn_sccipher, s->ppl.ssh, NULL, &s->ignorepkt, &hks,
- &s->hkflags, &s->can_send_ext_info)) {
+ &s->hkflags, &s->can_send_ext_info, !s->got_session_id,
+ &s->strict_kex)) {
sfree(hks.indices);
return; /* false means a fatal error function was called */
}
+ /*
+ * If we've just turned on strict kex mode, say so, and
+ * retrospectively fault any pre-KEXINIT extraneous packets.
+ */
+ if (!s->got_session_id && s->strict_kex) {
+ ppl_logevent("Enabling strict key exchange semantics");
+ if (s->seen_non_kexinit) {
+ ssh_proto_error(s->ppl.ssh, "Received a packet before KEXINIT "
+ "in strict-kex mode");
+ return;
+ }
+ }
+
/*
* In addition to deciding which host key we're actually going
* to use, we should make a list of the host keys offered by
@@ -1669,7 +1739,9 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
s->ppl.bpp,
s->out.cipher, cipher_key->u, cipher_iv->u,
s->out.mac, s->out.etm_mode, mac_key->u,
- s->out.comp, s->out.comp_delayed);
+ s->out.comp, s->out.comp_delayed,
+ s->strict_kex);
+ s->enabled_outgoing_crypto = true;
strbuf_free(cipher_key);
strbuf_free(cipher_iv);
@@ -1761,7 +1833,9 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
s->ppl.bpp,
s->in.cipher, cipher_key->u, cipher_iv->u,
s->in.mac, s->in.etm_mode, mac_key->u,
- s->in.comp, s->in.comp_delayed);
+ s->in.comp, s->in.comp_delayed,
+ s->strict_kex);
+ s->enabled_incoming_crypto = true;
strbuf_free(cipher_key);
strbuf_free(cipher_iv);
diff --git a/ssh/transport2.h b/ssh/transport2.h
index 204573fb..1322cf5b 100644
--- a/ssh/transport2.h
+++ b/ssh/transport2.h
@@ -202,6 +202,8 @@ struct ssh2_transport_state {
bool warned_about_no_gss_transient_hostkey;
bool got_session_id;
bool can_send_ext_info, post_newkeys_ext_info;
+ bool strict_kex, enabled_outgoing_crypto, enabled_incoming_crypto;
+ bool seen_non_kexinit;
SeatPromptResult spr;
bool guessok;
bool ignorepkt;
From 58fc33a155ad496bdcf380fa6193302240a15ae9 Mon Sep 17 00:00:00 2001
From: Jacob Nevins <jacobn@chiark.greenend.org.uk>
Date: Sun, 10 Dec 2023 23:04:54 +0000
Subject: [PATCH 14/19] Add missing flags to AES selector vtables.
They ought to have the same data as the real AES implementations they
will hand off to.
---
crypto/aes-select.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/crypto/aes-select.c b/crypto/aes-select.c
index 62b4ab01..b4daeed1 100644
--- a/crypto/aes-select.c
+++ b/crypto/aes-select.c
@@ -59,23 +59,26 @@ static ssh_cipher *aes_select(const ssh_cipheralg *alg)
__VA_ARGS__ \
}
-AES_SELECTOR_VTABLE(cbc, "aes128-cbc", "CBC", 128, );
-AES_SELECTOR_VTABLE(cbc, "aes192-cbc", "CBC", 192, );
-AES_SELECTOR_VTABLE(cbc, "aes256-cbc", "CBC", 256, );
+AES_SELECTOR_VTABLE(cbc, "aes128-cbc", "CBC", 128, .flags = SSH_CIPHER_IS_CBC);
+AES_SELECTOR_VTABLE(cbc, "aes192-cbc", "CBC", 192, .flags = SSH_CIPHER_IS_CBC);
+AES_SELECTOR_VTABLE(cbc, "aes256-cbc", "CBC", 256, .flags = SSH_CIPHER_IS_CBC);
AES_SELECTOR_VTABLE(sdctr, "aes128-ctr", "SDCTR", 128, );
AES_SELECTOR_VTABLE(sdctr, "aes192-ctr", "SDCTR", 192, );
AES_SELECTOR_VTABLE(sdctr, "aes256-ctr", "SDCTR", 256, );
AES_SELECTOR_VTABLE(gcm, "aes128-gcm@openssh.com", "GCM", 128,
- .required_mac = &ssh2_aesgcm_mac);
+ .required_mac = &ssh2_aesgcm_mac,
+ .flags = SSH_CIPHER_SEPARATE_LENGTH);
AES_SELECTOR_VTABLE(gcm, "aes256-gcm@openssh.com", "GCM", 256,
- .required_mac = &ssh2_aesgcm_mac);
+ .required_mac = &ssh2_aesgcm_mac,
+ .flags = SSH_CIPHER_SEPARATE_LENGTH);
/* 192-bit AES-GCM is included only so that testcrypt can run standard
* test vectors against it. OpenSSH doesn't define a protocol id for
* it. Hence setting its ssh2_id to NULL here, and more importantly,
* leaving it out of aesgcm_list[] below. */
AES_SELECTOR_VTABLE(gcm, NULL, "GCM", 192,
- .required_mac = &ssh2_aesgcm_mac);
+ .required_mac = &ssh2_aesgcm_mac,
+ .flags = SSH_CIPHER_SEPARATE_LENGTH);
static const ssh_cipheralg ssh_rijndael_lysator = {
/* Same as aes256_cbc, but with a different protocol ID */
@@ -84,6 +87,7 @@ static const ssh_cipheralg ssh_rijndael_lysator = {
.blksize = 16,
.real_keybits = 256,
.padded_keybytes = 256/8,
+ .flags = SSH_CIPHER_IS_CBC,
.text_name = "AES-256 CBC (dummy selector vtable)",
.extra = ssh_aes256_cbc_impls,
};
From 0b00e4ce26d89cd010e31e66fd02ac77cb982367 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Wed, 29 Nov 2023 08:50:45 +0000
Subject: [PATCH 15/19] Warn about Terrapin vulnerability for unpatched
servers.
If the KEXINIT exchange results in a vulnerable cipher mode, we now
give a warning, similar to the 'we selected a crypto primitive below
the warning threshold' one. But there's nothing we can do about it at
that point other than let the user abort the connection.
---
ssh.h | 10 +++++-
ssh/common.c | 32 +++++++++++++----
ssh/login1.c | 2 +-
ssh/transport2.c | 90 ++++++++++++++++++++++++++++++++++++++++++++----
ssh/transport2.h | 4 +++
5 files changed, 123 insertions(+), 15 deletions(-)
diff --git a/ssh.h b/ssh.h
index b871ce8b..be3a80f5 100644
--- a/ssh.h
+++ b/ssh.h
@@ -1903,6 +1903,13 @@ void add_to_commasep(strbuf *buf, const char *data);
void add_to_commasep_pl(strbuf *buf, ptrlen data);
bool get_commasep_word(ptrlen *list, ptrlen *word);
+/* Reasons why something warned by confirm_weak_crypto_primitive might
+ * be considered weak */
+typedef enum WeakCryptoReason {
+ WCR_BELOW_THRESHOLD, /* user has told us to consider it weak */
+ WCR_TERRAPIN, /* known vulnerability CVE-2023-48795 */
+} WeakCryptoReason;
+
SeatPromptResult verify_ssh_host_key(
InteractionReadySeat iseat, Conf *conf, const char *host, int port,
ssh_key *key, const char *keytype, char *keystr, const char *keydisp,
@@ -1910,7 +1917,8 @@ SeatPromptResult verify_ssh_host_key(
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
SeatPromptResult confirm_weak_crypto_primitive(
InteractionReadySeat iseat, const char *algtype, const char *algname,
- void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
+ void (*callback)(void *ctx, SeatPromptResult result), void *ctx,
+ WeakCryptoReason wcr);
SeatPromptResult confirm_weak_cached_hostkey(
InteractionReadySeat iseat, const char *algname, const char **betteralgs,
void (*callback)(void *ctx, SeatPromptResult result), void *ctx);
diff --git a/ssh/common.c b/ssh/common.c
index 2a1297c0..91a305f7 100644
--- a/ssh/common.c
+++ b/ssh/common.c
@@ -1087,7 +1087,8 @@ SeatPromptResult verify_ssh_host_key(
SeatPromptResult confirm_weak_crypto_primitive(
InteractionReadySeat iseat, const char *algtype, const char *algname,
- void (*callback)(void *ctx, SeatPromptResult result), void *ctx)
+ void (*callback)(void *ctx, SeatPromptResult result), void *ctx,
+ WeakCryptoReason wcr)
{
SeatDialogText *text = seat_dialog_text_new();
const SeatDialogPromptDescriptions *pds =
@@ -1095,11 +1096,30 @@ SeatPromptResult confirm_weak_crypto_primitive(
seat_dialog_text_append(text, SDT_TITLE, "%s Security Alert", appname);
- 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);
+ 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:
+ 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.");
+ break;
+ default:
+ unreachable("bad WeakCryptoReason");
+ }
/* In batch mode, we print the above information and then this
* abort message, and stop. */
diff --git a/ssh/login1.c b/ssh/login1.c
index 88d1dd04..808630ed 100644
--- a/ssh/login1.c
+++ b/ssh/login1.c
@@ -325,7 +325,7 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl)
if (warn) {
s->spr = confirm_weak_crypto_primitive(
ppl_get_iseat(&s->ppl), "cipher", cipher_string,
- ssh1_login_dialog_callback, s);
+ ssh1_login_dialog_callback, s, WCR_BELOW_THRESHOLD);
crMaybeWaitUntilV(s->spr.kind != SPRK_INCOMPLETE);
if (spr_is_abort(s->spr)) {
ssh_spr_close(s->ppl.ssh, s->spr, "cipher warning");
diff --git a/ssh/transport2.c b/ssh/transport2.c
index e60c1b30..748187d2 100644
--- a/ssh/transport2.c
+++ b/ssh/transport2.c
@@ -34,6 +34,11 @@ const static ptrlen kex_strict_c =
const static ptrlen kex_strict_s =
PTRLEN_DECL_LITERAL("kex-strict-s-v00@openssh.com");
+/* Pointer value to store in s->weak_algorithms_consented_to to
+ * indicate that the user has accepted the risk of the Terrapin
+ * attack */
+static const char terrapin_weakness[1];
+
static ssh_compressor *ssh_comp_none_init(void)
{
return NULL;
@@ -88,6 +93,8 @@ static void ssh2_transport_set_max_data_size(struct ssh2_transport_state *s);
static unsigned long sanitise_rekey_time(int rekey_time, unsigned long def);
static void ssh2_transport_higher_layer_packet_callback(void *context);
static void ssh2_transport_final_output(PacketProtocolLayer *ppl);
+static const char *terrapin_vulnerable(
+ bool strict_kex, const transport_direction *d);
static const PacketProtocolLayerVtable ssh2_transport_vtable = {
.free = ssh2_transport_free,
@@ -109,7 +116,7 @@ static bool ssh2_transport_timer_update(struct ssh2_transport_state *s,
unsigned long rekey_time);
static SeatPromptResult ssh2_transport_confirm_weak_crypto_primitive(
struct ssh2_transport_state *s, const char *type, const char *name,
- const void *alg);
+ const void *alg, WeakCryptoReason wcr);
static const char *const kexlist_descr[NKEXLIST] = {
"key exchange algorithm",
@@ -1574,7 +1581,8 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
if (s->warn_kex) {
s->spr = ssh2_transport_confirm_weak_crypto_primitive(
- s, "key-exchange algorithm", s->kex_alg->name, s->kex_alg);
+ s, "key-exchange algorithm", s->kex_alg->name, s->kex_alg,
+ WCR_BELOW_THRESHOLD);
crMaybeWaitUntilV(s->spr.kind != SPRK_INCOMPLETE);
if (spr_is_abort(s->spr)) {
ssh_spr_close(s->ppl.ssh, s->spr, "kex warning");
@@ -1626,7 +1634,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
* warning prompt */
s->spr = ssh2_transport_confirm_weak_crypto_primitive(
s, "host key type", s->hostkey_alg->ssh_id,
- s->hostkey_alg);
+ s->hostkey_alg, WCR_BELOW_THRESHOLD);
}
crMaybeWaitUntilV(s->spr.kind != SPRK_INCOMPLETE);
if (spr_is_abort(s->spr)) {
@@ -1638,7 +1646,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
if (s->warn_cscipher) {
s->spr = ssh2_transport_confirm_weak_crypto_primitive(
s, "client-to-server cipher", s->out.cipher->ssh2_id,
- s->out.cipher);
+ s->out.cipher, WCR_BELOW_THRESHOLD);
crMaybeWaitUntilV(s->spr.kind != SPRK_INCOMPLETE);
if (spr_is_abort(s->spr)) {
ssh_spr_close(s->ppl.ssh, s->spr, "cipher warning");
@@ -1649,7 +1657,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
if (s->warn_sccipher) {
s->spr = ssh2_transport_confirm_weak_crypto_primitive(
s, "server-to-client cipher", s->in.cipher->ssh2_id,
- s->in.cipher);
+ s->in.cipher, WCR_BELOW_THRESHOLD);
crMaybeWaitUntilV(s->spr.kind != SPRK_INCOMPLETE);
if (spr_is_abort(s->spr)) {
ssh_spr_close(s->ppl.ssh, s->spr, "cipher warning");
@@ -1657,6 +1665,44 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
}
}
+ {
+ s->terrapin.csvuln = terrapin_vulnerable(s->strict_kex, s->cstrans);
+ s->terrapin.scvuln = terrapin_vulnerable(s->strict_kex, s->sctrans);
+ s->terrapin.wcr = WCR_TERRAPIN;
+
+ if (s->terrapin.csvuln || s->terrapin.scvuln) {
+ ppl_logevent("SSH connection is vulnerable to 'Terrapin' attack "
+ "(CVE-2023-48795)");
+ }
+
+ if (s->terrapin.csvuln) {
+ s->spr = ssh2_transport_confirm_weak_crypto_primitive(
+ s, "client-to-server cipher", s->terrapin.csvuln,
+ terrapin_weakness, s->terrapin.wcr);
+ crMaybeWaitUntilV(s->spr.kind != SPRK_INCOMPLETE);
+ if (spr_is_abort(s->spr)) {
+ ssh_spr_close(s->ppl.ssh, s->spr, "vulnerability warning");
+ return;
+ }
+ }
+
+ if (s->terrapin.scvuln) {
+ s->spr = ssh2_transport_confirm_weak_crypto_primitive(
+ s, "server-to-client cipher", s->terrapin.scvuln,
+ terrapin_weakness, s->terrapin.wcr);
+ crMaybeWaitUntilV(s->spr.kind != SPRK_INCOMPLETE);
+ if (spr_is_abort(s->spr)) {
+ ssh_spr_close(s->ppl.ssh, s->spr, "vulnerability warning");
+ return;
+ }
+ }
+
+ if (s->terrapin.csvuln || s->terrapin.scvuln) {
+ ppl_logevent("Continuing despite 'Terrapin' vulnerability, "
+ "at user request");
+ }
+ }
+
/*
* If the other side has sent an initial key exchange packet that
* we must treat as a wrong guess, wait for it, and discard it.
@@ -2466,14 +2512,15 @@ static int ca_blob_compare(void *av, void *bv)
*/
static SeatPromptResult ssh2_transport_confirm_weak_crypto_primitive(
struct ssh2_transport_state *s, const char *type, const char *name,
- const void *alg)
+ const void *alg, WeakCryptoReason wcr)
{
if (find234(s->weak_algorithms_consented_to, (void *)alg, NULL))
return SPR_OK;
add234(s->weak_algorithms_consented_to, (void *)alg);
return confirm_weak_crypto_primitive(
- ppl_get_iseat(&s->ppl), type, name, ssh2_transport_dialog_callback, s);
+ ppl_get_iseat(&s->ppl), type, name, ssh2_transport_dialog_callback,
+ s, wcr);
}
static size_t ssh2_transport_queued_data_size(PacketProtocolLayer *ppl)
@@ -2492,3 +2539,32 @@ static void ssh2_transport_final_output(PacketProtocolLayer *ppl)
ssh_ppl_final_output(s->higher_layer);
}
+
+/* Check the settings for a transport direction to see if they're
+ * vulnerable to the Terrapin attack, aka CVE-2023-48795. If so,
+ * return a string describing the vulnerable thing. */
+static const char *terrapin_vulnerable(
+ bool strict_kex, const transport_direction *d)
+{
+ /*
+ * Strict kex mode eliminates the vulnerability. (That's what it's
+ * for.)
+ */
+ if (strict_kex)
+ return NULL;
+
+ /*
+ * ChaCha20-Poly1305 is vulnerable and perfectly exploitable.
+ */
+ if (d->cipher == &ssh2_chacha20_poly1305)
+ return "ChaCha20-Poly1305";
+
+ /*
+ * CBC-mode ciphers with OpenSSH's ETM modification are vulnerable
+ * and probabilistically exploitable.
+ */
+ if (d->etm_mode && (d->cipher->flags & SSH_CIPHER_IS_CBC))
+ return "a CBC-mode cipher in OpenSSH ETM mode";
+
+ return NULL;
+}
diff --git a/ssh/transport2.h b/ssh/transport2.h
index 1322cf5b..ea739df9 100644
--- a/ssh/transport2.h
+++ b/ssh/transport2.h
@@ -180,6 +180,10 @@ struct ssh2_transport_state {
int nbits, pbits;
bool warn_kex, warn_hk, warn_cscipher, warn_sccipher;
+ struct {
+ const char *csvuln, *scvuln;
+ WeakCryptoReason wcr;
+ } terrapin;
mp_int *p, *g, *e, *f;
strbuf *ebuf, *fbuf;
strbuf *kex_shared_secret;
From fdc891d17063ab26cf68c74245ab1fd9771556cb Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sun, 10 Dec 2023 15:09:50 +0000
Subject: [PATCH 16/19] Remove fatal-error reporting from scan_kexinits.
This will allow it to be called in a second circumstance where we're
trying to find out whether something _would_ have worked, so that we
never want to terminate the connection.
---
ssh/transport2.c | 83 +++++++++++++++++++++++++++++++++++++-----------
1 file changed, 64 insertions(+), 19 deletions(-)
diff --git a/ssh/transport2.c b/ssh/transport2.c
index 748187d2..0263673d 100644
--- a/ssh/transport2.c
+++ b/ssh/transport2.c
@@ -1005,13 +1005,27 @@ static bool kexinit_keyword_found(ptrlen list, ptrlen keyword)
return false;
}
-static bool ssh2_scan_kexinits(
+typedef struct ScanKexinitsResult {
+ bool success;
+
+ /* only if success is false */
+ enum {
+ SKR_INCOMPLETE,
+ SKR_UNKNOWN_ID,
+ SKR_NO_AGREEMENT,
+ } error;
+
+ const char *kind; /* what kind of thing did we fail to sort out? */
+ ptrlen desc; /* and what was it? or what was the available list? */
+} ScanKexinitsResult;
+
+static ScanKexinitsResult ssh2_scan_kexinits(
ptrlen client_kexinit, ptrlen server_kexinit, bool we_are_server,
struct kexinit_algorithm_list kexlists[NKEXLIST],
const ssh_kex **kex_alg, const ssh_keyalg **hostkey_alg,
transport_direction *cs, transport_direction *sc,
bool *warn_kex, bool *warn_hk, bool *warn_cscipher, bool *warn_sccipher,
- Ssh *ssh, bool *ignore_guess_cs_packet, bool *ignore_guess_sc_packet,
+ bool *ignore_guess_cs_packet, bool *ignore_guess_sc_packet,
struct server_hostkeys *server_hostkeys, unsigned *hkflags,
bool *can_send_ext_info, bool first_time, bool *strict_kex)
{
@@ -1040,11 +1054,10 @@ static bool ssh2_scan_kexinits(
clists[i] = get_string(client);
slists[i] = get_string(server);
if (get_err(client) || get_err(server)) {
- /* Report a better error than the spurious "Couldn't
- * agree" that we'd generate if we pressed on regardless
- * and treated the empty get_string() result as genuine */
- ssh_proto_error(ssh, "KEXINIT packet was incomplete");
- return false;
+ ScanKexinitsResult skr = {
+ .success = false, .error = SKR_INCOMPLETE,
+ };
+ return skr;
}
for (cfirst = true, clist = clists[i];
@@ -1092,10 +1105,11 @@ static bool ssh2_scan_kexinits(
* produce a reasonably useful message instead of an
* assertion failure.
*/
- ssh_sw_abort(ssh, "Selected %s \"%.*s\" does not correspond to "
- "any supported algorithm",
- kexlist_descr[i], PTRLEN_PRINTF(found));
- return false;
+ ScanKexinitsResult skr = {
+ .success = false, .error = SKR_UNKNOWN_ID,
+ .kind = kexlist_descr[i], .desc = found,
+ };
+ return skr;
}
/*
@@ -1150,9 +1164,11 @@ static bool ssh2_scan_kexinits(
/*
* Otherwise, any match failure _is_ a fatal error.
*/
- ssh_sw_abort(ssh, "Couldn't agree a %s (available: %.*s)",
- kexlist_descr[i], PTRLEN_PRINTF(slists[i]));
- return false;
+ ScanKexinitsResult skr = {
+ .success = false, .error = SKR_UNKNOWN_ID,
+ .kind = kexlist_descr[i], .desc = slists[i],
+ };
+ return skr;
}
switch (i) {
@@ -1248,7 +1264,33 @@ static bool ssh2_scan_kexinits(
}
}
- return true;
+ ScanKexinitsResult skr = { .success = true };
+ return skr;
+}
+
+static void ssh2_report_scan_kexinits_error(Ssh *ssh, ScanKexinitsResult skr)
+{
+ assert(!skr.success);
+
+ switch (skr.error) {
+ case SKR_INCOMPLETE:
+ /* Report a better error than the spurious "Couldn't
+ * agree" that we'd generate if we pressed on regardless
+ * and treated the empty get_string() result as genuine */
+ ssh_proto_error(ssh, "KEXINIT packet was incomplete");
+ break;
+ case SKR_UNKNOWN_ID:
+ ssh_sw_abort(ssh, "Selected %s \"%.*s\" does not correspond to "
+ "any supported algorithm",
+ skr.kind, PTRLEN_PRINTF(skr.desc));
+ break;
+ case SKR_NO_AGREEMENT:
+ ssh_sw_abort(ssh, "Couldn't agree a %s (available: %.*s)",
+ skr.kind, PTRLEN_PRINTF(skr.desc));
+ break;
+ default:
+ unreachable("bad ScanKexinitsResult");
+ }
}
static inline bool delay_outgoing_kexinit(struct ssh2_transport_state *s)
@@ -1529,16 +1571,19 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
{
struct server_hostkeys hks = { NULL, 0, 0 };
- if (!ssh2_scan_kexinits(
+ ScanKexinitsResult skr = ssh2_scan_kexinits(
ptrlen_from_strbuf(s->client_kexinit),
ptrlen_from_strbuf(s->server_kexinit), s->ssc != NULL,
s->kexlists, &s->kex_alg, &s->hostkey_alg, s->cstrans,
s->sctrans, &s->warn_kex, &s->warn_hk, &s->warn_cscipher,
- &s->warn_sccipher, s->ppl.ssh, NULL, &s->ignorepkt, &hks,
+ &s->warn_sccipher, NULL, &s->ignorepkt, &hks,
&s->hkflags, &s->can_send_ext_info, !s->got_session_id,
- &s->strict_kex)) {
+ &s->strict_kex);
+
+ if (!skr.success) {
sfree(hks.indices);
- return; /* false means a fatal error function was called */
+ ssh2_report_scan_kexinits_error(s->ppl.ssh, skr);
+ return; /* we just called a fatal error function */
}
/*
From b80a41d386dbfa1b095c17bd2ed001477f302d46 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sun, 10 Dec 2023 14:45:15 +0000
Subject: [PATCH 17/19] Terrapin warning: say if reconfiguration can help.
The Terrapin vulnerability affects the modified binary packet protocol
used with ChaCha20+Poly1305, and also CBC-mode ciphers in ETM mode.
It's best prevented by the new strict-kex mode, but if the server
can't handle that protocol alteration, another approach is to change
PuTTY's configuration so that it will negotiate a different algorithm.
That may not be possible either (an obvious case being if the server
has been manually configured to _only_ support vulnerable modes). But
if it is possible, then it would be nice for us to detect that and
show how to do it.
That could be a hard problem in general, but the most likely cause of
it is configuring ChaCha20 to the top of the cipher list, so that it's
selected ahead of things that aren't vulnerable. And it's reasonably
easy to do just one fantasy-renegotiation, having moved ChaCha20 down
to below the warn line, and see if that sorts it out. If it does, we
can pass on that advice to the user.
---
ssh.h | 1 +
ssh/common.c | 11 ++++
ssh/transport2.c | 138 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 150 insertions(+)
diff --git a/ssh.h b/ssh.h
index be3a80f5..bf976467 100644
--- a/ssh.h
+++ b/ssh.h
@@ -1908,6 +1908,7 @@ bool get_commasep_word(ptrlen *list, ptrlen *word);
typedef enum WeakCryptoReason {
WCR_BELOW_THRESHOLD, /* user has told us to consider it weak */
WCR_TERRAPIN, /* known vulnerability CVE-2023-48795 */
+ WCR_TERRAPIN_AVOIDABLE, /* same, but demoting ChaCha20 can avoid it */
} WeakCryptoReason;
SeatPromptResult verify_ssh_host_key(
diff --git a/ssh/common.c b/ssh/common.c
index 91a305f7..5142e102 100644
--- a/ssh/common.c
+++ b/ssh/common.c
@@ -1105,6 +1105,7 @@ SeatPromptResult confirm_weak_crypto_primitive(
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, "
@@ -1116,6 +1117,16 @@ SeatPromptResult confirm_weak_crypto_primitive(
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");
diff --git a/ssh/transport2.c b/ssh/transport2.c
index 0263673d..5dd73cfe 100644
--- a/ssh/transport2.c
+++ b/ssh/transport2.c
@@ -95,6 +95,7 @@ static void ssh2_transport_higher_layer_packet_callback(void *context);
static void ssh2_transport_final_output(PacketProtocolLayer *ppl);
static const char *terrapin_vulnerable(
bool strict_kex, const transport_direction *d);
+static bool try_to_avoid_terrapin(const struct ssh2_transport_state *s);
static const PacketProtocolLayerVtable ssh2_transport_vtable = {
.free = ssh2_transport_free,
@@ -1718,6 +1719,8 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
if (s->terrapin.csvuln || s->terrapin.scvuln) {
ppl_logevent("SSH connection is vulnerable to 'Terrapin' attack "
"(CVE-2023-48795)");
+ if (try_to_avoid_terrapin(s))
+ s->terrapin.wcr = WCR_TERRAPIN_AVOIDABLE;
}
if (s->terrapin.csvuln) {
@@ -2613,3 +2616,138 @@ static const char *terrapin_vulnerable(
return NULL;
}
+
+/*
+ * Called when we've detected that at least one transport direction
+ * has the Terrapin vulnerability.
+ *
+ * Before we report it, try to replay what would have happened if the
+ * user had reconfigured their cipher settings to demote
+ * ChaCha20+Poly1305 to below the warning threshold. If that would
+ * have avoided the vulnerability, we should say so in the dialog box.
+ *
+ * This is basically the only change in PuTTY's configuration that has
+ * a chance of avoiding the problem. Terrapin affects the modified
+ * binary packet protocol used with ChaCha20+Poly1305, and also
+ * CBC-mode ciphers in ETM mode. But PuTTY unconditionally offers the
+ * ETM mode of each MAC _after_ the non-ETM mode. So the latter case
+ * can only come up if the server has been configured to _only_ permit
+ * the ETM modes of those MACs, which means there's nothing we can do
+ * anyway.
+ */
+static bool try_to_avoid_terrapin(const struct ssh2_transport_state *s)
+{
+ bool avoidable = false;
+
+ strbuf *alt_client_kexinit = strbuf_new();
+ Conf *alt_conf = conf_copy(s->conf);
+ struct kexinit_algorithm_list alt_kexlists[NKEXLIST];
+ memset(alt_kexlists, 0, sizeof(alt_kexlists));
+
+ /*
+ * We only bother doing this if we're the client, because Uppity
+ * can't present a dialog box anyway.
+ */
+ if (s->ssc)
+ goto out;
+
+ /*
+ * Demote CIPHER_CHACHA20 to just below CIPHER_WARN, if it was
+ * previously above it. If not, don't do anything - we don't want
+ * to _promote_ it.
+ */
+ int ccp_pos_now = -1, ccp_pos_wanted = -1;
+ for (int i = 0; i < CIPHER_MAX; i++) {
+ switch (conf_get_int_int(alt_conf, CONF_ssh_cipherlist,
+ i)) {
+ case CIPHER_CHACHA20:
+ ccp_pos_now = i;
+ break;
+ case CIPHER_WARN:
+ ccp_pos_wanted = i;
+ break;
+ }
+ }
+ if (ccp_pos_now < 0 || ccp_pos_wanted < 0)
+ goto out; /* shouldn't ever happen: didn't find the two entries */
+ if (ccp_pos_now >= ccp_pos_wanted)
+ goto out; /* ChaCha20 is already demoted and it didn't help */
+ while (ccp_pos_now < ccp_pos_wanted) {
+ int cnext = conf_get_int_int(alt_conf, CONF_ssh_cipherlist,
+ ccp_pos_now + 1);
+ conf_set_int_int(alt_conf, CONF_ssh_cipherlist,
+ ccp_pos_now, cnext);
+ ccp_pos_now++;
+ }
+ conf_set_int_int(alt_conf, CONF_ssh_cipherlist,
+ ccp_pos_now + 1, CIPHER_CHACHA20);
+
+ /*
+ * Make the outgoing KEXINIT we would have made using this
+ * configuration.
+ */
+ put_byte(alt_client_kexinit, SSH2_MSG_KEXINIT);
+ put_padding(alt_client_kexinit, 16, 'x'); /* fake random padding */
+ ssh2_write_kexinit_lists(
+ BinarySink_UPCAST(alt_client_kexinit), alt_kexlists, alt_conf,
+ s->ssc, s->ppl.remote_bugs, s->savedhost, s->savedport, s->hostkey_alg,
+ s->thc, s->host_cas, s->hostkeys, s->nhostkeys, !s->got_session_id,
+ s->can_gssapi_keyex,
+ s->gss_kex_used && !s->need_gss_transient_hostkey);
+ put_bool(alt_client_kexinit, false); /* guess packet follows */
+ put_uint32(alt_client_kexinit, 0); /* reserved */
+
+ /*
+ * Re-analyse the incoming KEXINIT with respect to this one, to
+ * see what we'd have decided on.
+ */
+ transport_direction cstrans, sctrans;
+ bool warn_kex, warn_hk, warn_cscipher, warn_sccipher;
+ bool can_send_ext_info = false, strict_kex = false;
+ unsigned hkflags;
+ const ssh_kex *kex_alg;
+ const ssh_keyalg *hostkey_alg;
+
+ ScanKexinitsResult skr = ssh2_scan_kexinits(
+ ptrlen_from_strbuf(alt_client_kexinit),
+ ptrlen_from_strbuf(s->server_kexinit),
+ s->ssc != NULL, alt_kexlists, &kex_alg, &hostkey_alg,
+ &cstrans, &sctrans,
+ &warn_kex, &warn_hk, &warn_cscipher, &warn_sccipher, NULL, NULL, NULL,
+ &hkflags, &can_send_ext_info, !s->got_session_id, &strict_kex);
+ if (!skr.success) /* something else would have gone wrong */
+ goto out;
+
+ /*
+ * Reject this as an alternative solution if any of the warn flags
+ * has got worse, or if there's still anything
+ * Terrapin-vulnerable.
+ */
+ if (warn_kex > s->warn_kex)
+ goto out;
+ if (warn_hk > s->warn_hk)
+ goto out;
+ if (warn_cscipher > s->warn_cscipher)
+ goto out;
+ if (warn_sccipher > s->warn_sccipher)
+ goto out;
+ if (terrapin_vulnerable(strict_kex, &cstrans))
+ goto out;
+ if (terrapin_vulnerable(strict_kex, &sctrans))
+ goto out;
+
+ /*
+ * Success! The vulnerability could have been avoided by this Conf
+ * tweak, and we should tell the user so.
+ */
+ avoidable = true;
+
+ out:
+
+ for (size_t i = 0; i < NKEXLIST; i++)
+ sfree(alt_kexlists[i].algs);
+ strbuf_free(alt_client_kexinit);
+ conf_free(alt_conf);
+
+ return avoidable;
+}
From c14f079863ba6fb48137c7a96be7de5f7e5a8007 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Wed, 14 Sep 2022 16:04:14 +0100
Subject: [PATCH 18/19] windows/utils/registry.c: allow opening reg keys RO.
These handy wrappers on the verbose underlying Win32 registry API have
to lose some expressiveness, and one thing they lost was the ability
to open a registry key without asking for both read and write access.
This meant they couldn't be used for accessing keys not owned by the
calling user.
So far, I've only used them for accessing PuTTY's own saved data,
which means that hasn't been a problem. But I want to use them
elsewhere in an upcoming commit, so I need to fix that.
The obvious thing would be to change the meaning of the existing
'create' boolean flag so that if it's false, we also don't request
write access. The rationale would be that you're either reading or
writing, and if you're writing you want both RW access and to create
keys that don't already exist. But in fact that's not true: you do
want to set create==false and have write access in the case where
you're _deleting_ things from the key (or the whole key). So we really
do need three ways to call the wrapper function.
Rather than add another boolean field to every call site or mess about
with an 'access type' enum, I've taken an in-between route: the
underlying open_regkey_fn *function* takes a 'create' and a 'write'
flag, but at call sites, it's wrapped with a macro anyway (to append
NULL to the variadic argument list), so I've just made three macros
whose names request different access. That makes call sites marginally
_less_ verbose, while still
(cherry picked from commit 7339e00f4ac8a0e5f0e101d2a0effa18ef794034)
---
windows/platform.h | 10 +++++++---
windows/storage.c | 39 +++++++++++++++++++--------------------
windows/utils/registry.c | 10 +++++-----
3 files changed, 31 insertions(+), 28 deletions(-)
diff --git a/windows/platform.h b/windows/platform.h
index cd9ef989..aa221a88 100644
--- a/windows/platform.h
+++ b/windows/platform.h
@@ -734,9 +734,13 @@ char *get_jumplist_registry_entries(void);
#define CLIPUI_DEFAULT_INS CLIPUI_EXPLICIT
/* In utils */
-HKEY open_regkey_fn(bool create, HKEY base, const char *path, ...);
-#define open_regkey(create, base, ...) \
- open_regkey_fn(create, base, __VA_ARGS__, (const char *)NULL)
+HKEY open_regkey_fn(bool create, bool write, HKEY base, const char *path, ...);
+#define open_regkey_ro(base, ...) \
+ open_regkey_fn(false, false, base, __VA_ARGS__, (const char *)NULL)
+#define open_regkey_rw(base, ...) \
+ open_regkey_fn(false, true, base, __VA_ARGS__, (const char *)NULL)
+#define create_regkey(base, ...) \
+ open_regkey_fn(true, true, base, __VA_ARGS__, (const char *)NULL)
void close_regkey(HKEY key);
void del_regkey(HKEY key, const char *name);
char *enum_regkey(HKEY key, int index);
diff --git a/windows/storage.c b/windows/storage.c
index f2c33feb..29095939 100644
--- a/windows/storage.c
+++ b/windows/storage.c
@@ -42,7 +42,7 @@ settings_w *open_settings_w(const char *sessionname, char **errmsg)
strbuf *sb = strbuf_new();
escape_registry_key(sessionname, sb);
- HKEY sesskey = open_regkey(true, HKEY_CURRENT_USER, puttystr, sb->s);
+ HKEY sesskey = create_regkey(HKEY_CURRENT_USER, puttystr, sb->s);
if (!sesskey) {
*errmsg = dupprintf("Unable to create registry key\n"
"HKEY_CURRENT_USER\\%s\\%s", puttystr, sb->s);
@@ -85,7 +85,7 @@ settings_r *open_settings_r(const char *sessionname)
strbuf *sb = strbuf_new();
escape_registry_key(sessionname, sb);
- HKEY sesskey = open_regkey(false, HKEY_CURRENT_USER, puttystr, sb->s);
+ HKEY sesskey = open_regkey_ro(HKEY_CURRENT_USER, puttystr, sb->s);
strbuf_free(sb);
if (!sesskey)
@@ -196,7 +196,7 @@ void close_settings_r(settings_r *handle)
void del_settings(const char *sessionname)
{
- HKEY rkey = open_regkey(false, HKEY_CURRENT_USER, puttystr);
+ HKEY rkey = open_regkey_rw(HKEY_CURRENT_USER, puttystr);
if (!rkey)
return;
@@ -217,7 +217,7 @@ struct settings_e {
settings_e *enum_settings_start(void)
{
- HKEY key = open_regkey(false, HKEY_CURRENT_USER, puttystr);
+ HKEY key = open_regkey_ro(HKEY_CURRENT_USER, puttystr);
if (!key)
return NULL;
@@ -264,8 +264,8 @@ int check_stored_host_key(const char *hostname, int port,
strbuf *regname = strbuf_new();
hostkey_regname(regname, hostname, port, keytype);
- HKEY rkey = open_regkey(false, HKEY_CURRENT_USER,
- PUTTY_REG_POS "\\SshHostKeys");
+ HKEY rkey = open_regkey_ro(HKEY_CURRENT_USER,
+ PUTTY_REG_POS "\\SshHostKeys");
if (!rkey) {
strbuf_free(regname);
return 1; /* key does not exist in registry */
@@ -363,8 +363,8 @@ void store_host_key(const char *hostname, int port,
strbuf *regname = strbuf_new();
hostkey_regname(regname, hostname, port, keytype);
- HKEY rkey = open_regkey(true, HKEY_CURRENT_USER,
- PUTTY_REG_POS "\\SshHostKeys");
+ HKEY rkey = create_regkey(HKEY_CURRENT_USER,
+ PUTTY_REG_POS "\\SshHostKeys");
if (rkey) {
put_reg_sz(rkey, regname->s, key);
close_regkey(rkey);
@@ -383,7 +383,7 @@ host_ca_enum *enum_host_ca_start(void)
host_ca_enum *e;
HKEY key;
- if (!(key = open_regkey(false, HKEY_CURRENT_USER, host_ca_key)))
+ if (!(key = open_regkey_ro(HKEY_CURRENT_USER, host_ca_key)))
return NULL;
e = snew(host_ca_enum);
@@ -418,7 +418,7 @@ host_ca *host_ca_load(const char *name)
sb = strbuf_new();
escape_registry_key(name, sb);
- HKEY rkey = open_regkey(false, HKEY_CURRENT_USER, host_ca_key, sb->s);
+ HKEY rkey = open_regkey_ro(HKEY_CURRENT_USER, host_ca_key, sb->s);
strbuf_free(sb);
if (!rkey)
@@ -466,7 +466,7 @@ char *host_ca_save(host_ca *hca)
strbuf *sb = strbuf_new();
escape_registry_key(hca->name, sb);
- HKEY rkey = open_regkey(true, HKEY_CURRENT_USER, host_ca_key, sb->s);
+ HKEY rkey = create_regkey(HKEY_CURRENT_USER, host_ca_key, sb->s);
if (!rkey) {
char *err = dupprintf("Unable to create registry key\n"
"HKEY_CURRENT_USER\\%s\\%s", host_ca_key, sb->s);
@@ -495,7 +495,7 @@ char *host_ca_save(host_ca *hca)
char *host_ca_delete(const char *name)
{
- HKEY rkey = open_regkey(false, HKEY_CURRENT_USER, host_ca_key);
+ HKEY rkey = open_regkey_rw(HKEY_CURRENT_USER, host_ca_key);
if (!rkey)
return NULL;
@@ -561,7 +561,7 @@ static HANDLE access_random_seed(int action)
* Registry, if any.
*/
{
- HKEY rkey = open_regkey(false, HKEY_CURRENT_USER, PUTTY_REG_POS);
+ HKEY rkey = open_regkey_ro(HKEY_CURRENT_USER, PUTTY_REG_POS);
if (rkey) {
char *regpath = get_reg_sz(rkey, "RandSeedFile");
close_regkey(rkey);
@@ -688,7 +688,7 @@ void write_random_seed(void *data, int len)
static int transform_jumplist_registry(
const char *add, const char *rem, char **out)
{
- HKEY rkey = open_regkey(true, HKEY_CURRENT_USER, reg_jumplist_key);
+ HKEY rkey = create_regkey(HKEY_CURRENT_USER, reg_jumplist_key);
if (!rkey)
return JUMPLISTREG_ERROR_KEYOPENCREATE_FAILURE;
@@ -785,7 +785,7 @@ static void registry_recursive_remove(HKEY key)
DWORD i = 0;
while ((name = enum_regkey(key, i)) != NULL) {
- HKEY subkey = open_regkey(false, key, name);
+ HKEY subkey = open_regkey_rw(key, name);
if (subkey) {
registry_recursive_remove(subkey);
close_regkey(subkey);
@@ -816,7 +816,7 @@ void cleanup_all(void)
/*
* Open the main PuTTY registry key and remove everything in it.
*/
- HKEY key = open_regkey(false, HKEY_CURRENT_USER, PUTTY_REG_POS);
+ HKEY key = open_regkey_rw(HKEY_CURRENT_USER, PUTTY_REG_POS);
if (key) {
registry_recursive_remove(key);
close_regkey(key);
@@ -826,8 +826,7 @@ void cleanup_all(void)
* we've done that, see if the parent key has any other
* children.
*/
- if ((key = open_regkey(false, HKEY_CURRENT_USER,
- PUTTY_REG_PARENT)) != NULL) {
+ if ((key = open_regkey_rw(HKEY_CURRENT_USER, PUTTY_REG_PARENT)) != NULL) {
del_regkey(key, PUTTY_REG_PARENT_CHILD);
char *name = enum_regkey(key, 0);
close_regkey(key);
@@ -840,8 +839,8 @@ void cleanup_all(void)
if (name) {
sfree(name);
} else {
- if ((key = open_regkey(false, HKEY_CURRENT_USER,
- PUTTY_REG_GPARENT)) != NULL) {
+ if ((key = open_regkey_rw(HKEY_CURRENT_USER,
+ PUTTY_REG_GPARENT)) != NULL) {
del_regkey(key, PUTTY_REG_GPARENT_CHILD);
close_regkey(key);
}
diff --git a/windows/utils/registry.c b/windows/utils/registry.c
index 1f50e67a..9d65df1c 100644
--- a/windows/utils/registry.c
+++ b/windows/utils/registry.c
@@ -5,7 +5,7 @@
#include "putty.h"
-HKEY open_regkey_fn(bool create, HKEY hk, const char *path, ...)
+HKEY open_regkey_fn(bool create, bool write, HKEY hk, const char *path, ...)
{
HKEY toret = NULL;
bool hk_needs_close = false;
@@ -15,14 +15,14 @@ HKEY open_regkey_fn(bool create, HKEY hk, const char *path, ...)
for (; path; path = va_arg(ap, const char *)) {
HKEY hk_sub = NULL;
+ DWORD access = KEY_READ | (write ? KEY_WRITE : 0);
LONG status;
if (create)
status = RegCreateKeyEx(
hk, path, 0, NULL, REG_OPTION_NON_VOLATILE,
- KEY_READ | KEY_WRITE, NULL, &hk_sub, NULL);
+ access, NULL, &hk_sub, NULL);
else
- status = RegOpenKeyEx(
- hk, path, 0, KEY_READ | KEY_WRITE, &hk_sub);
+ status = RegOpenKeyEx(hk, path, 0, access, &hk_sub);
if (status != ERROR_SUCCESS)
goto out;
@@ -175,7 +175,7 @@ bool put_reg_multi_sz(HKEY key, const char *name, strbuf *str)
char *get_reg_sz_simple(HKEY key, const char *name, const char *leaf)
{
- HKEY subkey = open_regkey(false, key, name);
+ HKEY subkey = open_regkey_ro(key, name);
if (!subkey)
return NULL;
char *toret = get_reg_sz(subkey, leaf);
From c96fb0f10a4266513f8ebd3781550dac387d1dd1 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sat, 16 Dec 2023 13:08:16 +0000
Subject: [PATCH 19/19] Update version number for 0.80 release.
---
Buildscr | 2 +-
LATEST.VER | 2 +-
doc/plink.but | 2 +-
doc/pscp.but | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Buildscr b/Buildscr
index 826ad321..8c745510 100644
--- a/Buildscr
+++ b/Buildscr
@@ -35,7 +35,7 @@ module putty
ifeq "$(RELEASE)" "" set Ndate $(!builddate)
ifneq "$(Ndate)" "" in . do echo $(Ndate) | perl -pe 's/(....)(..)(..)/$$1-$$2-$$3/' > date
ifneq "$(Ndate)" "" read Date date
-set Epoch 18595 # update this at every release
+set Epoch 18707 # update this at every release
ifneq "$(Ndate)" "" in . do echo $(Ndate) | perl -ne 'use Time::Local; /(....)(..)(..)/ and print timegm(0,0,0,$$3,$$2-1,$$1) / 86400 - $(Epoch)' > days
ifneq "$(Ndate)" "" read Days days
diff --git a/LATEST.VER b/LATEST.VER
index 5827526e..885b0568 100644
--- a/LATEST.VER
+++ b/LATEST.VER
@@ -1 +1 @@
-0.79
+0.80
diff --git a/doc/plink.but b/doc/plink.but
index e91a295a..96d78c10 100644
--- a/doc/plink.but
+++ b/doc/plink.but
@@ -41,7 +41,7 @@ use Plink:
\c C:\>plink
\c Plink: command-line connection utility
-\c Release 0.79
+\c Release 0.80
\c Usage: plink [options] [user@]host [command]
\c ("host" can also be a PuTTY saved session name)
\c Options:
diff --git a/doc/pscp.but b/doc/pscp.but
index e788ffde..2d7f022f 100644
--- a/doc/pscp.but
+++ b/doc/pscp.but
@@ -39,7 +39,7 @@ use PSCP:
\c C:\>pscp
\c PuTTY Secure Copy client
-\c Release 0.79
+\c Release 0.80
\c Usage: pscp [options] [user@]host:source target
\c pscp [options] source [source...] [user@]host:target
\c pscp [options] -ls [user@]host:filespec