Skip to content

Commit 779974d

Browse files
committed
delay bailout for invalid authenticating user until after the packet
containing the request has been fully parsed. Reported by Dariusz Tytko and Michał Sajdak; ok deraadt
1 parent 1addc7a commit 779974d

File tree

3 files changed

+28
-19
lines changed

3 files changed

+28
-19
lines changed

usr.bin/ssh/auth2-gss.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: auth2-gss.c,v 1.28 2018/07/10 09:13:30 djm Exp $ */
1+
/* $OpenBSD: auth2-gss.c,v 1.29 2018/07/31 03:10:27 djm Exp $ */
22

33
/*
44
* Copyright (c) 2001-2003 Simon Wilkinson. All rights reserved.
@@ -65,9 +65,6 @@ userauth_gssapi(struct ssh *ssh)
6565
size_t len;
6666
u_char *doid = NULL;
6767

68-
if (!authctxt->valid || authctxt->user == NULL)
69-
return (0);
70-
7168
if ((r = sshpkt_get_u32(ssh, &mechs)) != 0)
7269
fatal("%s: %s", __func__, ssh_err(r));
7370

@@ -101,6 +98,12 @@ userauth_gssapi(struct ssh *ssh)
10198
return (0);
10299
}
103100

101+
if (!authctxt->valid || authctxt->user == NULL) {
102+
debug2("%s: disabled because of invalid user", __func__);
103+
free(doid);
104+
return (0);
105+
}
106+
104107
if (GSS_ERROR(PRIVSEP(ssh_gssapi_server_ctx(&ctxt, &goid)))) {
105108
if (ctxt != NULL)
106109
ssh_gssapi_delete_ctx(&ctxt);

usr.bin/ssh/auth2-hostbased.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: auth2-hostbased.c,v 1.35 2018/07/09 21:35:50 markus Exp $ */
1+
/* $OpenBSD: auth2-hostbased.c,v 1.36 2018/07/31 03:10:27 djm Exp $ */
22
/*
33
* Copyright (c) 2000 Markus Friedl. All rights reserved.
44
*
@@ -66,10 +66,6 @@ userauth_hostbased(struct ssh *ssh)
6666
size_t alen, blen, slen;
6767
int r, pktype, authenticated = 0;
6868

69-
if (!authctxt->valid) {
70-
debug2("%s: disabled because of invalid user", __func__);
71-
return 0;
72-
}
7369
/* XXX use sshkey_froms() */
7470
if ((r = sshpkt_get_cstring(ssh, &pkalg, &alen)) != 0 ||
7571
(r = sshpkt_get_string(ssh, &pkblob, &blen)) != 0 ||
@@ -116,6 +112,11 @@ userauth_hostbased(struct ssh *ssh)
116112
goto done;
117113
}
118114

115+
if (!authctxt->valid || authctxt->user == NULL) {
116+
debug2("%s: disabled because of invalid user", __func__);
117+
goto done;
118+
}
119+
119120
if ((b = sshbuf_new()) == NULL)
120121
fatal("%s: sshbuf_new failed", __func__);
121122
/* reconstruct packet */

usr.bin/ssh/auth2-pubkey.c

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* $OpenBSD: auth2-pubkey.c,v 1.82 2018/07/11 18:55:11 markus Exp $ */
1+
/* $OpenBSD: auth2-pubkey.c,v 1.83 2018/07/31 03:10:27 djm Exp $ */
22
/*
33
* Copyright (c) 2000 Markus Friedl. All rights reserved.
44
*
@@ -86,19 +86,15 @@ userauth_pubkey(struct ssh *ssh)
8686
{
8787
Authctxt *authctxt = ssh->authctxt;
8888
struct passwd *pw = authctxt->pw;
89-
struct sshbuf *b;
89+
struct sshbuf *b = NULL;
9090
struct sshkey *key = NULL;
91-
char *pkalg, *userstyle = NULL, *key_s = NULL, *ca_s = NULL;
92-
u_char *pkblob, *sig, have_sig;
91+
char *pkalg = NULL, *userstyle = NULL, *key_s = NULL, *ca_s = NULL;
92+
u_char *pkblob = NULL, *sig = NULL, have_sig;
9393
size_t blen, slen;
9494
int r, pktype;
9595
int authenticated = 0;
9696
struct sshauthopt *authopts = NULL;
9797

98-
if (!authctxt->valid) {
99-
debug2("%s: disabled because of invalid user", __func__);
100-
return 0;
101-
}
10298
if ((r = sshpkt_get_u8(ssh, &have_sig)) != 0 ||
10399
(r = sshpkt_get_cstring(ssh, &pkalg, NULL)) != 0 ||
104100
(r = sshpkt_get_string(ssh, &pkblob, &blen)) != 0)
@@ -164,6 +160,11 @@ userauth_pubkey(struct ssh *ssh)
164160
fatal("%s: sshbuf_put_string session id: %s",
165161
__func__, ssh_err(r));
166162
}
163+
if (!authctxt->valid || authctxt->user == NULL) {
164+
debug2("%s: disabled because of invalid user",
165+
__func__);
166+
goto done;
167+
}
167168
/* reconstruct packet */
168169
xasprintf(&userstyle, "%s%s%s", authctxt->user,
169170
authctxt->style ? ":" : "",
@@ -180,7 +181,6 @@ userauth_pubkey(struct ssh *ssh)
180181
#ifdef DEBUG_PK
181182
sshbuf_dump(b, stderr);
182183
#endif
183-
184184
/* test for correct signature */
185185
authenticated = 0;
186186
if (PRIVSEP(user_key_allowed(ssh, pw, key, 1, &authopts)) &&
@@ -191,7 +191,6 @@ userauth_pubkey(struct ssh *ssh)
191191
authenticated = 1;
192192
}
193193
sshbuf_free(b);
194-
free(sig);
195194
auth2_record_key(authctxt, authenticated, key);
196195
} else {
197196
debug("%s: test pkalg %s pkblob %s%s%s",
@@ -202,6 +201,11 @@ userauth_pubkey(struct ssh *ssh)
202201
if ((r = sshpkt_get_end(ssh)) != 0)
203202
fatal("%s: %s", __func__, ssh_err(r));
204203

204+
if (!authctxt->valid || authctxt->user == NULL) {
205+
debug2("%s: disabled because of invalid user",
206+
__func__);
207+
goto done;
208+
}
205209
/* XXX fake reply and always send PK_OK ? */
206210
/*
207211
* XXX this allows testing whether a user is allowed
@@ -235,6 +239,7 @@ userauth_pubkey(struct ssh *ssh)
235239
free(pkblob);
236240
free(key_s);
237241
free(ca_s);
242+
free(sig);
238243
return authenticated;
239244
}
240245

0 commit comments

Comments
 (0)