1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
|
Fix CVE-2018-15599:
http://lists.ucc.gu.uwa.edu.au/pipermail/dropbear/2018q3/002108.html
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-15599
Patch copied from upstream source repository:
https://github.com/mkj/dropbear/commit/52adbb34c32d3e2e1bcdb941e20a6f81138b8248
From 52adbb34c32d3e2e1bcdb941e20a6f81138b8248 Mon Sep 17 00:00:00 2001
From: Matt Johnston <matt@ucc.asn.au>
Date: Thu, 23 Aug 2018 23:43:12 +0800
Subject: [PATCH] Wait to fail invalid usernames
---
auth.h | 6 +++---
svr-auth.c | 19 +++++--------------
svr-authpam.c | 26 ++++++++++++++++++++++----
svr-authpasswd.c | 27 ++++++++++++++-------------
svr-authpubkey.c | 11 ++++++++++-
5 files changed, 54 insertions(+), 35 deletions(-)
diff --git a/auth.h b/auth.h
index da498f5b..98f54683 100644
--- a/auth.h
+++ b/auth.h
@@ -37,9 +37,9 @@ void recv_msg_userauth_request(void);
void send_msg_userauth_failure(int partial, int incrfail);
void send_msg_userauth_success(void);
void send_msg_userauth_banner(const buffer *msg);
-void svr_auth_password(void);
-void svr_auth_pubkey(void);
-void svr_auth_pam(void);
+void svr_auth_password(int valid_user);
+void svr_auth_pubkey(int valid_user);
+void svr_auth_pam(int valid_user);
#if DROPBEAR_SVR_PUBKEY_OPTIONS_BUILT
int svr_pubkey_allows_agentfwd(void);
diff --git a/svr-auth.c b/svr-auth.c
index c19c0901..edde86bc 100644
--- a/svr-auth.c
+++ b/svr-auth.c
@@ -149,10 +149,8 @@ void recv_msg_userauth_request() {
if (methodlen == AUTH_METHOD_PASSWORD_LEN &&
strncmp(methodname, AUTH_METHOD_PASSWORD,
AUTH_METHOD_PASSWORD_LEN) == 0) {
- if (valid_user) {
- svr_auth_password();
- goto out;
- }
+ svr_auth_password(valid_user);
+ goto out;
}
}
#endif
@@ -164,10 +162,8 @@ void recv_msg_userauth_request() {
if (methodlen == AUTH_METHOD_PASSWORD_LEN &&
strncmp(methodname, AUTH_METHOD_PASSWORD,
AUTH_METHOD_PASSWORD_LEN) == 0) {
- if (valid_user) {
- svr_auth_pam();
- goto out;
- }
+ svr_auth_pam(valid_user);
+ goto out;
}
}
#endif
@@ -177,12 +173,7 @@ void recv_msg_userauth_request() {
if (methodlen == AUTH_METHOD_PUBKEY_LEN &&
strncmp(methodname, AUTH_METHOD_PUBKEY,
AUTH_METHOD_PUBKEY_LEN) == 0) {
- if (valid_user) {
- svr_auth_pubkey();
- } else {
- /* pubkey has no failure delay */
- send_msg_userauth_failure(0, 0);
- }
+ svr_auth_pubkey(valid_user);
goto out;
}
#endif
diff --git a/svr-authpam.c b/svr-authpam.c
index 05e4f3e5..d201bc96 100644
--- a/svr-authpam.c
+++ b/svr-authpam.c
@@ -178,13 +178,14 @@ pamConvFunc(int num_msg,
* Keyboard interactive would be a lot nicer, but since PAM is synchronous, it
* gets very messy trying to send the interactive challenges, and read the
* interactive responses, over the network. */
-void svr_auth_pam() {
+void svr_auth_pam(int valid_user) {
struct UserDataS userData = {NULL, NULL};
struct pam_conv pamConv = {
pamConvFunc,
&userData /* submitted to pamvConvFunc as appdata_ptr */
};
+ const char* printable_user = NULL;
pam_handle_t* pamHandlep = NULL;
@@ -204,12 +205,23 @@ void svr_auth_pam() {
password = buf_getstring(ses.payload, &passwordlen);
+ /* We run the PAM conversation regardless of whether the username is valid
+ in case the conversation function has an inherent delay.
+ Use ses.authstate.username rather than ses.authstate.pw_name.
+ After PAM succeeds we then check the valid_user flag too */
+
/* used to pass data to the PAM conversation function - don't bother with
* strdup() etc since these are touched only by our own conversation
* function (above) which takes care of it */
- userData.user = ses.authstate.pw_name;
+ userData.user = ses.authstate.username;
userData.passwd = password;
+ if (ses.authstate.pw_name) {
+ printable_user = ses.authstate.pw_name;
+ } else {
+ printable_user = "<invalid username>";
+ }
+
/* Init pam */
if ((rc = pam_start("sshd", NULL, &pamConv, &pamHandlep)) != PAM_SUCCESS) {
dropbear_log(LOG_WARNING, "pam_start() failed, rc=%d, %s",
@@ -242,7 +254,7 @@ void svr_auth_pam() {
rc, pam_strerror(pamHandlep, rc));
dropbear_log(LOG_WARNING,
"Bad PAM password attempt for '%s' from %s",
- ses.authstate.pw_name,
+ printable_user,
svr_ses.addrstring);
send_msg_userauth_failure(0, 1);
goto cleanup;
@@ -253,12 +265,18 @@ void svr_auth_pam() {
rc, pam_strerror(pamHandlep, rc));
dropbear_log(LOG_WARNING,
"Bad PAM password attempt for '%s' from %s",
- ses.authstate.pw_name,
+ printable_user,
svr_ses.addrstring);
send_msg_userauth_failure(0, 1);
goto cleanup;
}
+ if (!valid_user) {
+ /* PAM auth succeeded but the username isn't allowed in for another reason
+ (checkusername() failed) */
+ send_msg_userauth_failure(0, 1);
+ }
+
/* successful authentication */
dropbear_log(LOG_NOTICE, "PAM password auth succeeded for '%s' from %s",
ses.authstate.pw_name,
diff --git a/svr-authpasswd.c b/svr-authpasswd.c
index bdee2aa1..69c7d8af 100644
--- a/svr-authpasswd.c
+++ b/svr-authpasswd.c
@@ -48,22 +48,14 @@ static int constant_time_strcmp(const char* a, const char* b) {
/* Process a password auth request, sending success or failure messages as
* appropriate */
-void svr_auth_password() {
+void svr_auth_password(int valid_user) {
char * passwdcrypt = NULL; /* the crypt from /etc/passwd or /etc/shadow */
char * testcrypt = NULL; /* crypt generated from the user's password sent */
- char * password;
+ char * password = NULL;
unsigned int passwordlen;
-
unsigned int changepw;
- passwdcrypt = ses.authstate.pw_passwd;
-
-#ifdef DEBUG_HACKCRYPT
- /* debugging crypt for non-root testing with shadows */
- passwdcrypt = DEBUG_HACKCRYPT;
-#endif
-
/* check if client wants to change password */
changepw = buf_getbool(ses.payload);
if (changepw) {
@@ -73,12 +65,21 @@ void svr_auth_password() {
}
password = buf_getstring(ses.payload, &passwordlen);
-
- /* the first bytes of passwdcrypt are the salt */
- testcrypt = crypt(password, passwdcrypt);
+ if (valid_user) {
+ /* the first bytes of passwdcrypt are the salt */
+ passwdcrypt = ses.authstate.pw_passwd;
+ testcrypt = crypt(password, passwdcrypt);
+ }
m_burn(password, passwordlen);
m_free(password);
+ /* After we have got the payload contents we can exit if the username
+ is invalid. Invalid users have already been logged. */
+ if (!valid_user) {
+ send_msg_userauth_failure(0, 1);
+ return;
+ }
+
if (testcrypt == NULL) {
/* crypt() with an invalid salt like "!!" */
dropbear_log(LOG_WARNING, "User account '%s' is locked",
diff --git a/svr-authpubkey.c b/svr-authpubkey.c
index aa6087c9..ff481c87 100644
--- a/svr-authpubkey.c
+++ b/svr-authpubkey.c
@@ -79,7 +79,7 @@ static int checkfileperm(char * filename);
/* process a pubkey auth request, sending success or failure message as
* appropriate */
-void svr_auth_pubkey() {
+void svr_auth_pubkey(int valid_user) {
unsigned char testkey; /* whether we're just checking if a key is usable */
char* algo = NULL; /* pubkey algo */
@@ -102,6 +102,15 @@ void svr_auth_pubkey() {
keybloblen = buf_getint(ses.payload);
keyblob = buf_getptr(ses.payload, keybloblen);
+ if (!valid_user) {
+ /* Return failure once we have read the contents of the packet
+ required to validate a public key.
+ Avoids blind user enumeration though it isn't possible to prevent
+ testing for user existence if the public key is known */
+ send_msg_userauth_failure(0, 0);
+ goto out;
+ }
+
/* check if the key is valid */
if (checkpubkey(algo, algolen, keyblob, keybloblen) == DROPBEAR_FAILURE) {
send_msg_userauth_failure(0, 0);
|