Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
home:favogt:symbolictw
openssh
0001-auth-pam-Immediately-report-instructions-t...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 0001-auth-pam-Immediately-report-instructions-to-clients-and-fix-handling-in-ssh-client.patch of Package openssh
From 7c116ef927a8ef14d09065757f75560fa0ab79d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net> Date: Tue, 17 Oct 2023 04:04:13 +0200 Subject: [PATCH 1/6] auth: Add KbdintResult definition to define result values explicitly kbdint result vfunc may return various values, so use an enum to make it clearer what each result means without having to dig into the struct documentation. --- auth-bsdauth.c | 2 +- auth-pam.c | 10 +++++----- auth.h | 5 +++++ auth2-chall.c | 4 ++-- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/auth-bsdauth.c b/auth-bsdauth.c index d124e994e77..ca41735debb 100644 --- a/auth-bsdauth.c +++ b/auth-bsdauth.c @@ -111,7 +111,7 @@ bsdauth_respond(void *ctx, u_int numresponses, char **responses) authctxt->as = NULL; debug3("bsdauth_respond: <%s> = <%d>", responses[0], authok); - return (authok == 0) ? -1 : 0; + return (authok == 0) ? KbdintResultFailure : KbdintResultSuccess; } static void diff --git a/auth-pam.c b/auth-pam.c index b49d415e7c7..86137a1acdb 100644 --- a/auth-pam.c +++ b/auth-pam.c @@ -990,15 +990,15 @@ sshpam_respond(void *ctx, u_int num, char **resp) switch (ctxt->pam_done) { case 1: sshpam_authenticated = 1; - return (0); + return KbdintResultSuccess; case 0: break; default: - return (-1); + return KbdintResultFailure; } if (num != 1) { error("PAM: expected one response, got %u", num); - return (-1); + return KbdintResultFailure; } if ((buffer = sshbuf_new()) == NULL) fatal("%s: sshbuf_new failed", __func__); @@ -1015,10 +1015,10 @@ sshpam_respond(void *ctx, u_int num, char **resp) } if (ssh_msg_send(ctxt->pam_psock, PAM_AUTHTOK, buffer) == -1) { sshbuf_free(buffer); - return (-1); + return KbdintResultFailure; } sshbuf_free(buffer); - return (1); + return KbdintResultAgain; } static void diff --git a/auth.h b/auth.h index 6d2d3976234..aac1e92d9cd 100644 --- a/auth.h +++ b/auth.h @@ -51,6 +51,7 @@ struct sshauthopt; typedef struct Authctxt Authctxt; typedef struct Authmethod Authmethod; typedef struct KbdintDevice KbdintDevice; +typedef int KbdintResult; struct Authctxt { sig_atomic_t success; @@ -111,6 +112,10 @@ struct Authmethod { # int *enabled; int (*userauth)(struct ssh *, const char *); }; +#define KbdintResultFailure -1 +#define KbdintResultSuccess 0 +#define KbdintResultAgain 1 + /* * Keyboard interactive device: * init_ctx returns: non NULL upon success diff --git a/auth2-chall.c b/auth2-chall.c index 021df829173..047d4e83c33 100644 --- a/auth2-chall.c +++ b/auth2-chall.c @@ -331,11 +331,11 @@ input_userauth_info_response(int type, u_int32_t seq, struct ssh *ssh) free(response); switch (res) { - case 0: + case KbdintResultSuccess: /* Success! */ authenticated = authctxt->valid ? 1 : 0; break; - case 1: + case KbdintResultAgain: /* Authentication needs further interaction */ if (send_userauth_info_request(ssh) == 1) authctxt->postponed = 1; From 91ef15e8ed01a7e16d96ba6cb9ed51965dca9641 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net> Date: Mon, 16 Oct 2023 21:15:45 +0200 Subject: [PATCH 2/6] auth-pam: Add an enum to define the PAM done status Makes things more readable and easier to extend --- auth-pam.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/auth-pam.c b/auth-pam.c index 86137a1acdb..21291631011 100644 --- a/auth-pam.c +++ b/auth-pam.c @@ -136,11 +136,16 @@ typedef pid_t sp_pthread_t; #define pthread_join fake_pthread_join #endif +typedef int SshPamDone; +#define SshPamError -1 +#define SshPamNone 0 +#define SshPamAuthenticated 1 + struct pam_ctxt { sp_pthread_t pam_thread; int pam_psock; int pam_csock; - int pam_done; + SshPamDone pam_done; }; static void sshpam_free_ctx(void *); @@ -904,7 +909,7 @@ sshpam_query(void *ctx, char **name, char **info, **prompts = NULL; *num = 0; **echo_on = 0; - ctxt->pam_done = -1; + ctxt->pam_done = SshPamError; free(msg); sshbuf_free(buffer); return 0; @@ -931,7 +936,7 @@ sshpam_query(void *ctx, char **name, char **info, import_environments(buffer); *num = 0; **echo_on = 0; - ctxt->pam_done = 1; + ctxt->pam_done = SshPamAuthenticated; free(msg); sshbuf_free(buffer); return (0); @@ -944,7 +949,7 @@ sshpam_query(void *ctx, char **name, char **info, *num = 0; **echo_on = 0; free(msg); - ctxt->pam_done = -1; + ctxt->pam_done = SshPamError; sshbuf_free(buffer); return (-1); } @@ -988,10 +993,10 @@ sshpam_respond(void *ctx, u_int num, char **resp) debug2("PAM: %s entering, %u responses", __func__, num); switch (ctxt->pam_done) { - case 1: + case SshPamAuthenticated: sshpam_authenticated = 1; return KbdintResultSuccess; - case 0: + case SshPamNone: break; default: return KbdintResultFailure; From 6fa8934d31cb9925c856f1b992fc5e04dd26da21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net> Date: Tue, 17 Oct 2023 04:35:17 +0200 Subject: [PATCH 3/6] auth-pam: Add debugging information when we receive PAM messages --- auth-pam.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/auth-pam.c b/auth-pam.c index 21291631011..7a72e724adc 100644 --- a/auth-pam.c +++ b/auth-pam.c @@ -450,6 +450,9 @@ sshpam_thread_conv(int n, sshpam_const struct pam_message **msg, break; case PAM_ERROR_MSG: case PAM_TEXT_INFO: + debug3("PAM: Got message of type %d: %s", + PAM_MSG_MEMBER(msg, i, msg_style), + PAM_MSG_MEMBER(msg, i, msg)); if ((r = sshbuf_put_cstring(buffer, PAM_MSG_MEMBER(msg, i, msg))) != 0) fatal("%s: buffer error: %s", From 598ee34312b541fa7b3988b4896641bf81996e27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net> Date: Tue, 17 Oct 2023 04:27:32 +0200 Subject: [PATCH 4/6] auth-pam: Immediately report interactive instructions to clients SSH keyboard-interactive authentication method supports instructions but sshd didn't show them until an user prompt was requested. This is quite inconvenient for various PAM modules that need to notify an user without requiring for their explicit input. So, properly implement RFC4256 making instructions to be shown to users when they are requested from PAM. Closes: https://bugzilla.mindrot.org/show_bug.cgi?id=2876 --- auth-pam.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/auth-pam.c b/auth-pam.c index 7a72e724adc..b756f0e5221 100644 --- a/auth-pam.c +++ b/auth-pam.c @@ -140,6 +140,7 @@ typedef int SshPamDone; #define SshPamError -1 #define SshPamNone 0 #define SshPamAuthenticated 1 +#define SshPamAgain 2 struct pam_ctxt { sp_pthread_t pam_thread; @@ -868,6 +869,8 @@ sshpam_query(void *ctx, char **name, char **info, **prompts = NULL; plen = 0; *echo_on = xmalloc(sizeof(u_int)); + ctxt->pam_done = SshPamNone; + while (ssh_msg_recv(ctxt->pam_psock, buffer) == 0) { if (++nmesg > PAM_MAX_NUM_MSG) fatal_f("too many query messages"); @@ -888,15 +891,13 @@ sshpam_query(void *ctx, char **name, char **info, return (0); case PAM_ERROR_MSG: case PAM_TEXT_INFO: - /* accumulate messages */ - len = plen + mlen + 2; - **prompts = xreallocarray(**prompts, 1, len); - strlcpy(**prompts + plen, msg, len - plen); - plen += mlen; - strlcat(**prompts + plen, "\n", len - plen); - plen++; - free(msg); - break; + *num = 0; + free(*info); + *info = msg; /* Steal the message */ + msg = NULL; + ctxt->pam_done = SshPamAgain; + sshbuf_free(buffer); + return (0); case PAM_ACCT_EXPIRED: case PAM_MAXTRIES: if (type == PAM_ACCT_EXPIRED) @@ -1001,6 +1002,8 @@ sshpam_respond(void *ctx, u_int num, char **resp) return KbdintResultSuccess; case SshPamNone: break; + case SshPamAgain: + return KbdintResultAgain; default: return KbdintResultFailure; } From cc14301ce0542cdbb825eff8041ce98a1da9ef08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net> Date: Tue, 17 Oct 2023 06:12:03 +0200 Subject: [PATCH 5/6] sshconnect2: Write kbd-interactive service, info and instructions as utf-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As per the previous server change now the keyboard-interactive service and instruction values could be reported as soon as they are available and so they're not prompts anymore and not parsed like them. While this was already supported by the SSH client, these messages were not properly written as the escaped sequences they contained were not correctly reported. So for example a message containing "\" was represented as "\\" and similarly for all the other C escape sequences. This was leading to more problems when it come to utf-8 chars, as they were only represented by their octal representation. This was easily testable by adding a line like the one below to the sshd PAM service: auth requisite pam_echo.so Hello SSHD! Want some 🍕? Which was causing this to be written instead: Hello SSHD! Want some \360\237\215\225? To handle this, instead of simply using fmprintf, we're using the notifier in a way can be exposed to users in the proper format and UI. --- sshconnect2.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/sshconnect2.c b/sshconnect2.c index 5831a00c6d1..543431218c1 100644 --- a/sshconnect2.c +++ b/sshconnect2.c @@ -1091,6 +1091,7 @@ input_userauth_passwd_changereq(int type, u_int32_t seqnr, struct ssh *ssh) char *info = NULL, *lang = NULL, *password = NULL, *retype = NULL; char prompt[256]; const char *host; + size_t info_len; int r; debug2("input_userauth_passwd_changereq"); @@ -1100,11 +1101,15 @@ input_userauth_passwd_changereq(int type, u_int32_t seqnr, struct ssh *ssh) "no authentication context"); host = options.host_key_alias ? options.host_key_alias : authctxt->host; - if ((r = sshpkt_get_cstring(ssh, &info, NULL)) != 0 || + if ((r = sshpkt_get_cstring(ssh, &info, &info_len)) != 0 || (r = sshpkt_get_cstring(ssh, &lang, NULL)) != 0) goto out; - if (strlen(info) > 0) - logit("%s", info); + if (info_len > 0) { + struct notifier_ctx *notifier = NULL; + debug_f("input_userauth_passwd_changereq info: %s", info); + notifier = notify_start(0, "%s", info); + notify_complete(notifier, NULL); + } if ((r = sshpkt_start(ssh, SSH2_MSG_USERAUTH_REQUEST)) != 0 || (r = sshpkt_put_cstring(ssh, authctxt->server_user)) != 0 || (r = sshpkt_put_cstring(ssh, authctxt->service)) != 0 || @@ -1938,8 +1943,10 @@ input_userauth_info_req(int type, u_int32_t seq, struct ssh *ssh) Authctxt *authctxt = ssh->authctxt; char *name = NULL, *inst = NULL, *lang = NULL, *prompt = NULL; char *display_prompt = NULL, *response = NULL; + struct notifier_ctx *notifier = NULL; u_char echo = 0; u_int num_prompts, i; + size_t name_len, inst_len; int r; debug2_f("entering"); @@ -1949,14 +1956,22 @@ input_userauth_info_req(int type, u_int32_t seq, struct ssh *ssh) authctxt->info_req_seen = 1; - if ((r = sshpkt_get_cstring(ssh, &name, NULL)) != 0 || - (r = sshpkt_get_cstring(ssh, &inst, NULL)) != 0 || + if ((r = sshpkt_get_cstring(ssh, &name, &name_len)) != 0 || + (r = sshpkt_get_cstring(ssh, &inst, &inst_len)) != 0 || (r = sshpkt_get_cstring(ssh, &lang, NULL)) != 0) goto out; - if (strlen(name) > 0) - logit("%s", name); - if (strlen(inst) > 0) - logit("%s", inst); + if (name_len > 0) { + debug_f("kbd int name: %s", name); + notifier = notify_start(0, "%s", name); + notify_complete(notifier, NULL); + notifier = NULL; + } + if (inst_len > 0) { + debug_f("kbd int inst: %s", inst); + notifier = notify_start(0, "%s", inst); + notify_complete(notifier, NULL); + notifier = NULL; + } if ((r = sshpkt_get_u32(ssh, &num_prompts)) != 0) goto out; From 99656caabc5cff24122e5b9a140e5a38ab418a5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net> Date: Tue, 17 Oct 2023 06:05:59 +0200 Subject: [PATCH 6/6] auth2-chall: Fix selection of the keyboard-interactive device We were only checking if the prefix of a device name was matching what we had in the devices list, so if the device list contained "pam", then also the device "pam-foo" was matching. --- auth2-chall.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auth2-chall.c b/auth2-chall.c index 047d4e83c33..db658c9b4a7 100644 --- a/auth2-chall.c +++ b/auth2-chall.c @@ -170,7 +170,7 @@ kbdint_next_device(Authctxt *authctxt, KbdintAuthctxt *kbdintctxt) "keyboard-interactive", devices[i]->name)) continue; if (strncmp(kbdintctxt->devices, devices[i]->name, - len) == 0) { + len) == 0 && strlen(devices[i]->name) == len) { kbdintctxt->device = devices[i]; kbdintctxt->devices_done |= 1 << i; }
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
