Skip to content

Commit 54c87be

Browse files
committed
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: <nim-lang/Nim#19104> Added server and client/server tests. Added constant time check for server implementation.
1 parent ae3006a commit 54c87be

File tree

7 files changed

+203
-29
lines changed

7 files changed

+203
-29
lines changed

scram/client.nim

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import strformat
12
import base64, pegs, strutils, hmac, sha1, nimSHA2, md5, private/[utils,types]
23

34
export MD5Digest, SHA1Digest, SHA224Digest, SHA256Digest, SHA384Digest, SHA512Digest, Keccak512Digest
@@ -53,9 +54,16 @@ proc prepareFinalMessage*[T](s: ScramClient[T], password, serverFirstMessage: st
5354
iterations: int
5455
var matches: array[3, string]
5556
if match(serverFirstMessage, SERVER_FIRST_MESSAGE, matches):
56-
nonce = matches[0]
57-
salt = base64.decode(matches[1])
58-
iterations = parseInt(matches[2])
57+
#nonce = matches[0]
58+
#salt = base64.decode(matches[1])
59+
#iterations = parseInt(matches[2])
60+
for kv in serverFirstMessage.split(','):
61+
if kv[0..1] == "i=":
62+
iterations = parseInt(kv[2..^1])
63+
elif kv[0..1] == "r=":
64+
nonce = kv[2..^1]
65+
elif kv[0..1] == "s=":
66+
salt = base64.decode(kv[2..^1])
5967
else:
6068
s.state = ENDED
6169
return ""
@@ -78,6 +86,19 @@ proc prepareFinalMessage*[T](s: ScramClient[T], password, serverFirstMessage: st
7886
var clientProof = clientKey
7987
clientProof ^= clientSignature
8088
s.state = FINAL_PREPARED
89+
# echo &"client password {password}"
90+
# echo &"client salt {salt}"
91+
# echo &"client iterations {iterations}"
92+
# echo &"client saltedPassword {base64.encode(saltedPassword)}"
93+
# echo &"client clientKey {base64.encode(clientKey)}"
94+
# echo &"client storedKey {base64.encode(storedKey)}"
95+
# echo &"client serverKey {base64.encode(serverKey)}"
96+
# echo &"client authMessage.1 {s.clientFirstMessageBare}"
97+
# echo &"client authMessage.2 {serverFirstMessage}"
98+
# echo &"client authMessage.3 {clientFinalMessageWithoutProof}"
99+
# echo &"client authMessage {authMessage}"
100+
# echo &"client clientSignature {base64.encode(clientSignature)}"
101+
# echo &"client clientProof {base64.encode(clientProof)}"
81102
when NimMajor >= 1 and (NimMinor >= 1 or NimPatch >= 2):
82103
clientFinalMessageWithoutProof & ",p=" & base64.encode(clientProof)
83104
else:
@@ -89,7 +110,11 @@ proc verifyServerFinalMessage*(s: ScramClient, serverFinalMessage: string): bool
89110
s.state = ENDED
90111
var matches: array[1, string]
91112
if match(serverFinalMessage, SERVER_FINAL_MESSAGE, matches):
92-
let proposedServerSignature = base64.decode(matches[0])
113+
var proposedServerSignature: string
114+
for kv in serverFinalMessage.split(','):
115+
if kv[0..1] == "v=":
116+
proposedServerSignature = base64.decode(kv[2..^1])
117+
#let proposedServerSignature = base64.decode(matches[0])
93118
s.isSuccessful = proposedServerSignature == $%s.serverSignature
94119
s.isSuccessful
95120

scram/private/types.nim

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
type
2-
ScramError* = object of Exception
2+
ScramError* = object of CatchableError
33

44
DigestType* = enum
55
MD5

scram/private/utils.nim

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import random, base64, strutils, types, hmac
1+
import random, base64, strutils, types, hmac, bitops
22
from md5 import MD5Digest
33
from sha1 import Sha1Digest
44
from nimSHA2 import Sha224Digest, Sha256Digest, Sha384Digest, Sha512Digest
@@ -20,6 +20,21 @@ template `^=`*[T](a, b: T) =
2020
else:
2121
a[x] = (a[x].int32 xor b[x].int32).char
2222

23+
proc custom_xor*[T](bytes: T, str: string): string =
24+
if bytes.len != str.len:
25+
raise newException(RangeDefect, "xor must have both arguments of the same size")
26+
result = str
27+
for x in 0..<bytes.len:
28+
result[x] = (bytes[x].uint8 xor str[x].uint8).char
29+
30+
proc constantTimeEqual*(a, b: string): bool =
31+
if a.len != b.len:
32+
raise newException(RangeDefect, "must have both arguments of the same size")
33+
var res: uint8 = 0
34+
for x in 0..<a.len:
35+
res = bitor(res, bitxor(a[x].uint8, b[x].uint8))
36+
result = (res == 0)
37+
2338
proc HMAC*[T](password, salt: string): T =
2439
when T is MD5Digest:
2540
result = hmac_md5(password, salt)
@@ -36,6 +51,12 @@ proc HMAC*[T](password, salt: string): T =
3651
elif T is Keccak512Digest:
3752
result = hmac_keccak512(password, salt)
3853

54+
proc raw_str*[T](digest: T): string =
55+
when T is Sha1Digest:
56+
for c in digest: result.add(char(c))
57+
else:
58+
result = $digest
59+
3960
proc HASH*[T](s: string): T =
4061
when T is MD5Digest:
4162
result = hash_md5(s)
@@ -57,7 +78,6 @@ proc debug[T](s: T): string =
5778
for x in s:
5879
result.add strutils.toHex(x.uint8).toLowerAscii
5980

60-
6181
proc hi*[T](password, salt: string, iterations: int): T =
6282
var previous = HMAC[T](password, salt & INT_1)
6383
result = previous

scram/server.nim

Lines changed: 77 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1+
import strformat, strutils
12
import base64, pegs, strutils, hmac, nimSHA2, private/[utils,types]
23

34
type
45
ScramServer*[T] = ref object of RootObj
5-
serverNonce: string
6+
serverNonce*: string
67
clientFirstMessageBare: string
78
serverFirstMessage: string
8-
state: ScramState
9+
state*: ScramState
910
isSuccessful: bool
1011
userData: UserData
1112

@@ -19,22 +20,33 @@ let
1920
CLIENT_FIRST_MESSAGE = peg"^([pny]'='?([^,]*)','([^,]*)','){('m='([^,]*)',')?'n='{[^,]*}',r='{[^,]*}(','(.*))*}$"
2021
CLIENT_FINAL_MESSAGE = peg"{'c='{[^,]*}',r='{[^,]*}}',p='{.*}$"
2122

22-
proc initUserData*(password: string, iterations = 4096): UserData =
23+
proc initUserData*[T](typ: typedesc[T], password: string, iterations = 4096): UserData =
2324
var iterations = iterations
2425
if password.len == 0:
2526
iterations = 1
2627
let
2728
salt = makeNonce()[0..24]
28-
saltedPassword = hi[SHA256Digest](password, salt, iterations)
29-
clientKey = HMAC[SHA256Digest]($%saltedPassword, CLIENT_KEY)
30-
storedKey = HASH[SHA256Digest]($%clientKey)
31-
serverKey = HMAC[SHA256Digest]($%saltedPassword, SERVER_KEY)
29+
saltedPassword = hi[T](password, salt, iterations)
30+
clientKey = HMAC[T]($%saltedPassword, CLIENT_KEY)
31+
storedKey = HASH[T]($%clientKey)
32+
serverKey = HMAC[T]($%saltedPassword, SERVER_KEY)
33+
34+
# echo &"server password {password}"
35+
# echo &"server salt {salt}"
36+
# echo &"server iterations {iterations}"
37+
# echo &"server saltedPassword {base64.encode(saltedPassword)}"
38+
# echo &"server clientKey {base64.encode(clientKey)}"
39+
# echo &"server serverKey {base64.encode(serverKey)}"
40+
# echo &"server storedKey {base64.encode(storedKey)}"
3241

3342
result.salt = base64.encode(salt)
3443
result.iterations = iterations
3544
result.storedKey = base64.encode($%storedKey)
3645
result.serverKey = base64.encode($%serverKey)
3746

47+
proc initUserData*(password: string, iterations = 4096): UserData =
48+
initUserData(Sha256Digest, password, iterations)
49+
3850
proc initUserData*(salt: string, iterations: int, serverKey, storedKey: string): UserData =
3951
result.salt = salt
4052
result.iterations = iterations
@@ -45,33 +57,61 @@ proc newScramServer*[T](): ScramServer[T] {.deprecated: "use `new ScramServer[T]
4557
new ScramServer[T]
4658

4759
proc handleClientFirstMessage*[T](s: ScramServer[T],clientFirstMessage: string): string =
60+
let parts = clientFirstMessage.split(',', 2)
4861
var matches: array[3, string]
49-
if not match(clientFirstMessage, CLIENT_FIRST_MESSAGE, matches):
62+
# echo &"client first message {clientFirstMessage}"
63+
if not match(clientFirstMessage, CLIENT_FIRST_MESSAGE, matches) or not parts.len == 3:
5064
s.state = ENDED
5165
return
52-
s.clientFirstMessageBare = matches[0]
53-
s.serverNonce = matches[2] & makeNonce()
66+
# echo &"client first message matches {matches}"
67+
s.clientFirstMessageBare = parts[2]
68+
# Disabled code until this is resolved
69+
# <https://github.com/nim-lang/Nim/issues/19104>
70+
#s.serverNonce = matches[2] & makeNonce()
71+
#echo &"s.serverNonce = {s.serverNonce}"
72+
#echo &"username = {matches[1]}"
73+
#s.state = FIRST_CLIENT_MESSAGE_HANDLED
74+
#matches[1] # username
75+
5476
s.state = FIRST_CLIENT_MESSAGE_HANDLED
55-
matches[1] # username
77+
for kv in s.clientFirstMessageBare.split(','):
78+
if kv[0..1] == "n=":
79+
result = kv[2..^1]
80+
elif kv[0..1] == "r=":
81+
s.serverNonce = kv[2..^1] & makeNonce()
5682

5783
proc prepareFirstMessage*(s: ScramServer, userData: UserData): string =
5884
s.state = FIRST_PREPARED
5985
s.userData = userData
6086
s.serverFirstMessage = "r=$#,s=$#,i=$#" % [s.serverNonce, userData.salt, $userData.iterations]
87+
# echo &"server first message: {s.serverFirstMessage}"
6188
s.serverFirstMessage
6289

6390
proc prepareFinalMessage*[T](s: ScramServer[T], clientFinalMessage: string): string =
6491
var matches: array[4, string]
92+
# echo &"client final message {clientFinalMessage}"
6593
if not match(clientFinalMessage, CLIENT_FINAL_MESSAGE, matches):
6694
s.state = ENDED
6795
return
68-
let
69-
clientFinalMessageWithoutProof = matches[0]
70-
nonce = matches[2]
71-
proof = matches[3]
96+
# echo &"client final message matches {matches}"
97+
#let
98+
# clientFinalMessageWithoutProof = matches[0]
99+
# nonce = matches[2]
100+
# proof = matches[3]
101+
var clientFinalMessageWithoutProof, nonce, proof: string
102+
for kv in clientFinalMessage.split(','):
103+
if kv[0..1] == "p=":
104+
proof = kv[2..^1]
105+
else:
106+
if clientFinalMessageWithoutProof.len > 0:
107+
clientFinalMessageWithoutProof.add(',')
108+
clientFinalMessageWithoutProof.add(kv)
109+
if kv[0..1] == "r=":
110+
nonce = kv[2..^1]
72111

73112
if nonce != s.serverNonce:
74113
s.state = ENDED
114+
# echo &"nonce mismatch {nonce} != {s.serverNonce}"
75115
return
76116

77117
let
@@ -80,19 +120,34 @@ proc prepareFinalMessage*[T](s: ScramServer[T], clientFinalMessage: string): str
80120
clientSignature = HMAC[T](storedKey, authMessage)
81121
serverSignature = HMAC[T](decode(s.userData.serverKey), authMessage)
82122
decodedProof = base64.decode(proof)
83-
var clientKey = $clientSignature
84-
clientKey ^= decodedProof
85-
86-
let resultKey = $HASH[T](clientKey)
87-
if resultKey != storedKey:
123+
clientKey = custom_xor(clientSignature, decodedProof)
124+
#var clientKey = $clientSignature
125+
#clientKey ^= decodedProof
126+
let resultKey = HASH[T](clientKey).raw_str
127+
# echo &"server storedKey {base64.encode(storedKey)}"
128+
# echo &"server resultKey {base64.encode(resultKey)}"
129+
# echo &"server authMessage.1 {s.clientFirstMessageBare}"
130+
# echo &"server authMessage.2 {s.serverFirstMessage}"
131+
# echo &"server authMessage.3 {clientFinalMessageWithoutProof}"
132+
# echo &"server authMessage {authMessage}"
133+
# echo &"server clientSignature {base64.encode(clientSignature)}"
134+
# echo &"server clientKey {base64.encode(clientKey)} .len = {clientKey.len} {$typeof(clientSignature)}"
135+
# echo &"server decodedProof {base64.encode(decodedProof)} .len = {decodedProof.len}"
136+
137+
# SECURITY: constant time HMAC check
138+
if not constantTimeEqual(resultKey, storedKey):
139+
let k1 = base64.encode(resultKey)
140+
let k2 = base64.encode(storedKey)
141+
# echo &"key mismatch {k1} != {k2}"
88142
return
89143

90144
s.isSuccessful = true
91145
s.state = ENDED
92146
when NimMajor >= 1 and (NimMinor >= 1 or NimPatch >= 2):
93-
"v=" & base64.encode(serverSignature)
147+
result = "v=" & base64.encode(serverSignature)
94148
else:
95-
"v=" & base64.encode(serverSignature, newLine="")
149+
result = "v=" & base64.encode(serverSignature, newLine="")
150+
# echo &"server final message: {result}"
96151

97152

98153
proc isSuccessful*(s: ScramServer): bool =

tests/test_both.nim

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import unittest, scram/server, scram/client, sha1, nimSHA2, base64, scram/private/[utils,types]
2+
3+
4+
proc test[T](user, password: string) =
5+
var client = newScramClient[T]()
6+
var server = new ScramServer[T]
7+
let cfirst = client.prepareFirstMessage(user)
8+
assert server.handleClientFirstMessage(cfirst) == user, "incorrect detected username"
9+
assert server.state == FIRST_CLIENT_MESSAGE_HANDLED, "incorrect state"
10+
let sfirst = server.prepareFirstMessage(initUserData(T, password))
11+
let cfinal = client.prepareFinalMessage(password, sfirst)
12+
let sfinal = server.prepareFinalMessage(cfinal)
13+
assert client.verifyServerFinalMessage(sfinal), "incorrect server final message"
14+
15+
suite "Scram Client-Server tests":
16+
test "SCRAM-SHA1":
17+
test[Sha1Digest](
18+
"user",
19+
"pencil"
20+
)
21+
22+
test "SCRAM-SHA256":
23+
test[Sha256Digest](
24+
"bob",
25+
"secret"
26+
)
File renamed without changes.

tests/test_server.nim

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import unittest, scram/server, sha1, nimSHA2, base64, scram/private/[utils,types]
2+
3+
4+
proc test[T](user, password, nonce, salt, cfirst, sfirst, cfinal, sfinal: string) =
5+
var server = new ScramServer[T]
6+
assert server.handleClientFirstMessage(cfirst) == user, "incorrect detected username"
7+
assert server.state == FIRST_CLIENT_MESSAGE_HANDLED, "incorrect state"
8+
server.serverNonce = nonce
9+
let
10+
iterations = 4096
11+
decodedSalt = base64.decode(salt)
12+
saltedPassword = hi[T](password, decodedSalt, iterations)
13+
clientKey = HMAC[T]($%saltedPassword, CLIENT_KEY)
14+
storedKey = HASH[T]($%clientKey)
15+
serverKey = HMAC[T]($%saltedPassword, SERVER_KEY)
16+
ud = UserData(
17+
salt: base64.encode(decodedSalt),
18+
iterations: iterations,
19+
storedKey: base64.encode($%storedKey),
20+
serverKey: base64.encode($%serverKey))
21+
assert ud.salt == salt, "Incorrect salt initialization"
22+
assert server.prepareFirstMessage(ud) == sfirst, "incorrect first message"
23+
assert server.prepareFinalMessage(cfinal) == sfinal, "incorrect last message"
24+
25+
suite "Scram Server tests":
26+
test "SCRAM-SHA1":
27+
test[Sha1Digest](
28+
"user",
29+
"pencil",
30+
"fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j",
31+
"QSXCR+Q6sek8bf92",
32+
"n,,n=user,r=fyko+d2lbbFgONRv9qkxdawL",
33+
"r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,s=QSXCR+Q6sek8bf92,i=4096",
34+
"c=biws,r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,p=v0X8v3Bz2T0CJGbJQyF0X+HI4Ts=",
35+
"v=rmF9pqV8S7suAoZWja4dJRkFsKQ="
36+
)
37+
38+
test "SCRAM-SHA256":
39+
test[Sha256Digest](
40+
"bob",
41+
"secret",
42+
"VeAOLsQ22fn/tjalHQIz7cQTmeE5qJh8qKEe8wALMut1",
43+
"ldZSefTzKxPNJhP73AmW/A==",
44+
"n,,n=bob,r=VeAOLsQ22fn/tjalHQIz7cQT",
45+
"r=VeAOLsQ22fn/tjalHQIz7cQTmeE5qJh8qKEe8wALMut1,s=ldZSefTzKxPNJhP73AmW/A==,i=4096",
46+
"c=biws,r=VeAOLsQ22fn/tjalHQIz7cQTmeE5qJh8qKEe8wALMut1,p=AtNtxGzsMA8evcWBM0MXFjxN8OcG1KRkLkFyoHlupOU=",
47+
"v=jeEn7M7PgnBZ7GRd+f3Ikaj40dw4EGKZ0x8FcQztLLs="
48+
)

0 commit comments

Comments
 (0)