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