Fossil SCM

Protect against timing attacks by using constant-time comparison function to compare passwords and cookies.

dmitry 2011-09-29 17:21 trunk
Commit 7f110475ec15be9cc1bcda7d78ab2888f914f915
Parent 9bfa186be09f3f9…
3 files changed +26 +43 -11 +2 -2
~ src/blob.c ~ src/login.c ~ src/xfer.c
M src/blob.c
+26
--- src/blob.c
+++ src/blob.c
@@ -311,10 +311,36 @@
311311
 sz = szA<szB ? szA : szB;
312312
 rc = memcmp(blob_buffer(pA), blob_buffer(pB), sz);
313313
 if( rc==0 ){
314314
 rc = szA - szB;
315315
 }
316
+ return rc;
317
+}
318
+
319
+/*
320
+** Compare two blobs in constant time and return zero if they are equal.
321
+** Constant time comparison only applies for blobs of the same length.
322
+** If lengths are different, immediately returns false.
323
+*/
324
+int blob_constant_time_eq(Blob *pA, Blob *pB){
325
+ int szA, szB, i;
326
+ unsigned char *buf1, *buf2;
327
+ unsigned char rc = 0;
328
+
329
+ blob_is_init(pA);
330
+ blob_is_init(pB);
331
+ szA = blob_size(pA);
332
+ szB = blob_size(pB);
333
+ if( szA!=szB ) return 1;
334
+
335
+ buf1 = blob_buffer(pA);
336
+ buf2 = blob_buffer(pB);
337
+
338
+ for( i=0; i<szA; i++ ){
339
+ rc = rc | (buf1[i] ^ buf2[i]);
340
+ }
341
+
316342
 return rc;
317343
 }
318344
319345
 /*
320346
 ** Compare a blob to a string. Return TRUE if they are equal.
321347
--- src/blob.c
+++ src/blob.c
@@ -311,10 +311,36 @@
311 sz = szA<szB ? szA : szB;
312 rc = memcmp(blob_buffer(pA), blob_buffer(pB), sz);
313 if( rc==0 ){
314 rc = szA - szB;
315 }
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
316 return rc;
317 }
318
319 /*
320 ** Compare a blob to a string. Return TRUE if they are equal.
321
--- src/blob.c
+++ src/blob.c
@@ -311,10 +311,36 @@
311 sz = szA<szB ? szA : szB;
312 rc = memcmp(blob_buffer(pA), blob_buffer(pB), sz);
313 if( rc==0 ){
314 rc = szA - szB;
315 }
316 return rc;
317 }
318
319 /*
320 ** Compare two blobs in constant time and return zero if they are equal.
321 ** Constant time comparison only applies for blobs of the same length.
322 ** If lengths are different, immediately returns false.
323 */
324 int blob_constant_time_eq(Blob *pA, Blob *pB){
325 int szA, szB, i;
326 unsigned char *buf1, *buf2;
327 unsigned char rc = 0;
328
329 blob_is_init(pA);
330 blob_is_init(pB);
331 szA = blob_size(pA);
332 szB = blob_size(pB);
333 if( szA!=szB ) return 1;
334
335 buf1 = blob_buffer(pA);
336 buf2 = blob_buffer(pB);
337
338 for( i=0; i<szA; i++ ){
339 rc = rc | (buf1[i] ^ buf2[i]);
340 }
341
342 return rc;
343 }
344
345 /*
346 ** Compare a blob to a string. Return TRUE if they are equal.
347
M src/login.c
+43 -11
--- src/login.c
+++ src/login.c
@@ -230,12 +230,14 @@
230230
 }
231231
 if( g.perm.Password && zPasswd && (zNew1 = P("n1"))!=0 && (zNew2 = P("n2"))!=0 ){
232232
 /* The user requests a password change */
233233
 zSha1Pw = sha1_shared_secret(zPasswd, g.zLogin, 0);
234234
 if( db_int(1, "SELECT 0 FROM user"
235
- " WHERE uid=%d AND (pw=%Q OR pw=%Q)",
236
- g.userUid, zPasswd, zSha1Pw) ){
235
+ " WHERE uid=%d"
236
+ " AND (constant_time_eq(pw,%Q)=0"
237
+ " OR constant_time_eq(pw,%Q)=0)",
238
+ g.userUid, zSha1Pw, zPasswd) ){
237239
 sleep(1);
238240
 zErrMsg =
239241
 @ <p><span class="loginError">
240242
 @ You entered an incorrect old password while attempting to change
241243
 @ your password. Your password is unchanged.
@@ -308,12 +310,12 @@
308310
 uid = db_int(0,
309311
 "SELECT uid FROM user"
310312
 " WHERE login=%Q"
311313
 " AND length(cap)>0 AND length(pw)>0"
312314
 " AND login NOT IN ('anonymous','nobody','developer','reader')"
313
- " AND (pw=%Q OR pw=%Q)",
314
- zUsername, zPasswd, zSha1Pw
315
+ " AND (constant_time_eq(pw,%Q)=0 OR constant_time_eq(pw,%Q)=0)",
316
+ zUsername, zSha1Pw, zPasswd
315317
 );
316318
 if( uid<=0 ){
317319
 sleep(1);
318320
 zErrMsg =
319321
 @ <p><span class="loginError">
@@ -484,17 +486,17 @@
484486
 if( rc==SQLITE_OK ){
485487
 sqlite3_create_function(pOther,"now",0,SQLITE_ANY,0,db_now_function,0,0);
486488
 sqlite3_busy_timeout(pOther, 5000);
487489
 zSQL = mprintf(
488490
 "SELECT cexpire FROM user"
489
- " WHERE cookie=%Q"
491
+ " WHERE login=%Q"
490492
 " AND ipaddr=%Q"
491
- " AND login=%Q"
492493
 " AND length(cap)>0"
493494
 " AND length(pw)>0"
494
- " AND cexpire>julianday('now')",
495
- zHash, zRemoteAddr, zLogin
495
+ " AND cexpire>julianday('now')"
496
+ " AND constant_time_eq(cookie,%Q)=0",
497
+ zLogin, zRemoteAddr, zHash
496498
 );
497499
 pStmt = 0;
498500
 rc = sqlite3_prepare_v2(pOther, zSQL, -1, &pStmt, 0);
499501
 if( rc==SQLITE_OK && sqlite3_step(pStmt)==SQLITE_ROW ){
500502
 db_multi_exec(
@@ -527,19 +529,46 @@
527529
 if( fossil_strcmp(zLogin, "developer")==0 ) return 0;
528530
 if( fossil_strcmp(zLogin, "reader")==0 ) return 0;
529531
 uid = db_int(0,
530532
 "SELECT uid FROM user"
531533
 " WHERE login=%Q"
532
- " AND cookie=%Q"
533534
 " AND ipaddr=%Q"
534535
 " AND cexpire>julianday('now')"
535536
 " AND length(cap)>0"
536
- " AND length(pw)>0",
537
- zLogin, zCookie, zRemoteAddr
537
+ " AND length(pw)>0"
538
+ " AND constant_time_eq(cookie,%Q)=0",
539
+ zLogin, zRemoteAddr, zCookie
538540
 );
539541
 return uid;
540542
 }
543
+
544
+/*
545
+** SQL function for constant time comparison of two values.
546
+** Sets result to 0 if two values are equal.
547
+*/
548
+static void constant_time_eq_function(
549
+ sqlite3_context *context,
550
+ int argc,
551
+ sqlite3_value **argv
552
+){
553
+ const unsigned char *buf1, *buf2;
554
+ int len, i;
555
+ unsigned char rc = 0;
556
+
557
+ assert( argc==2 );
558
+ len = sqlite3_value_bytes(argv[0]);
559
+ if( len==0 || len!=sqlite3_value_bytes(argv[1]) ){
560
+ rc = 1;
561
+ }else{
562
+ buf1 = sqlite3_value_text(argv[0]);
563
+ buf2 = sqlite3_value_text(argv[1]);
564
+ for( i=0; i<len; i++ ){
565
+ rc = rc | (buf1[i] ^ buf2[i]);
566
+ }
567
+ }
568
+ sqlite3_result_int(context, rc);
569
+}
541570
542571
 /*
543572
 ** This routine examines the login cookie to see if it exists and
544573
 ** and is valid. If the login cookie checks out, it then sets
545574
 ** global variables appropriately. Global variables set include
@@ -554,10 +583,13 @@
554583
 char *zRemoteAddr; /* Abbreviated IP address of the requestor */
555584
 const char *zCap = 0; /* Capability string */
556585
557586
 /* Only run this check once. */
558587
 if( g.userUid!=0 ) return;
588
+
589
+ sqlite3_create_function(g.db, "constant_time_eq", 2, SQLITE_UTF8, 0,
590
+ constant_time_eq_function, 0, 0);
559591
560592
 /* If the HTTP connection is coming over 127.0.0.1 and if
561593
 ** local login is disabled and if we are using HTTP and not HTTPS,
562594
 ** then there is no need to check user credentials.
563595
 **
564596
--- src/login.c
+++ src/login.c
@@ -230,12 +230,14 @@
230 }
231 if( g.perm.Password && zPasswd && (zNew1 = P("n1"))!=0 && (zNew2 = P("n2"))!=0 ){
232 /* The user requests a password change */
233 zSha1Pw = sha1_shared_secret(zPasswd, g.zLogin, 0);
234 if( db_int(1, "SELECT 0 FROM user"
235 " WHERE uid=%d AND (pw=%Q OR pw=%Q)",
236 g.userUid, zPasswd, zSha1Pw) ){
 
 
237 sleep(1);
238 zErrMsg =
239 @ <p><span class="loginError">
240 @ You entered an incorrect old password while attempting to change
241 @ your password. Your password is unchanged.
@@ -308,12 +310,12 @@
308 uid = db_int(0,
309 "SELECT uid FROM user"
310 " WHERE login=%Q"
311 " AND length(cap)>0 AND length(pw)>0"
312 " AND login NOT IN ('anonymous','nobody','developer','reader')"
313 " AND (pw=%Q OR pw=%Q)",
314 zUsername, zPasswd, zSha1Pw
315 );
316 if( uid<=0 ){
317 sleep(1);
318 zErrMsg =
319 @ <p><span class="loginError">
@@ -484,17 +486,17 @@
484 if( rc==SQLITE_OK ){
485 sqlite3_create_function(pOther,"now",0,SQLITE_ANY,0,db_now_function,0,0);
486 sqlite3_busy_timeout(pOther, 5000);
487 zSQL = mprintf(
488 "SELECT cexpire FROM user"
489 " WHERE cookie=%Q"
490 " AND ipaddr=%Q"
491 " AND login=%Q"
492 " AND length(cap)>0"
493 " AND length(pw)>0"
494 " AND cexpire>julianday('now')",
495 zHash, zRemoteAddr, zLogin
 
496 );
497 pStmt = 0;
498 rc = sqlite3_prepare_v2(pOther, zSQL, -1, &pStmt, 0);
499 if( rc==SQLITE_OK && sqlite3_step(pStmt)==SQLITE_ROW ){
500 db_multi_exec(
@@ -527,19 +529,46 @@
527 if( fossil_strcmp(zLogin, "developer")==0 ) return 0;
528 if( fossil_strcmp(zLogin, "reader")==0 ) return 0;
529 uid = db_int(0,
530 "SELECT uid FROM user"
531 " WHERE login=%Q"
532 " AND cookie=%Q"
533 " AND ipaddr=%Q"
534 " AND cexpire>julianday('now')"
535 " AND length(cap)>0"
536 " AND length(pw)>0",
537 zLogin, zCookie, zRemoteAddr
 
538 );
539 return uid;
540 }
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
541
542 /*
543 ** This routine examines the login cookie to see if it exists and
544 ** and is valid. If the login cookie checks out, it then sets
545 ** global variables appropriately. Global variables set include
@@ -554,10 +583,13 @@
554 char *zRemoteAddr; /* Abbreviated IP address of the requestor */
555 const char *zCap = 0; /* Capability string */
556
557 /* Only run this check once. */
558 if( g.userUid!=0 ) return;
 
 
 
559
560 /* If the HTTP connection is coming over 127.0.0.1 and if
561 ** local login is disabled and if we are using HTTP and not HTTPS,
562 ** then there is no need to check user credentials.
563 **
564
--- src/login.c
+++ src/login.c
@@ -230,12 +230,14 @@
230 }
231 if( g.perm.Password && zPasswd && (zNew1 = P("n1"))!=0 && (zNew2 = P("n2"))!=0 ){
232 /* The user requests a password change */
233 zSha1Pw = sha1_shared_secret(zPasswd, g.zLogin, 0);
234 if( db_int(1, "SELECT 0 FROM user"
235 " WHERE uid=%d"
236 " AND (constant_time_eq(pw,%Q)=0"
237 " OR constant_time_eq(pw,%Q)=0)",
238 g.userUid, zSha1Pw, zPasswd) ){
239 sleep(1);
240 zErrMsg =
241 @ <p><span class="loginError">
242 @ You entered an incorrect old password while attempting to change
243 @ your password. Your password is unchanged.
@@ -308,12 +310,12 @@
310 uid = db_int(0,
311 "SELECT uid FROM user"
312 " WHERE login=%Q"
313 " AND length(cap)>0 AND length(pw)>0"
314 " AND login NOT IN ('anonymous','nobody','developer','reader')"
315 " AND (constant_time_eq(pw,%Q)=0 OR constant_time_eq(pw,%Q)=0)",
316 zUsername, zSha1Pw, zPasswd
317 );
318 if( uid<=0 ){
319 sleep(1);
320 zErrMsg =
321 @ <p><span class="loginError">
@@ -484,17 +486,17 @@
486 if( rc==SQLITE_OK ){
487 sqlite3_create_function(pOther,"now",0,SQLITE_ANY,0,db_now_function,0,0);
488 sqlite3_busy_timeout(pOther, 5000);
489 zSQL = mprintf(
490 "SELECT cexpire FROM user"
491 " WHERE login=%Q"
492 " AND ipaddr=%Q"
 
493 " AND length(cap)>0"
494 " AND length(pw)>0"
495 " AND cexpire>julianday('now')"
496 " AND constant_time_eq(cookie,%Q)=0",
497 zLogin, zRemoteAddr, zHash
498 );
499 pStmt = 0;
500 rc = sqlite3_prepare_v2(pOther, zSQL, -1, &pStmt, 0);
501 if( rc==SQLITE_OK && sqlite3_step(pStmt)==SQLITE_ROW ){
502 db_multi_exec(
@@ -527,19 +529,46 @@
529 if( fossil_strcmp(zLogin, "developer")==0 ) return 0;
530 if( fossil_strcmp(zLogin, "reader")==0 ) return 0;
531 uid = db_int(0,
532 "SELECT uid FROM user"
533 " WHERE login=%Q"
 
534 " AND ipaddr=%Q"
535 " AND cexpire>julianday('now')"
536 " AND length(cap)>0"
537 " AND length(pw)>0"
538 " AND constant_time_eq(cookie,%Q)=0",
539 zLogin, zRemoteAddr, zCookie
540 );
541 return uid;
542 }
543
544 /*
545 ** SQL function for constant time comparison of two values.
546 ** Sets result to 0 if two values are equal.
547 */
548 static void constant_time_eq_function(
549 sqlite3_context *context,
550 int argc,
551 sqlite3_value **argv
552 ){
553 const unsigned char *buf1, *buf2;
554 int len, i;
555 unsigned char rc = 0;
556
557 assert( argc==2 );
558 len = sqlite3_value_bytes(argv[0]);
559 if( len==0 || len!=sqlite3_value_bytes(argv[1]) ){
560 rc = 1;
561 }else{
562 buf1 = sqlite3_value_text(argv[0]);
563 buf2 = sqlite3_value_text(argv[1]);
564 for( i=0; i<len; i++ ){
565 rc = rc | (buf1[i] ^ buf2[i]);
566 }
567 }
568 sqlite3_result_int(context, rc);
569 }
570
571 /*
572 ** This routine examines the login cookie to see if it exists and
573 ** and is valid. If the login cookie checks out, it then sets
574 ** global variables appropriately. Global variables set include
@@ -554,10 +583,13 @@
583 char *zRemoteAddr; /* Abbreviated IP address of the requestor */
584 const char *zCap = 0; /* Capability string */
585
586 /* Only run this check once. */
587 if( g.userUid!=0 ) return;
588
589 sqlite3_create_function(g.db, "constant_time_eq", 2, SQLITE_UTF8, 0,
590 constant_time_eq_function, 0, 0);
591
592 /* If the HTTP connection is coming over 127.0.0.1 and if
593 ** local login is disabled and if we are using HTTP and not HTTPS,
594 ** then there is no need to check user credentials.
595 **
596
M src/xfer.c
+2 -2
--- src/xfer.c
+++ src/xfer.c
@@ -573,11 +573,11 @@
573573
 blob_zero(&combined);
574574
 blob_copy(&combined, pNonce);
575575
 blob_append(&combined, blob_buffer(&pw), szPw);
576576
 sha1sum_blob(&combined, &hash);
577577
 assert( blob_size(&hash)==40 );
578
- rc = blob_compare(&hash, pSig);
578
+ rc = blob_constant_time_eq(&hash, pSig);
579579
 blob_reset(&hash);
580580
 blob_reset(&combined);
581581
 if( rc!=0 && szPw!=40 ){
582582
 /* If this server stores cleartext passwords and the password did not
583583
 ** match, then perhaps the client is sending SHA1 passwords. Try
@@ -588,11 +588,11 @@
588588
 blob_zero(&combined);
589589
 blob_copy(&combined, pNonce);
590590
 blob_append(&combined, zSecret, -1);
591591
 free(zSecret);
592592
 sha1sum_blob(&combined, &hash);
593
- rc = blob_compare(&hash, pSig);
593
+ rc = blob_constant_time_eq(&hash, pSig);
594594
 blob_reset(&hash);
595595
 blob_reset(&combined);
596596
 }
597597
 if( rc==0 ){
598598
 const char *zCap;
599599
--- src/xfer.c
+++ src/xfer.c
@@ -573,11 +573,11 @@
573 blob_zero(&combined);
574 blob_copy(&combined, pNonce);
575 blob_append(&combined, blob_buffer(&pw), szPw);
576 sha1sum_blob(&combined, &hash);
577 assert( blob_size(&hash)==40 );
578 rc = blob_compare(&hash, pSig);
579 blob_reset(&hash);
580 blob_reset(&combined);
581 if( rc!=0 && szPw!=40 ){
582 /* If this server stores cleartext passwords and the password did not
583 ** match, then perhaps the client is sending SHA1 passwords. Try
@@ -588,11 +588,11 @@
588 blob_zero(&combined);
589 blob_copy(&combined, pNonce);
590 blob_append(&combined, zSecret, -1);
591 free(zSecret);
592 sha1sum_blob(&combined, &hash);
593 rc = blob_compare(&hash, pSig);
594 blob_reset(&hash);
595 blob_reset(&combined);
596 }
597 if( rc==0 ){
598 const char *zCap;
599
--- src/xfer.c
+++ src/xfer.c
@@ -573,11 +573,11 @@
573 blob_zero(&combined);
574 blob_copy(&combined, pNonce);
575 blob_append(&combined, blob_buffer(&pw), szPw);
576 sha1sum_blob(&combined, &hash);
577 assert( blob_size(&hash)==40 );
578 rc = blob_constant_time_eq(&hash, pSig);
579 blob_reset(&hash);
580 blob_reset(&combined);
581 if( rc!=0 && szPw!=40 ){
582 /* If this server stores cleartext passwords and the password did not
583 ** match, then perhaps the client is sending SHA1 passwords. Try
@@ -588,11 +588,11 @@
588 blob_zero(&combined);
589 blob_copy(&combined, pNonce);
590 blob_append(&combined, zSecret, -1);
591 free(zSecret);
592 sha1sum_blob(&combined, &hash);
593 rc = blob_constant_time_eq(&hash, pSig);
594 blob_reset(&hash);
595 blob_reset(&combined);
596 }
597 if( rc==0 ){
598 const char *zCap;
599

Keyboard Shortcuts

Open search /
Next entry (timeline) j
Previous entry (timeline) k
Open focused entry Enter
Show this help ?
Toggle theme Top nav button