Skip to content

Commit e85ea98

Browse files
author
Colin James
committed
CP-50518: Add quicktests for crypt_r
The following small tests are provided as a quicktest suite to run on hosts: - Valid salts: a few valid salts are confirmed to compute a hash. - Invalid salts: a few erronerous salts are confirmed to fail. - Implicit salt truncation: the behaviour that any salt longer than a specified maximum (at present, 16) does not cause hash computation to fail, but rather its value gets implicitly truncated by the algorithm. - Increasing string length: strings from language 'a'+ are tested in increasing length to ensure they compute to a set of hashes that are pairwise distinct. This test is really to ensure that the algorithm does not cap the maximum key length - which would be implicitly truncated on our behalf and cause lengths over a certain threshold to hash to equivalent values. This property is worth tracking, currently it would appear there is no limit to worry about. - C style termination: OCaml strings don't rely on a C-style null terminator character ('\0') to determine their length. Consequently, '\0' can appear anywhere in OCaml strings without impeding various computations on strings. This test exercises the property that the C API expectedly does stop reading after seeing '\0'. This property should not be relied upon and its behaviour is documented here as a test. - Multiple threads hashing simultaneously: this test exercises the property that crypt_r truly is re-entrant. The test spawns a few threads to compute an initial hash and then each thread spends ~200ms attempting to compute the same hash again. If any hash computed within this ~200ms window differs from the initial hash, or any hash fails to be computed, the test fails. This property must be tested because the C stub temporarily relinquishes the runtime lock when computing the hash, thus allowing the possibility that hashes are computed in parallel. It is not committed here but this test was shown to be well formed by using the non-reentrant "crypt" function in place of crypt_r, where the test reliably fails. Switching this test over crypt_r allows it to pass. It is important that the tests are actually executed on a host - as the implementation of crypt_r (from libcrypt.so) could differ across development and target machines, but also across updates to XenServer. Some of the tests are for regression purposes only, to ensure that certain expectations are retained from time of writing. Signed-off-by: Colin James <colin.barr@cloud.com>
1 parent 2fa3eb1 commit e85ea98

File tree

3 files changed

+205
-0
lines changed

3 files changed

+205
-0
lines changed

ocaml/quicktest/dune

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
http_lib
1414
mtime
1515
mtime.clock.os
16+
pam
1617
qcheck-alcotest
1718
result
1819
rresult

ocaml/quicktest/quicktest.ml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ let () =
4242
; ("Quicktest_max_vdi_size", Quicktest_max_vdi_size.tests ())
4343
; ("Quicktest_static_vdis", Quicktest_static_vdis.tests ())
4444
; ("Quicktest_date", Quicktest_date.tests ())
45+
; ("Quicktest_crypt_r", Quicktest_crypt_r.tests ())
4546
]
4647
@
4748
if not !Quicktest_args.using_unix_domain_socket then

ocaml/quicktest/quicktest_crypt_r.ml

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
(*
2+
* Copyright (c) Cloud Software Group, Inc
3+
*
4+
* This program is free software; you can redistribute it and/or modify
5+
* it under the terms of the GNU Lesser General Public License as published
6+
* by the Free Software Foundation; version 2.1 only. with the special
7+
* exception on linking described in file LICENSE.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
* GNU Lesser General Public License for more details.
13+
*)
14+
15+
let valid_salts =
16+
[
17+
"salt" (* Don't need to specify algorithm, will default to something. *)
18+
; "$5$salt$" (* 5 = SHA-256 should work. *)
19+
; "$6$salt$" (* 6 = SHA-512 should work. *)
20+
]
21+
22+
let invalid_salts =
23+
[
24+
"" (* Salt cannot be empty. *)
25+
; "$" (* Salt cannot be $. *)
26+
; "$9$salt$" (* Salt must specify valid algorithm constant. *)
27+
; "$6,rounds=1000$salt$" (* Salt cannot specify iteration count. *)
28+
; "£6£salt£" (* Only American currency is legal tender. *)
29+
]
30+
31+
let test_salts ~msg ~succeeds salts =
32+
let test salt =
33+
let actual = Option.is_some (Pam.crypt_r ~key:"password" ~setting:salt) in
34+
Alcotest.((check' bool) ~msg ~expected:succeeds ~actual)
35+
in
36+
List.iter test salts
37+
38+
let test_valid_salts () =
39+
test_salts ~msg:"Hash can be computed from valid salt" ~succeeds:true
40+
valid_salts
41+
42+
let test_invalid_salts () =
43+
test_salts ~msg:"Hash cannot be computed from invalid salt" ~succeeds:false
44+
invalid_salts
45+
46+
let test_salt_truncation () =
47+
let salt_max_length = 16 in
48+
let salt = "a_salt_that_is_longer_than_is_actually_accepted" in
49+
assert (String.length salt > salt_max_length) ;
50+
let test prefix_length =
51+
(* The C API accepts at most 16 chars for the salt, optionally
52+
enclosed within $k$salt$ - anything else is ignored (implicitly
53+
truncated). *)
54+
let truncated_salt = String.sub salt 0 prefix_length in
55+
let sha512 = Printf.sprintf "$6$%s$" in
56+
let key = "password" in
57+
let h = Pam.crypt_r ~key ~setting:(sha512 salt) in
58+
let h' = Pam.crypt_r ~key ~setting:(sha512 truncated_salt) in
59+
if Option.(is_none h || is_none h') then
60+
failwith (Printf.sprintf "Failed to compute hash in %s" __FUNCTION__)
61+
else
62+
Option.equal ( = ) h h'
63+
in
64+
let msg =
65+
Printf.sprintf
66+
"Hash computed with implicitly truncated salt is the same as explicitly \
67+
truncated (len = %d)\n\
68+
."
69+
in
70+
let expectation len =
71+
(* We expect all lengths greater than max salt length to succeed,
72+
as they are implicitly truncated. Any length < salt_max_length
73+
should fail. *)
74+
len >= salt_max_length
75+
in
76+
for len = 0 to String.length salt do
77+
let msg = msg len in
78+
let actual = test len in
79+
let expected = expectation len in
80+
Alcotest.(check' bool) ~msg ~expected ~actual
81+
done
82+
83+
(* Invalidate the following tests if any hash fails to be computed. *)
84+
let crypt_r ~key ~setting =
85+
match Pam.crypt_r ~key ~setting with
86+
| Some hash ->
87+
hash
88+
| _ ->
89+
failwith "Invalid input provided to crypt_r"
90+
91+
let test_crypt_r_many_threads () =
92+
Printexc.record_backtrace true ;
93+
let lock = Mutex.create () in
94+
let c = Condition.create () in
95+
let ready = ref false in
96+
let values = List.map (fun salt -> ("password", salt)) valid_salts in
97+
(* Each thread will populate an entry in results. *)
98+
let results : (unit, _) result array =
99+
Array.make (List.length values) (Ok ())
100+
in
101+
let spawn i (key, setting) =
102+
(* Computes an initial hash and then repeatedly attempts to
103+
computes the same hash to ensure its value does not change even
104+
when interleaved with other threads doing the same. *)
105+
let run () =
106+
Mutex.lock lock ;
107+
let initial =
108+
try crypt_r ~key ~setting
109+
with exn ->
110+
(* If initial hash fails, release lock and propagate
111+
exception. *)
112+
Mutex.unlock lock ; raise exn
113+
in
114+
while not !ready do
115+
Condition.wait c lock
116+
done ;
117+
Mutex.unlock lock ;
118+
(* Repeatedly compute the same initial hash for ~200ms. *)
119+
let now = Unix.gettimeofday in
120+
let start = now () in
121+
while now () -. start < 0.2 do
122+
let after = crypt_r ~key ~setting in
123+
if initial <> after then
124+
failwith (Printf.sprintf "%s <> %s" initial after)
125+
done
126+
in
127+
(* Run the test, capturing any exception as a result to the
128+
negative. *)
129+
results.(i) <- Rresult.R.trap_exn run ()
130+
in
131+
(* Spawn a thread per valid test case, then signal them all to start. *)
132+
Mutex.lock lock ;
133+
let tids = List.mapi (fun i -> Thread.create (spawn i)) values in
134+
ready := true ;
135+
Condition.broadcast c ;
136+
Mutex.unlock lock ;
137+
List.iter Thread.join tids ;
138+
(* Re-raise the first encountered trapped exception with its
139+
backtrace to ensure the test fails if any thread reported
140+
failure. *)
141+
let reraise = function
142+
| Error (`Exn_trap (exn, bt)) ->
143+
Printexc.raise_with_backtrace exn bt
144+
| _ ->
145+
()
146+
in
147+
Array.iter reraise results
148+
149+
(* This test hashes strings of language 'a'+ over a small range of lengths to
150+
ensure no duplicates occur. A suitable cryptographic hash function should have
151+
no collisions doing this. So, if a collision occurs, it is more likely because
152+
the underlying algorithm has a maximum length key size (and is truncating our
153+
input). *)
154+
let test_increasing_length () =
155+
let min, max = (50, 140) in
156+
(* Records hash -> length, so colliding lengths can be reported. *)
157+
let tbl = Hashtbl.create 127 in
158+
let setting = "$6$salt$" in
159+
let go len =
160+
let key = String.make len 'a' in
161+
let hash =
162+
try crypt_r ~key ~setting
163+
with _ ->
164+
failwith (Printf.sprintf "Failed to compute hash aa..a of length %d" len)
165+
in
166+
match Hashtbl.find_opt tbl hash with
167+
| Some len' ->
168+
failwith
169+
(Printf.sprintf "Hash value a.. (len = %d) matches a.. (len %d)" len
170+
len'
171+
)
172+
| _ ->
173+
Hashtbl.add tbl hash len
174+
in
175+
for i = min to max do
176+
go i
177+
done
178+
179+
(* This test demonstrates the behaviour that the C API will
180+
(expectedly) only read up to the null terminator character. OCaml
181+
strings are stored as an array of words, with the final byte
182+
specifying how many padding bytes precede it. Since the number of
183+
words and number of padding bytes is used to determine string length,
184+
there is no reliance on a C-style null terminator - so '\0' can appear
185+
anywhere in an OCaml string. *)
186+
let test_c_truncation () =
187+
let key = "password" in
188+
let key' = key ^ "\x00_arbitrary_data_here" in
189+
let setting = "$6$salt$" in
190+
let hash = crypt_r ~key ~setting in
191+
let hash' = crypt_r ~key:key' ~setting in
192+
if hash <> hash' then
193+
failwith "Expected truncation using C-style null termination failed"
194+
195+
let tests () =
196+
[
197+
("Valid salts", `Quick, test_valid_salts)
198+
; ("Invalid salts", `Quick, test_invalid_salts)
199+
; ("Implicit salt truncation", `Quick, test_salt_truncation)
200+
; ("Increasing string length", `Quick, test_increasing_length)
201+
; ("C-style termination", `Quick, test_c_truncation)
202+
; ("Multiple threads", `Quick, test_crypt_r_many_threads)
203+
]

0 commit comments

Comments
 (0)