Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dropbear: backport CVE-2018-15599,CVE-2018-20685 fix #582

Merged
merged 2 commits into from
Feb 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions trunk/user/dropbear/dropbear-201X.XX/auth.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(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);

#ifdef ENABLE_SVR_PUBKEY_OPTIONS
int svr_pubkey_allows_agentfwd(void);
Expand Down
3 changes: 2 additions & 1 deletion trunk/user/dropbear/dropbear-201X.XX/scp.c
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,8 @@ sink(int argc, char **argv)
size = size * 10 + (*cp++ - '0');
if (*cp++ != ' ')
SCREWUP("size not delimited");
if ((strchr(cp, '/') != NULL) || (strcmp(cp, "..") == 0)) {
if (*cp == '\0' || strchr(cp, '/') != NULL ||
strcmp(cp, ".") == 0 || strcmp(cp, "..") == 0) {
run_err("error: unexpected filename: %s", cp);
exit(1);
}
Expand Down
17 changes: 5 additions & 12 deletions trunk/user/dropbear/dropbear-201X.XX/svr-auth.c
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,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;
}
}
}
Expand All @@ -191,9 +190,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;
}
}
}
Expand All @@ -204,12 +202,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
Expand Down
26 changes: 22 additions & 4 deletions trunk/user/dropbear/dropbear-201X.XX/svr-authpam.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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",
Expand All @@ -236,7 +248,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;
Expand All @@ -247,12 +259,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,
Expand Down
25 changes: 14 additions & 11 deletions trunk/user/dropbear/dropbear-201X.XX/svr-authpasswd.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,15 @@ 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) {
Expand All @@ -74,11 +67,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",
Expand Down
11 changes: 10 additions & 1 deletion trunk/user/dropbear/dropbear-201X.XX/svr-authpubkey.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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);
Expand Down