Module: sems
Branch: master
Commit: b10c740718a2313f939e2b137bf485ed7be5823b
URL:    
http://git.sip-router.org/cgi-bin/gitweb.cgi/sems/?a=commit;h=b10c740718a2313f939e2b137bf485ed7be5823b

Author: Stefan Sayer <[email protected]>
Committer: Stefan Sayer <[email protected]>
Date:   Tue Apr 15 12:16:49 2014 +0200

auth: prevent side-channel (timing) attack on UAS auth

---

 core/plug-in/uac_auth/UACAuth.cpp |   28 ++++++++++++++++++++++++++--
 core/plug-in/uac_auth/UACAuth.h   |    9 ++++++++-
 core/tests/test_auth.cpp          |   30 ++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/core/plug-in/uac_auth/UACAuth.cpp 
b/core/plug-in/uac_auth/UACAuth.cpp
index 188e33a..f44ee53 100644
--- a/core/plug-in/uac_auth/UACAuth.cpp
+++ b/core/plug-in/uac_auth/UACAuth.cpp
@@ -293,6 +293,30 @@ void w_MD5Update(MD5_CTX *ctx, const string& s) {
   MD5Update(ctx, a, s.length());
 }
 
+/** time-constant string compare function, but leaks timing of length mismatch 
*/
+bool UACAuth::tc_isequal(const std::string& s1, const std::string& s2) {
+  if (s1.length() != s2.length())
+    return false;
+
+  bool res = false;
+
+  for (size_t i=0;i<s1.length();i++)
+    res |= s1[i]^s2[i];
+
+  return !res;
+}
+
+/** time-constant string compare function, but leaks timing of length mismatch 
*/
+bool UACAuth::tc_isequal(const char* s1, const char* s2, size_t len) {
+
+  bool res = false;
+
+  for (size_t i=0;i<len;i++)
+    res |= s1[i]^s2[i];
+
+  return !res;
+}
+
 // supr gly
 string UACAuth::find_attribute(const string& name, const string& header) {
   size_t pos1 = header.find(name);
@@ -597,7 +621,7 @@ bool UACAuth::checkNonce(const string& nonce) {
   MD5Final(RespHash, &Md5Ctx);
   cvt_hex(RespHash, hash);
 
-  return !strncmp((const char*)hash, &nonce[INT_HEX_LEN], HASHHEXLEN);
+  return tc_isequal((const char*)hash, &nonce[INT_HEX_LEN], HASHHEXLEN);
 }
 
 void UACAuth::setServerSecret(const string& secret) {
@@ -709,7 +733,7 @@ void UACAuth::checkAuthentication(const AmSipRequest* req, 
const string& realm,
     uac_calc_response( ha1, ha2, r_challenge, r_cnonce, qop_value, 
client_nonce_count, response);
     DBG("calculated our response vs request: '%s' vs '%s'", response, 
r_response.c_str());
 
-    if (!strncmp((const char*)response, r_response.c_str(), HASHHEXLEN)) {
+    if (tc_isequal((const char*)response, r_response.c_str(), HASHHEXLEN)) {
       DBG("Auth: authentication successfull\n");
       authenticated = true;
     } else {
diff --git a/core/plug-in/uac_auth/UACAuth.h b/core/plug-in/uac_auth/UACAuth.h
index 0b91c6b..3952596 100644
--- a/core/plug-in/uac_auth/UACAuth.h
+++ b/core/plug-in/uac_auth/UACAuth.h
@@ -112,7 +112,7 @@ class UACAuth : public AmSessionEventHandler
   unsigned int nonce_count;
 
   bool nonce_reuse; // reused nonce?
-
+ 
   static std::string find_attribute(const std::string& name, const 
std::string& header);
   static bool parse_header(const std::string& auth_hdr, 
UACAuthDigestChallenge& challenge);
 
@@ -171,6 +171,13 @@ class UACAuth : public AmSessionEventHandler
                                  const string& user, const string& pwd, AmArg& 
ret);
 
   static void setServerSecret(const string& secret);
+
+  /** time-constant string compare function (but leaks timing of length 
mismatch)
+      @return true if matching */
+  static bool tc_isequal(const std::string& s1, const std::string& s2);
+  /** time-constant string compare function @return true if matching */
+  static bool tc_isequal(const char* s1, const char* s2, size_t len);
+
 };
 
 
diff --git a/core/tests/test_auth.cpp b/core/tests/test_auth.cpp
index 31afa5d..f225640 100644
--- a/core/tests/test_auth.cpp
+++ b/core/tests/test_auth.cpp
@@ -48,4 +48,34 @@ FCTMF_SUITE_BGN(test_auth) {
       fct_chk( !UACAuth::checkNonce(nonce));
     } FCT_TEST_END();
 
+    FCT_TEST_BGN(t_cmp_len) {
+      string s1 = "1234secret";
+      string s2 = "1234s3ecret";
+      fct_chk( !UACAuth::tc_isequal(s1,s2) );
+    } FCT_TEST_END();
+
+    FCT_TEST_BGN(t_cmp_eq) {
+      string s1 = "1234secret";
+      string s2 = "1234secret";
+      fct_chk( UACAuth::tc_isequal(s1,s2) );
+    } FCT_TEST_END();
+
+
+    FCT_TEST_BGN(t_cmp_empty) {
+      fct_chk( UACAuth::tc_isequal("","") );
+    } FCT_TEST_END();
+
+    FCT_TEST_BGN(t_cmp_uneq) {
+      fct_chk( !UACAuth::tc_isequal("1234secret","2134secret") );
+    } FCT_TEST_END();
+
+    FCT_TEST_BGN(t_cmp_uneq_chr) {
+      fct_chk( !UACAuth::tc_isequal("1234secret","2134secret", 10) );
+    } FCT_TEST_END();
+
+    FCT_TEST_BGN(t_cmp_eq_charptr) {
+      fct_chk( UACAuth::tc_isequal("1234secret","1234secret", 10) );
+    } FCT_TEST_END();
+
+
 } FCTMF_SUITE_END();

_______________________________________________
Semsdev mailing list
[email protected]
http://lists.iptel.org/mailman/listinfo/semsdev

Reply via email to