Merge ~3v1n0/ubuntu/+source/openssh:ubuntu/jammy-devel into ubuntu/+source/openssh:ubuntu/jammy-devel

Proposed by Marco Trevisan (Treviño)
Status: Merged
Merge reported by: Robie Basak
Merged at revision: 9263b6511f3d7709d071bbb3b4de0ec5384331a7
Proposed branch: ~3v1n0/ubuntu/+source/openssh:ubuntu/jammy-devel
Merge into: ubuntu/+source/openssh:ubuntu/jammy-devel
Diff against target: 140 lines (+118/-0)
3 files modified
debian/changelog (+9/-0)
debian/patches/series (+1/-0)
debian/patches/sshconnect2-Write-kbd-interactive-service-info-and-instru.patch (+108/-0)
Reviewer Review Type Date Requested Status
Vladimir Petko (community) Abstain
git-ubuntu import Pending
Review via email: mp+471761@code.launchpad.net

Description of the change

To post a comment you must log in.
Revision history for this message
Sebastien Bacher (seb128) wrote :

I've sponsored the change now, whoever has access to change the MP status can mark is as merged

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Ideally this should happen as soon as it actually gets merged in the devel branch, not sure if you can do it also through git ubuntu?

Revision history for this message
Vladimir Petko (vpa1977) wrote :

Claiming review slot to remove from the sponsoring queue.

review: Abstain

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index f4bf292..f2a65f0 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,12 @@
6+openssh (1:8.9p1-3ubuntu0.11) UNRELEASED; urgency=medium
7+
8+ * debian/patches: Properly notify keyboard interactive info and instructions.
9+ Use notifier to send keyboard interactive info and instruction messages
10+ to properly print non-ASCII chars when using PAM-based authentication.
11+ (LP: #2077576)
12+
13+ -- Marco Trevisan (Treviño) <marco@ubuntu.com> Wed, 21 Aug 2024 16:13:57 -0400
14+
15 openssh (1:8.9p1-3ubuntu0.10) jammy-security; urgency=medium
16
17 * SECURITY UPDATE: remote code execution via signal handler race
18diff --git a/debian/patches/series b/debian/patches/series
19index ffa2662..a28e95f 100644
20--- a/debian/patches/series
21+++ b/debian/patches/series
22@@ -36,3 +36,4 @@ CVE-2023-28531.patch
23 CVE-2023-51384.patch
24 CVE-2023-51385.patch
25 CVE-2024-6387.patch
26+sshconnect2-Write-kbd-interactive-service-info-and-instru.patch
27diff --git a/debian/patches/sshconnect2-Write-kbd-interactive-service-info-and-instru.patch b/debian/patches/sshconnect2-Write-kbd-interactive-service-info-and-instru.patch
28new file mode 100644
29index 0000000..497478d
30--- /dev/null
31+++ b/debian/patches/sshconnect2-Write-kbd-interactive-service-info-and-instru.patch
32@@ -0,0 +1,108 @@
33+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
34+Date: Tue, 17 Oct 2023 06:12:03 +0200
35+Subject: sshconnect2: Write kbd-interactive service,
36+ info and instructions as utf-8
37+MIME-Version: 1.0
38+Content-Type: text/plain; charset="utf-8"
39+Content-Transfer-Encoding: 8bit
40+
41+As per the previous server change now the keyboard-interactive service
42+and instruction values could be reported as soon as they are available
43+and so they're not prompts anymore and not parsed like them.
44+
45+While this was already supported by the SSH client, these messages were
46+not properly written as the escaped sequences they contained were not
47+correctly reported.
48+
49+So for example a message containing "\" was represented as "\\" and
50+similarly for all the other C escape sequences.
51+
52+This was leading to more problems when it come to utf-8 chars, as they
53+were only represented by their octal representation.
54+
55+This was easily testable by adding a line like the one below to the
56+sshd PAM service:
57+ auth requisite pam_echo.so Hello SSHD! Want some 🍕?
58+
59+Which was causing this to be written instead:
60+ Hello SSHD! Want some \360\237\215\225?
61+
62+To handle this, instead of simply using fmprintf, we're using the notifier
63+in a way can be exposed to users in the proper format and UI.
64+
65+Origin: https://github.com/openssh/openssh-portable/pull/452/commits/cc14301ce05
66+---
67+ sshconnect2.c | 33 ++++++++++++++++++++++++---------
68+ 1 file changed, 24 insertions(+), 9 deletions(-)
69+
70+diff --git a/sshconnect2.c b/sshconnect2.c
71+index 87b74ff..ec46130 100644
72+--- a/sshconnect2.c
73++++ b/sshconnect2.c
74+@@ -1243,6 +1243,7 @@ input_userauth_passwd_changereq(int type, u_int32_t seqnr, struct ssh *ssh)
75+ char *info = NULL, *lang = NULL, *password = NULL, *retype = NULL;
76+ char prompt[256];
77+ const char *host;
78++ size_t info_len;
79+ int r;
80+
81+ debug2("input_userauth_passwd_changereq");
82+@@ -1252,11 +1253,15 @@ input_userauth_passwd_changereq(int type, u_int32_t seqnr, struct ssh *ssh)
83+ "no authentication context");
84+ host = options.host_key_alias ? options.host_key_alias : authctxt->host;
85+
86+- if ((r = sshpkt_get_cstring(ssh, &info, NULL)) != 0 ||
87++ if ((r = sshpkt_get_cstring(ssh, &info, &info_len)) != 0 ||
88+ (r = sshpkt_get_cstring(ssh, &lang, NULL)) != 0)
89+ goto out;
90+- if (strlen(info) > 0)
91+- logit("%s", info);
92++ if (info_len > 0) {
93++ struct notifier_ctx *notifier = NULL;
94++ debug_f("input_userauth_passwd_changereq info: %s", info);
95++ notifier = notify_start(0, "%s", info);
96++ notify_complete(notifier, NULL);
97++ }
98+ if ((r = sshpkt_start(ssh, SSH2_MSG_USERAUTH_REQUEST)) != 0 ||
99+ (r = sshpkt_put_cstring(ssh, authctxt->server_user)) != 0 ||
100+ (r = sshpkt_put_cstring(ssh, authctxt->service)) != 0 ||
101+@@ -2095,8 +2100,10 @@ input_userauth_info_req(int type, u_int32_t seq, struct ssh *ssh)
102+ Authctxt *authctxt = ssh->authctxt;
103+ char *name = NULL, *inst = NULL, *lang = NULL, *prompt = NULL;
104+ char *display_prompt = NULL, *response = NULL;
105++ struct notifier_ctx *notifier = NULL;
106+ u_char echo = 0;
107+ u_int num_prompts, i;
108++ size_t name_len, inst_len;
109+ int r;
110+
111+ debug2_f("entering");
112+@@ -2106,14 +2113,22 @@ input_userauth_info_req(int type, u_int32_t seq, struct ssh *ssh)
113+
114+ authctxt->info_req_seen = 1;
115+
116+- if ((r = sshpkt_get_cstring(ssh, &name, NULL)) != 0 ||
117+- (r = sshpkt_get_cstring(ssh, &inst, NULL)) != 0 ||
118++ if ((r = sshpkt_get_cstring(ssh, &name, &name_len)) != 0 ||
119++ (r = sshpkt_get_cstring(ssh, &inst, &inst_len)) != 0 ||
120+ (r = sshpkt_get_cstring(ssh, &lang, NULL)) != 0)
121+ goto out;
122+- if (strlen(name) > 0)
123+- logit("%s", name);
124+- if (strlen(inst) > 0)
125+- logit("%s", inst);
126++ if (name_len > 0) {
127++ debug_f("kbd int name: %s", name);
128++ notifier = notify_start(0, "%s", name);
129++ notify_complete(notifier, NULL);
130++ notifier = NULL;
131++ }
132++ if (inst_len > 0) {
133++ debug_f("kbd int inst: %s", inst);
134++ notifier = notify_start(0, "%s", inst);
135++ notify_complete(notifier, NULL);
136++ notifier = NULL;
137++ }
138+
139+ if ((r = sshpkt_get_u32(ssh, &num_prompts)) != 0)
140+ goto out;

Subscribers

People subscribed via source and target branches