Cryptographic primitives for hashing and HMAC #1712

pchiusano · 2020-10-07T15:23:59Z

This PR adds support for hashing and HMAC. See the transcript for docs and tests. Note that the newly created unison-src/new-runtime-transcripts directory has transcripts that will only be run using the new runtime and there's a tweak to the test harness to support this.

Closes #1694, but see loose ends tickets below.

Implementation notes

Followed the instructions in adding-builtins.md. Nothing too exciting. I hit a couple bugs that @dolio was kind enough to fix. I have some ideas on how to improve the handling of foreign data types in the runtime but will work on that separately.

I revamped the Bytes implementation to use a ByteArray interface provided by the memory package, to get various encoding/decoding functions (like converting to and from base 16 and base 32) and to switch to using unpinned memory. That package is already a dependency of cryptonite which we are using for crypto. See the diff in Bytes.hs for details.

See self-review.

Interesting/controversial decisions

I had wanted an incremental hashing (and HMAC) implementation but put that aside for now: you have to feed it the full Bytes or other value you are hashing. An incremental interface would look like:

Hash.new : HashAlgorithm -> Hash
Hash.add : Bytes -> Hash -> Hash
Hash.finish : Hash -> Bytes

This will let you do things like hash a large file without having to load the whole file contents in memory at once. The cryptonite library we are using supports incremental hashing, but the data type it uses to support it isn't safely serializable - it's literally just an opaque byte array internally, and the hashing algorithm (which is implemented in C) then makes assumptions based on these bytes. I was concerned that if you just deserialize any old byte array and do hashing with it, you could get buffer overruns and so on.

We could support an incremental interface later.

Test coverage

The transcript has some basic tests. The various Bytes encoding functions could use more tests.

Loose ends

Bytes encoding/decoding tests: Add tests for Bytes base16/32/64 encoding and decoding #1713
Provide implementations of universal hashing and functions: Universal hashing and HMAC implementations #1714

ex : Bytes ex = h = Sha3_512.new h2 = h `appendBytes` Bytes.fromList [1,2,3,4] Sha3_512.finish h2 > ex unison: pack bytes: non-natural closure: DataU1 589824 1 CallStack (from HasCallStack): error, called at src/Unison/Runtime/Machine.hs:1027:14 in unison-parser-typechecker-0.1-DahyA8UvBK0B5OVYJToEFT:Unison.Runtime.Machine

@dolio

…e2b{512,256}, Blake2s{256}. Still pending work from @dolio for the universal hashing functions.

…ontexts. In prep for HMAC implementation.

…ializing / decompiling the hash / hmac state is a can of worms

…ts directory for transcripts that are only run with the new runtime

…by Bytes

pchiusano

Self review.

pchiusano · 2020-10-07T16:47:56Z

parser-typechecker/src/Unison/Runtime/Builtin.hs

+-- Pure ForeignOp taking two boxed values
+pfopbb :: ForeignOp
+pfopbb instr
+  = ([BX,BX],)
+  . TAbss [b1,b2]
+  $ TFOp instr [b1,b2]
+  where
+  [b1,b2] = freshes 2
+
+pfopbbb :: ForeignOp
+pfopbbb instr
+  = ([BX,BX,BX],)
+  . TAbss [b1,b2,b3]
+  $ TFOp instr [b1,b2,b3]
+  where
+  [b1,b2,b3] = freshes 3
+
+-- Pure ForeignOp taking no values
+pfop0 :: ForeignOp
+pfop0 instr = ([],) $ TFOp instr []
+
+-- Pure ForeignOp taking 1 boxed value and returning 1 boxed value
+pfopb :: ForeignOp
+pfopb instr
+  = ([BX],)
+  . TAbss [b]
+  $ TFOp instr [b]
+  where
+  [b] = freshes 1
+
+-- Pure ForeignOp taking 1 boxed value and returning 1 Either, both sides boxed
+pfopb_ebb :: ForeignOp
+pfopb_ebb instr
+  = ([BX],)
+  . TAbss [b]
+  . TLet e UN (AFOp instr [b])
+  . TMatch e . MatchSum
+  $ mapFromList
+  [ (0, ([BX], TAbs ev $ TCon eitherReference 0 [ev]))
+  , (1, ([BX], TAbs ev $ TCon eitherReference 1 [ev]))
+  ]
+  where
+  [e,b,ev] = freshes 3


I added these common cases and am recycling them in a few places for various builtins.

pchiusano · 2020-10-07T16:51:12Z

parser-typechecker/src/Unison/Runtime/Builtin.hs

+  -- declareForeign ("crypto.hash") pfopbb . mkForeign $ \(HashAlgorithm _ref _alg, _a :: Closure) ->
+  --   pure $ Bytes.empty -- todo : implement me
+
+  declareForeign "crypto.hashBytes" pfopbb . mkForeign $


Notice am using the pfopbb : ForeignOp convention here - it's a pure foreign operation that takes 2 boxed values. Probably a lot of the foreign operations in this file could be cleaned up to use a small number of ForeignOp types.

pchiusano · 2020-10-07T16:59:13Z

parser-typechecker/src/Unison/Runtime/Foreign/Function.hs

+instance {-# overlappable #-} BuiltinForeign b => ForeignConvention b where
+  readForeign = readForeignBuiltin
+  writeForeign = writeForeignBuiltin


This saves you from needing to provide a ForeignConvention for a type that's already a BuiltinForeign.

dolio · 2020-10-08T16:29:48Z

parser-typechecker/src/Unison/Util/Bytes.hs

+  }
+
+instance B.ByteArrayAccess bytes => Eq (View bytes) where
+  v1 == v2 = viewSize v1 == viewSize v2 && unsafeDupablePerformIO (


To be honest, this (unsafeDupablePerformIO) seems like the wrong way to be doing this now. GHC has primops for comparing byte arrays that don't require dangerous stuff like this.

dolio

Things overall look all right to me.

I think it'd be better to get those unsafeDupablePerformIO calls eliminated, but it's not really different than depending on a library that does it (but shouldn't).

pchiusano · 2020-10-08T20:03:10Z

I wonder if we could just build some utilities around Vector Word8 rather than using the ByteArray interface. I'm not really sold on that interface. But I'd also like to start moving away from ByteString as I think the use of pinned memory isn't what we want.

Going to merge this but we can revisit the Bytes internals anytime.

pchiusano added 19 commits September 29, 2020 18:57

WIP on sha3 hashing

8e1cb81

Merge remote-tracking branch 'origin/trunk' into topic/hashing

87b83a3

remove redundant import

c9e45f0

Merge remote-tracking branch 'origin/trunk' into topic/hashing

2f2ec70

Something implemented and runs for Sha3{512,256}, Sha2{512,256}, Blak…

27173e5

…e2b{512,256}, Blake2s{256}. Still pending work from @dolio for the universal hashing functions.

Merge remote-tracking branch 'origin/trunk' into topic/hashing

91220df

sync with master

238dfbe

Greatly simplified hashing API and support decompilation of hashing c…

25a2e43

…ontexts. In prep for HMAC implementation.

Reworked API some more. It's looking good. Still testing left to do.

f17fafb

simplify API - not supporting streaming hashing / hmac for now as ser…

af9190e

…ializing / decompiling the hash / hmac state is a can of worms

Bytes.{to,from}Base{16,32,64,64UrlUnpadded} builtins

1e39533

Merge remote-tracking branch 'origin/trunk' into topic/hashing

1447b0a

working on transcript test and docs. Also added new-runtime-transcrip…

b2882fe

…ts directory for transcripts that are only run with the new runtime

WIP of transcripts and docs

2e5833d

Merge remote-tracking branch 'origin/trunk' into topic/basic-hashing

65acd45

Finish up transcript and tests

9ed2a46

A bit of cleanup to not hardcode the underlying bytestring type used …

9244f14

…by Bytes

fix imports

6fc235c

This was referenced Oct 7, 2020

Add tests for Bytes base16/32/64 encoding and decoding #1713

Open

Universal hashing and HMAC implementations #1714

Closed

Added comment on bytes conversion functions

9f23277

pchiusano commented Oct 7, 2020

View reviewed changes

pchiusano requested review from dolio and stew October 7, 2020 17:01

pchiusano marked this pull request as ready for review October 7, 2020 17:02

switch to using unpinned memory

ee9a547

pchiusano mentioned this pull request Oct 7, 2020

Switch to unpinned byte array type in Bytes implementation #1715

Closed

pchiusano linked an issue Oct 7, 2020 that may be closed by this pull request

Switch to unpinned byte array type in Bytes implementation #1715

Closed

dolio reviewed Oct 8, 2020

View reviewed changes

dolio approved these changes Oct 8, 2020

View reviewed changes

stew approved these changes Oct 8, 2020

View reviewed changes

pchiusano merged commit 8186cb1 into trunk Oct 8, 2020

pchiusano deleted the topic/basic-hashing branch October 8, 2020 20:03

pchiusano mentioned this pull request May 11, 2021

M2 Release Notes #1930

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cryptographic primitives for hashing and HMAC #1712

Cryptographic primitives for hashing and HMAC #1712

pchiusano commented Oct 7, 2020 •

edited

Loading

pchiusano left a comment

pchiusano Oct 7, 2020

pchiusano Oct 7, 2020

pchiusano Oct 7, 2020

dolio Oct 8, 2020

dolio left a comment

pchiusano commented Oct 8, 2020

Cryptographic primitives for hashing and HMAC #1712

Cryptographic primitives for hashing and HMAC #1712

Conversation

pchiusano commented Oct 7, 2020 • edited Loading

Implementation notes

Interesting/controversial decisions

Test coverage

Loose ends

pchiusano left a comment

Choose a reason for hiding this comment

pchiusano Oct 7, 2020

Choose a reason for hiding this comment

pchiusano Oct 7, 2020

Choose a reason for hiding this comment

pchiusano Oct 7, 2020

Choose a reason for hiding this comment

dolio Oct 8, 2020

Choose a reason for hiding this comment

dolio left a comment

Choose a reason for hiding this comment

pchiusano commented Oct 8, 2020

pchiusano commented Oct 7, 2020 •

edited

Loading