Merge pull request #20 from mildred/mildred-fix-scram

Fix for Nim 1.6.6

Author: ba0f3

scram/client.nim

@@ -1,3 +1,4 @@
+import strformat
 import base64, pegs, strutils, hmac, sha1, nimSHA2, md5, private/[utils,types]
 
 export MD5Digest, SHA1Digest, SHA224Digest, SHA256Digest, SHA384Digest, SHA512Digest, Keccak512Digest
@@ -53,9 +54,13 @@ proc prepareFinalMessage*[T](s: ScramClient[T], password, serverFirstMessage: st
     iterations: int
   var matches: array[3, string]
   if match(serverFirstMessage, SERVER_FIRST_MESSAGE, matches):
-    nonce = matches[0]
-    salt = base64.decode(matches[1])
-    iterations = parseInt(matches[2])
+    for kv in serverFirstMessage.split(','):
+      if kv[0..1] == "i=":
+        iterations = parseInt(kv[2..^1])
+      elif kv[0..1] == "r=":
+        nonce = kv[2..^1]
+      elif kv[0..1] == "s=":
+        salt = base64.decode(kv[2..^1])
   else:
     s.state = ENDED
     return ""
@@ -89,7 +94,10 @@ proc verifyServerFinalMessage*(s: ScramClient, serverFinalMessage: string): bool
   s.state = ENDED
   var matches: array[1, string]
   if match(serverFinalMessage, SERVER_FINAL_MESSAGE, matches):
-    let proposedServerSignature = base64.decode(matches[0])
+    var proposedServerSignature: string
+    for kv in serverFinalMessage.split(','):
+      if kv[0..1] == "v=":
+        proposedServerSignature = base64.decode(kv[2..^1])
     s.isSuccessful = proposedServerSignature == $%s.serverSignature
   s.isSuccessful
 

scram/private/types.nim

@@ -1,5 +1,5 @@
 type
-  ScramError* = object of Exception
+  ScramError* = object of CatchableError
 
   DigestType* = enum
     MD5

scram/private/utils.nim

@@ -1,4 +1,4 @@
-import random, base64, strutils, types, hmac
+import random, base64, strutils, types, hmac, bitops
 from md5 import MD5Digest
 from sha1 import Sha1Digest
 from nimSHA2 import Sha224Digest, Sha256Digest, Sha384Digest, Sha512Digest
@@ -20,6 +20,21 @@ template `^=`*[T](a, b: T) =
     else:
       a[x] = (a[x].int32 xor b[x].int32).char
 
+proc custom_xor*[T](bytes: T, str: string): string =
+  if bytes.len != str.len:
+    raise newException(RangeDefect, "xor must have both arguments of the same size")
+  result = str
+  for x in 0..<bytes.len:
+    result[x] = (bytes[x].uint8 xor str[x].uint8).char
+
+proc constantTimeEqual*(a, b: string): bool =
+  if a.len != b.len:
+    raise newException(RangeDefect, "must have both arguments of the same size")
+  var res: uint8 = 0
+  for x in 0..<a.len:
+    res = bitor(res, bitxor(a[x].uint8, b[x].uint8))
+  result = (res == 0)
+
 proc HMAC*[T](password, salt: string): T =
   when T is MD5Digest:
     result = hmac_md5(password, salt)
@@ -36,6 +51,12 @@ proc HMAC*[T](password, salt: string): T =
   elif T is Keccak512Digest:
     result = hmac_keccak512(password, salt)
 
+proc raw_str*[T](digest: T): string =
+  when T is Sha1Digest:
+    for c in digest: result.add(char(c))
+  else:
+    result = $digest
+
 proc HASH*[T](s: string): T =
   when T is MD5Digest:
     result = hash_md5(s)
@@ -57,7 +78,6 @@ proc debug[T](s: T): string =
   for x in s:
     result.add strutils.toHex(x.uint8).toLowerAscii
 
-
 proc hi*[T](password, salt: string, iterations: int): T =
   var previous = HMAC[T](password, salt & INT_1)
   result = previous

scram/server.nim

@@ -1,11 +1,12 @@
+import strformat, strutils
 import base64, pegs, strutils, hmac, nimSHA2, private/[utils,types]
 
 type
   ScramServer*[T] = ref object of RootObj
-    serverNonce: string
+    serverNonce*: string
     clientFirstMessageBare: string
     serverFirstMessage: string
-    state: ScramState
+    state*: ScramState
     isSuccessful: bool
     userData: UserData
 
@@ -19,22 +20,25 @@ let
   CLIENT_FIRST_MESSAGE = peg"^([pny]'='?([^,]*)','([^,]*)','){('m='([^,]*)',')?'n='{[^,]*}',r='{[^,]*}(','(.*))*}$"
   CLIENT_FINAL_MESSAGE = peg"{'c='{[^,]*}',r='{[^,]*}}',p='{.*}$"
 
-proc initUserData*(password: string, iterations = 4096): UserData =
+proc initUserData*[T](typ: typedesc[T], password: string, iterations = 4096): UserData =
   var iterations = iterations
   if password.len == 0:
     iterations = 1
   let
     salt = makeNonce()[0..24]
-    saltedPassword = hi[SHA256Digest](password, salt, iterations)
-    clientKey = HMAC[SHA256Digest]($%saltedPassword, CLIENT_KEY)
-    storedKey = HASH[SHA256Digest]($%clientKey)
-    serverKey = HMAC[SHA256Digest]($%saltedPassword, SERVER_KEY)
+    saltedPassword = hi[T](password, salt, iterations)
+    clientKey = HMAC[T]($%saltedPassword, CLIENT_KEY)
+    storedKey = HASH[T]($%clientKey)
+    serverKey = HMAC[T]($%saltedPassword, SERVER_KEY)
 
   result.salt = base64.encode(salt)
   result.iterations = iterations
   result.storedKey = base64.encode($%storedKey)
   result.serverKey = base64.encode($%serverKey)
 
+proc initUserData*(password: string, iterations = 4096): UserData =
+  initUserData(Sha256Digest, password, iterations)
+
 proc initUserData*(salt: string, iterations: int, serverKey, storedKey: string): UserData =
   result.salt = salt
   result.iterations = iterations
@@ -45,14 +49,19 @@ proc newScramServer*[T](): ScramServer[T] {.deprecated: "use `new ScramServer[T]
   new ScramServer[T]
 
 proc handleClientFirstMessage*[T](s: ScramServer[T],clientFirstMessage: string): string =
+  let parts = clientFirstMessage.split(',', 2)
   var matches: array[3, string]
-  if not match(clientFirstMessage, CLIENT_FIRST_MESSAGE, matches):
+  if not match(clientFirstMessage, CLIENT_FIRST_MESSAGE, matches) or not parts.len == 3:
     s.state = ENDED
     return
-  s.clientFirstMessageBare = matches[0]
-  s.serverNonce = matches[2] & makeNonce()
+  s.clientFirstMessageBare = parts[2]
+
   s.state = FIRST_CLIENT_MESSAGE_HANDLED
-  matches[1] # username
+  for kv in s.clientFirstMessageBare.split(','):
+    if kv[0..1] == "n=":
+      result = kv[2..^1]
+    elif kv[0..1] == "r=":
+      s.serverNonce = kv[2..^1] & makeNonce()
 
 proc prepareFirstMessage*(s: ScramServer, userData: UserData): string =
   s.state = FIRST_PREPARED
@@ -65,10 +74,16 @@ proc prepareFinalMessage*[T](s: ScramServer[T], clientFinalMessage: string): str
   if not match(clientFinalMessage, CLIENT_FINAL_MESSAGE, matches):
     s.state = ENDED
     return
-  let
-    clientFinalMessageWithoutProof = matches[0]
-    nonce = matches[2]
-    proof = matches[3]
+  var clientFinalMessageWithoutProof, nonce, proof: string
+  for kv in clientFinalMessage.split(','):
+    if kv[0..1] == "p=":
+      proof = kv[2..^1]
+    else:
+      if clientFinalMessageWithoutProof.len > 0:
+        clientFinalMessageWithoutProof.add(',')
+      clientFinalMessageWithoutProof.add(kv)
+      if kv[0..1] == "r=":
+        nonce = kv[2..^1]
 
   if nonce != s.serverNonce:
     s.state = ENDED
@@ -80,19 +95,21 @@ proc prepareFinalMessage*[T](s: ScramServer[T], clientFinalMessage: string): str
     clientSignature = HMAC[T](storedKey, authMessage)
     serverSignature = HMAC[T](decode(s.userData.serverKey), authMessage)
     decodedProof = base64.decode(proof)
-  var clientKey = $clientSignature
-  clientKey ^= decodedProof
+    clientKey = custom_xor(clientSignature, decodedProof)
+  let resultKey = HASH[T](clientKey).raw_str
 
-  let resultKey = $HASH[T](clientKey)
-  if resultKey != storedKey:
+  # SECURITY: constant time HMAC check
+  if not constantTimeEqual(resultKey, storedKey):
+    let k1 = base64.encode(resultKey)
+    let k2 = base64.encode(storedKey)
     return
 
   s.isSuccessful = true
   s.state = ENDED
   when NimMajor >= 1 and (NimMinor >= 1 or NimPatch >= 2):
-    "v=" & base64.encode(serverSignature)
+    result = "v=" & base64.encode(serverSignature)
   else:
-    "v=" & base64.encode(serverSignature, newLine="")
+    result = "v=" & base64.encode(serverSignature, newLine="")
 
 
 proc isSuccessful*(s: ScramServer): bool =

tests/test_both.nim

@@ -0,0 +1,26 @@
+import unittest, scram/server, scram/client, sha1, nimSHA2, base64, scram/private/[utils,types]
+
+
+proc test[T](user, password: string) =
+  var client = newScramClient[T]()
+  var server = new ScramServer[T]
+  let cfirst = client.prepareFirstMessage(user)
+  assert server.handleClientFirstMessage(cfirst) == user, "incorrect detected username"
+  assert server.state == FIRST_CLIENT_MESSAGE_HANDLED, "incorrect state"
+  let sfirst = server.prepareFirstMessage(initUserData(T, password))
+  let cfinal = client.prepareFinalMessage(password, sfirst)
+  let sfinal = server.prepareFinalMessage(cfinal)
+  assert client.verifyServerFinalMessage(sfinal), "incorrect server final message"
+
+suite "Scram Client-Server tests":
+  test "SCRAM-SHA1":
+    test[Sha1Digest](
+      "user",
+      "pencil"
+    )
+
+  test "SCRAM-SHA256":
+    test[Sha256Digest](
+      "bob",
+      "secret"
+    )

tests/test1.nim renamed to tests/test_client.nim

@@ -0,0 +1,48 @@
+import unittest, scram/server, sha1, nimSHA2, base64, scram/private/[utils,types]
+
+
+proc test[T](user, password, nonce, salt, cfirst, sfirst, cfinal, sfinal: string) =
+  var server = new ScramServer[T]
+  assert server.handleClientFirstMessage(cfirst) == user, "incorrect detected username"
+  assert server.state == FIRST_CLIENT_MESSAGE_HANDLED, "incorrect state"
+  server.serverNonce = nonce
+  let
+    iterations = 4096
+    decodedSalt = base64.decode(salt)
+    saltedPassword = hi[T](password, decodedSalt, iterations)
+    clientKey = HMAC[T]($%saltedPassword, CLIENT_KEY)
+    storedKey = HASH[T]($%clientKey)
+    serverKey = HMAC[T]($%saltedPassword, SERVER_KEY)
+    ud = UserData(
+      salt: base64.encode(decodedSalt),
+      iterations: iterations,
+      storedKey: base64.encode($%storedKey),
+      serverKey: base64.encode($%serverKey))
+  assert ud.salt == salt, "Incorrect salt initialization"
+  assert server.prepareFirstMessage(ud) == sfirst, "incorrect first message"
+  assert server.prepareFinalMessage(cfinal) == sfinal, "incorrect last message"
+
+suite "Scram Server tests":
+  test "SCRAM-SHA1":
+    test[Sha1Digest](
+      "user",
+      "pencil",
+      "fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j",
+      "QSXCR+Q6sek8bf92",
+      "n,,n=user,r=fyko+d2lbbFgONRv9qkxdawL",
+      "r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,s=QSXCR+Q6sek8bf92,i=4096",
+      "c=biws,r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,p=v0X8v3Bz2T0CJGbJQyF0X+HI4Ts=",
+      "v=rmF9pqV8S7suAoZWja4dJRkFsKQ="
+    )
+
+  test "SCRAM-SHA256":
+    test[Sha256Digest](
+      "bob",
+      "secret",
+      "VeAOLsQ22fn/tjalHQIz7cQTmeE5qJh8qKEe8wALMut1",
+      "ldZSefTzKxPNJhP73AmW/A==",
+      "n,,n=bob,r=VeAOLsQ22fn/tjalHQIz7cQT",
+      "r=VeAOLsQ22fn/tjalHQIz7cQTmeE5qJh8qKEe8wALMut1,s=ldZSefTzKxPNJhP73AmW/A==,i=4096",
+      "c=biws,r=VeAOLsQ22fn/tjalHQIz7cQTmeE5qJh8qKEe8wALMut1,p=AtNtxGzsMA8evcWBM0MXFjxN8OcG1KRkLkFyoHlupOU=",
+      "v=jeEn7M7PgnBZ7GRd+f3Ikaj40dw4EGKZ0x8FcQztLLs="
+    )