From 54c87be702b7edc88523b2b4415be9f7642e6840 Mon Sep 17 00:00:00 2001 From: Mildred Ki'Lya Date: Sat, 23 Jul 2022 11:20:51 +0200 Subject: [PATCH] Fix for nim 1.6.6 pegs matches are not longer working well and string conversion from Sha1Digest leads to debug representation instead of keeping the underlying bytes. Keep pegs for validating the format but hand parse the messages. See probable issue: Added server and client/server tests. Added constant time check for server implementation. --- scram/client.nim | 33 ++++++++-- scram/private/types.nim | 2 +- scram/private/utils.nim | 24 ++++++- scram/server.nim | 99 +++++++++++++++++++++------- tests/test_both.nim | 26 ++++++++ tests/{test1.nim => test_client.nim} | 0 tests/test_server.nim | 48 ++++++++++++++ 7 files changed, 203 insertions(+), 29 deletions(-) create mode 100644 tests/test_both.nim rename tests/{test1.nim => test_client.nim} (100%) create mode 100644 tests/test_server.nim diff --git a/scram/client.nim b/scram/client.nim index 704010d..77aa121 100644 --- a/scram/client.nim +++ b/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,16 @@ 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]) + #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 "" @@ -78,6 +86,19 @@ proc prepareFinalMessage*[T](s: ScramClient[T], password, serverFirstMessage: st var clientProof = clientKey clientProof ^= clientSignature s.state = FINAL_PREPARED + # echo &"client password {password}" + # echo &"client salt {salt}" + # echo &"client iterations {iterations}" + # echo &"client saltedPassword {base64.encode(saltedPassword)}" + # echo &"client clientKey {base64.encode(clientKey)}" + # echo &"client storedKey {base64.encode(storedKey)}" + # echo &"client serverKey {base64.encode(serverKey)}" + # echo &"client authMessage.1 {s.clientFirstMessageBare}" + # echo &"client authMessage.2 {serverFirstMessage}" + # echo &"client authMessage.3 {clientFinalMessageWithoutProof}" + # echo &"client authMessage {authMessage}" + # echo &"client clientSignature {base64.encode(clientSignature)}" + # echo &"client clientProof {base64.encode(clientProof)}" when NimMajor >= 1 and (NimMinor >= 1 or NimPatch >= 2): clientFinalMessageWithoutProof & ",p=" & base64.encode(clientProof) else: @@ -89,7 +110,11 @@ 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]) + #let proposedServerSignature = base64.decode(matches[0]) s.isSuccessful = proposedServerSignature == $%s.serverSignature s.isSuccessful diff --git a/scram/private/types.nim b/scram/private/types.nim index c38a8ee..e916bc5 100644 --- a/scram/private/types.nim +++ b/scram/private/types.nim @@ -1,5 +1,5 @@ type - ScramError* = object of Exception + ScramError* = object of CatchableError DigestType* = enum MD5 diff --git a/scram/private/utils.nim b/scram/private/utils.nim index 267885c..d7225db 100644 --- a/scram/private/utils.nim +++ b/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.. + #s.serverNonce = matches[2] & makeNonce() + #echo &"s.serverNonce = {s.serverNonce}" + #echo &"username = {matches[1]}" + #s.state = FIRST_CLIENT_MESSAGE_HANDLED + #matches[1] # username + 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 s.userData = userData s.serverFirstMessage = "r=$#,s=$#,i=$#" % [s.serverNonce, userData.salt, $userData.iterations] + # echo &"server first message: {s.serverFirstMessage}" s.serverFirstMessage proc prepareFinalMessage*[T](s: ScramServer[T], clientFinalMessage: string): string = var matches: array[4, string] + # echo &"client final message {clientFinalMessage}" if not match(clientFinalMessage, CLIENT_FINAL_MESSAGE, matches): s.state = ENDED return - let - clientFinalMessageWithoutProof = matches[0] - nonce = matches[2] - proof = matches[3] + # echo &"client final message matches {matches}" + #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 + # echo &"nonce mismatch {nonce} != {s.serverNonce}" return let @@ -80,19 +120,34 @@ 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 - - let resultKey = $HASH[T](clientKey) - if resultKey != storedKey: + clientKey = custom_xor(clientSignature, decodedProof) + #var clientKey = $clientSignature + #clientKey ^= decodedProof + let resultKey = HASH[T](clientKey).raw_str + # echo &"server storedKey {base64.encode(storedKey)}" + # echo &"server resultKey {base64.encode(resultKey)}" + # echo &"server authMessage.1 {s.clientFirstMessageBare}" + # echo &"server authMessage.2 {s.serverFirstMessage}" + # echo &"server authMessage.3 {clientFinalMessageWithoutProof}" + # echo &"server authMessage {authMessage}" + # echo &"server clientSignature {base64.encode(clientSignature)}" + # echo &"server clientKey {base64.encode(clientKey)} .len = {clientKey.len} {$typeof(clientSignature)}" + # echo &"server decodedProof {base64.encode(decodedProof)} .len = {decodedProof.len}" + + # SECURITY: constant time HMAC check + if not constantTimeEqual(resultKey, storedKey): + let k1 = base64.encode(resultKey) + let k2 = base64.encode(storedKey) + # echo &"key mismatch {k1} != {k2}" 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="") + # echo &"server final message: {result}" proc isSuccessful*(s: ScramServer): bool = diff --git a/tests/test_both.nim b/tests/test_both.nim new file mode 100644 index 0000000..1f6ae93 --- /dev/null +++ b/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" + ) diff --git a/tests/test1.nim b/tests/test_client.nim similarity index 100% rename from tests/test1.nim rename to tests/test_client.nim diff --git a/tests/test_server.nim b/tests/test_server.nim new file mode 100644 index 0000000..2f82fc2 --- /dev/null +++ b/tests/test_server.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=" + )