From 3855d7ea022caf02b5daf4a59827862ed16529cd Mon Sep 17 00:00:00 2001 From: Markus Triska Date: Sun, 17 May 2020 22:01:22 +0200 Subject: [PATCH 1/8] type test for salt in crypto_password_hash/3 --- src/prolog/lib/crypto.pl | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/prolog/lib/crypto.pl b/src/prolog/lib/crypto.pl index b93a9cff..be6b6b31 100644 --- a/src/prolog/lib/crypto.pl +++ b/src/prolog/lib/crypto.pl @@ -379,7 +379,7 @@ crypto_password_hash(Password0, Hash, Options) :- Algorithm = 'pbkdf2-sha512', % current default and only option option(algorithm(Algorithm), Options, Algorithm), ( member(salt(SaltBytes), Options) -> - true + must_be_bytes(SaltBytes, crypto_password_hash/2) ; crypto_n_random_bytes(16, SaltBytes) ), '$crypto_password_hash'(Password, SaltBytes, Iterations, HashBytes), @@ -601,8 +601,6 @@ encoding_bytes(utf8, Cs, Bs) :- ; domain_error(encryption_encoding, Cs, crypto) ). -char_code(Char, Code) :- atom_codes(Char, [Code]). - /* - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Modular multiplicative inverse. From 95ca8a263007666fb210ed8ff6be6429c78b8bc2 Mon Sep 17 00:00:00 2001 From: Markus Triska Date: Mon, 18 May 2020 09:58:09 +0200 Subject: [PATCH 2/8] throw instantiation error if the list of options contains a variable (#523) Many thanks to @notoria for the test case! --- src/prolog/lib/crypto.pl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/prolog/lib/crypto.pl b/src/prolog/lib/crypto.pl index be6b6b31..7a246432 100644 --- a/src/prolog/lib/crypto.pl +++ b/src/prolog/lib/crypto.pl @@ -259,6 +259,10 @@ crypto_data_hkdf(Data0, L, Bytes, Options0) :- '$crypto_data_hkdf'(Data, SaltBytes, Info, Algorithm, L, Bytes). option(What, Options, Default) :- + ( member(V, Options), var(V) -> + instantiation_error(option/3) + ; true + ), ( member(What, Options) -> true ; What =.. [_,Default] ). From 70ad44adfd7110d7627949e4be29230aff784737 Mon Sep 17 00:00:00 2001 From: Markus Triska Date: Mon, 18 May 2020 11:12:59 +0200 Subject: [PATCH 3/8] type check for length argument in crypto_data_hkdf/4 Reported by @notoria in #527. --- src/prolog/lib/crypto.pl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/prolog/lib/crypto.pl b/src/prolog/lib/crypto.pl index 7a246432..419c3d4c 100644 --- a/src/prolog/lib/crypto.pl +++ b/src/prolog/lib/crypto.pl @@ -250,6 +250,8 @@ hash_algorithm(sha512_256). crypto_data_hkdf(Data0, L, Bytes, Options0) :- functor_hash_options(algorithm, Algorithm, Options0, Options), + must_be(integer, L), + L >= 0, option(encoding(Encoding), Options, utf8), encoding_bytes(Encoding, Data0, Data), option(salt(SaltBytes), Options, []), From 23034dd4f54b45c9a707e8f8a6d37dffd0fe7a01 Mon Sep 17 00:00:00 2001 From: Markus Triska Date: Mon, 18 May 2020 11:21:38 +0200 Subject: [PATCH 4/8] raise instantiation errors for variable encoding Reported by notoria in #527. Note that from a declarative perspective, it would indeed be valid to give answers for both available encodings. --- src/prolog/lib/crypto.pl | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/prolog/lib/crypto.pl b/src/prolog/lib/crypto.pl index 419c3d4c..bbb80a60 100644 --- a/src/prolog/lib/crypto.pl +++ b/src/prolog/lib/crypto.pl @@ -184,7 +184,7 @@ crypto_random_byte(B) :- '$crypto_random_byte'(B). crypto_data_hash(Data0, Hash, Options0) :- must_be(list, Options0), - option(encoding(Encoding), Options0, utf8), + encoding_options(Encoding, Options0), encoding_bytes(Encoding, Data0, Data), functor_hash_options(algorithm, A, Options0, _), ( hash_algorithm(A) -> true @@ -193,6 +193,9 @@ crypto_data_hash(Data0, Hash, Options0) :- '$crypto_data_hash'(Data, HashBytes, A), hex_bytes(Hash, HashBytes). +encoding_options(Encoding, Options) :- + option(encoding(Encoding), Options, utf8), + must_be(atom, Encoding). default_hash(sha256). @@ -252,7 +255,7 @@ crypto_data_hkdf(Data0, L, Bytes, Options0) :- functor_hash_options(algorithm, Algorithm, Options0, Options), must_be(integer, L), L >= 0, - option(encoding(Encoding), Options, utf8), + encoding_options(Encoding, Options), encoding_bytes(Encoding, Data0, Data), option(salt(SaltBytes), Options, []), must_be_bytes(SaltBytes, crypto_data_hkdf/4), @@ -537,7 +540,7 @@ bytes_base64_([A,B,C|Ls]) --> [W,X,Y,Z], - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - */ crypto_data_encrypt(PlainText0, Algorithm, Key, IV, CipherText, Options) :- - option(encoding(Encoding), Options, utf8), + encoding_options(Encoding, Options), encoding_bytes(Encoding, PlainText0, PlainText), option(tag(Tag), Options, _), ( nonvar(Tag) -> @@ -586,7 +589,7 @@ crypto_data_decrypt(CipherText0, Algorithm, Key, IV, PlainText, Options) :- must_be_bytes(Key, crypto_data_decrypt/6), must_be_bytes(IV, crypto_data_decrypt/6), must_be(atom, Algorithm), - option(encoding(Encoding), Options, utf8), + encoding_options(Encoding, Options), must_be(list, CipherText0), encoding_bytes(octet, CipherText0, CipherText1), append(CipherText1, Tag, CipherText), From e9f8b3591832cad22788c6507c8c81b8e5e31be3 Mon Sep 17 00:00:00 2001 From: Markus Triska Date: Mon, 18 May 2020 11:29:35 +0200 Subject: [PATCH 5/8] centralize reasoning about encoding --- src/prolog/lib/crypto.pl | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/prolog/lib/crypto.pl b/src/prolog/lib/crypto.pl index bbb80a60..12c5b73b 100644 --- a/src/prolog/lib/crypto.pl +++ b/src/prolog/lib/crypto.pl @@ -184,8 +184,7 @@ crypto_random_byte(B) :- '$crypto_random_byte'(B). crypto_data_hash(Data0, Hash, Options0) :- must_be(list, Options0), - encoding_options(Encoding, Options0), - encoding_bytes(Encoding, Data0, Data), + options_data_bytes(Options0, Data0, Data), functor_hash_options(algorithm, A, Options0, _), ( hash_algorithm(A) -> true ; domain_error(hash_algorithm, A, crypto_data_hash/3) @@ -193,9 +192,10 @@ crypto_data_hash(Data0, Hash, Options0) :- '$crypto_data_hash'(Data, HashBytes, A), hex_bytes(Hash, HashBytes). -encoding_options(Encoding, Options) :- +options_data_bytes(Options, Data, Bytes) :- option(encoding(Encoding), Options, utf8), - must_be(atom, Encoding). + must_be(atom, Encoding), + encoding_bytes(Encoding, Data, Bytes). default_hash(sha256). @@ -255,8 +255,7 @@ crypto_data_hkdf(Data0, L, Bytes, Options0) :- functor_hash_options(algorithm, Algorithm, Options0, Options), must_be(integer, L), L >= 0, - encoding_options(Encoding, Options), - encoding_bytes(Encoding, Data0, Data), + options_data_bytes(Options, Data0, Data), option(salt(SaltBytes), Options, []), must_be_bytes(SaltBytes, crypto_data_hkdf/4), option(info(Info0), Options, []), @@ -540,8 +539,7 @@ bytes_base64_([A,B,C|Ls]) --> [W,X,Y,Z], - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - */ crypto_data_encrypt(PlainText0, Algorithm, Key, IV, CipherText, Options) :- - encoding_options(Encoding, Options), - encoding_bytes(Encoding, PlainText0, PlainText), + options_data_bytes(Options, PlainText0, PlainText), option(tag(Tag), Options, _), ( nonvar(Tag) -> must_be_bytes(Tag, crypto_data_encrypt/6) From fac7ba70c87a3c69daec45bd18d078920ff0b510 Mon Sep 17 00:00:00 2001 From: Markus Triska Date: Mon, 18 May 2020 13:11:39 +0200 Subject: [PATCH 6/8] crypto_password_hash/3: fail if the number of iterations is too high Discussed in #527. --- src/prolog/machine/system_calls.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/prolog/machine/system_calls.rs b/src/prolog/machine/system_calls.rs index 4061bdbc..17d456ed 100644 --- a/src/prolog/machine/system_calls.rs +++ b/src/prolog/machine/system_calls.rs @@ -5285,7 +5285,10 @@ impl MachineState { u64::try_from(n).unwrap() } Ok(Number::Integer(n)) => { - n.to_u64().unwrap() + match n.to_u64() { + Some(i) => { i } + None => { self.fail = true; return Ok(()); } + } } _ => { unreachable!() From fd732550d8ae83cbe17e83e366175227914ea64c Mon Sep 17 00:00:00 2001 From: Markus Triska Date: Mon, 18 May 2020 13:21:20 +0200 Subject: [PATCH 7/8] crypto_data_hkdf/4: Fail if the length is too long. Due to the way the counter is constructed in the HKDF specification, the requested output length can be at most 255 times the size of the digest algorithm's output. Reported by @notoria in #527. Many thanks! --- src/prolog/machine/system_calls.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/prolog/machine/system_calls.rs b/src/prolog/machine/system_calls.rs index 17d456ed..c71fc5ba 100644 --- a/src/prolog/machine/system_calls.rs +++ b/src/prolog/machine/system_calls.rs @@ -5266,7 +5266,10 @@ impl MachineState { let salt = hkdf::Salt::new(digest_alg, &salt); let mut bytes : Vec = Vec::new(); bytes.resize(length, 0); - salt.extract(&data).expand(&[&info[..]], MyKey(length)).unwrap().fill(&mut bytes).unwrap(); + match salt.extract(&data).expand(&[&info[..]], MyKey(length)) { + Ok(r) => { r.fill(&mut bytes).unwrap(); } + _ => { self.fail = true; return Ok(()); } + } Addr::HeapCell(self.heap.to_list(bytes.iter().map(|b| HeapCellValue::Integer(Rc::new(Integer::from(*b)))))) }; From a423eb53237a01e468796ac3900e9fb4ae826811 Mon Sep 17 00:00:00 2001 From: Markus Triska Date: Mon, 18 May 2020 13:28:52 +0200 Subject: [PATCH 8/8] stronger validation of input lists for cryptographic routines Example: ?- crypto_data_hkdf(Var, 32, Bs, []). caught: error(instantiation_error,must_be/2) Reported by @notoria in #527. Many thanks! --- src/prolog/lib/crypto.pl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/prolog/lib/crypto.pl b/src/prolog/lib/crypto.pl index 12c5b73b..909127d4 100644 --- a/src/prolog/lib/crypto.pl +++ b/src/prolog/lib/crypto.pl @@ -597,12 +597,14 @@ crypto_data_decrypt(CipherText0, Algorithm, Key, IV, PlainText, Options) :- '$crypto_data_decrypt'(CipherText, Key, IV, Encoding, PlainText). encoding_bytes(octet, Bs0, Bs) :- + must_be(list, Bs0), ( maplist(integer, Bs0) -> Bs0 = Bs ; maplist(char_code, Bs0, Bs) ), must_be_bytes(Bs, crypto_encoding). encoding_bytes(utf8, Cs, Bs) :- + must_be(list, Cs), ( maplist(atom, Cs) -> chars_bytes_(Cs, Bs, crypto_encoding) ; domain_error(encryption_encoding, Cs, crypto)