OpenDoas icon indicating copy to clipboard operation
OpenDoas copied to clipboard

compare strings in constant time

Open syndrome2 opened this issue 11 months ago • 0 comments

I understand that on a well-tuned system, the success of a timing attack is doubtful, but why not increase the security for free? compare_in_constant_time.patch.txt

diff '--color=auto' -ru a/shadow.c b/shadow.c
--- a/shadow.c	2022-01-26 18:01:11.000000000 +0200
+++ b/shadow.c	2023-07-19 08:27:01.806569020 +0300
@@ -41,6 +41,19 @@
 #define HOST_NAME_MAX _POSIX_HOST_NAME_MAX
 #endif

+int compare_in_constant_time(const char * str1, const char * str2)
+{
+	char result = 0;
+	while (1)
+	{
+		char a = *str1 ^ *str2;
+		result = result | a;
+		if ((*str1 == 0) || (*str2 == 0)) {break;}
+		str1++; str2++;
+	}
+	return (int)result;
+}
+
 void
 shadowauth(const char *myname, int persist)
 {
@@ -94,7 +107,7 @@
 		errx(1, "Authentication failed");
 	}
 	explicit_bzero(rbuf, sizeof(rbuf));
-	if (strcmp(encrypted, hash) != 0) {
+	if (compare_in_constant_time(encrypted, hash) != 0) {
 		syslog(LOG_AUTHPRIV | LOG_NOTICE, "failed auth for %s", myname);
 		errx(1, "Authentication failed");
 	}

syndrome2 avatar Jul 19 '23 07:07 syndrome2