nhyatt/putty-source
1
0
Fork 0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-03-22 06:38:37 -05:00
Code Issues Projects Releases Wiki Activity

Improve time-safety of XDM-AUTHORIZATION-1 validation.

Browse Source 
While writing the previous patch, I realise that walking along a
decrypted string and stopping to complain about the first mismatch you
find is an anti-pattern. If we're going to deliberately give the same
error message for various mismatches, so as not to give away which
part failed first, then we should also avoid giving away the same
information via a timing leak!

I don't think this is serious enough to warrant the full-on advisory
protocol, because XDM-AUTHORIZATION-1 is rarely used these days and
also DES-based, so there are bigger problems with it. (Plus, why on
earth is it based on encryption anyway, not a MAC?) But since I
spotted it in passing, might as well fix it.

(cherry picked from commit 8e7e3c59448013be44b8cba2b7f89a894ee34113)
This commit is contained in:
Simon Tatham 2023-04-01 16:07:29 +01:00
parent 3f5873f1fe
commit a02fd09854
1 changed files with 17 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
ssh/x11fwd.c
View File

@ -231,22 +231,24 @@ static char *x11_verify(unsigned long peer_ip, int peer_port,
return dupstr("cannot do XDM-AUTHORIZATION-1 without remote " return dupstr("cannot do XDM-AUTHORIZATION-1 without remote "
"address data"); "address data");
des_decrypt_xdmauth(auth->data+9, data, 24); des_decrypt_xdmauth(auth->data+9, data, 24);
if (memcmp(auth->data, data, 8) != 0) {
/* cookie wrong */ /* Bitwise-OR together any mismatches in the fixed parts of
return dupstr("XDM-AUTHORIZATION-1 data failed check"); * the data, to allow checking it all at once */
} uint32_t mismatches = 0;
if (GET_32BIT_MSB_FIRST(data+8) != peer_ip) { /* Check non-key half of auth cookie */
/* IP wrong */ for (i = 0; i < 8; i++)
return dupstr("XDM-AUTHORIZATION-1 data failed check"); mismatches |= auth->data[i] ^ data[i];
} /* Check IP address and port */
if ((int)GET_16BIT_MSB_FIRST(data+12) != peer_port) { mismatches |= GET_32BIT_MSB_FIRST(data+8) ^ peer_ip;
/* port wrong */ mismatches |= (unsigned short)(GET_16BIT_MSB_FIRST(data+12) ^
return dupstr("XDM-AUTHORIZATION-1 data failed check"); peer_port);
} /* Check zero padding */
t = GET_32BIT_MSB_FIRST(data+14);
for (i = 18; i < 24; i++) for (i = 18; i < 24; i++)
if (data[i] != 0) /* zero padding wrong */ mismatches |= data[i];
return dupstr("XDM-AUTHORIZATION-1 data failed check"); if (mismatches)
return dupstr("XDM-AUTHORIZATION-1 data failed check");
t = GET_32BIT_MSB_FIRST(data+14);
tim = time(NULL); tim = time(NULL);
if (((unsigned long)t - (unsigned long)tim if (((unsigned long)t - (unsigned long)tim
+ XDM_MAXSKEW) > 2*XDM_MAXSKEW) + XDM_MAXSKEW) > 2*XDM_MAXSKEW)