provide ciphers with {de,en}crypt_into functionality #231

hannesm · 2024-06-07T09:58:09Z

This avoids buffer allocations. There are as well unsafe functions exposed for elevated performance.

Reviews welcome, this is WIP since for now only ECB and ECB modes have this new interface (still missing CTR, GCM, CCM). Will continue work on that. Will superseed #204 (at a future point in time).

src/cipher_block.ml

palainp · 2024-06-07T11:33:08Z

Thank you for that work, I appreciate these (unsafe functions)! I don't understand the CI failures here, and the code seems good to me :)

src/mirage_crypto.mli

hannesm · 2024-06-09T14:22:59Z

I pushed the rest, numbers are (all MB/s), main is dc08c71.

Again, these are on my laptop (a pretty old i7-5600U) with OCaml 4.14.2.

[chacha20-poly1305]

bs	main	this PR	unsafe
16	28.68	28.94	29.17
64	107.21	110.50	112.71
256	210.71	219.14	224.66
1024	288.92	297.65	299.14
8192	301.33	331.52	221.89

[aes-128-ecb]

bs	main	this PR	unsafe
16	334.13	366.06	549.37
64	940.04	1030.37	1235.77
256	2890.97	3002.71	3450.04
1024	4114.06	4107.82	4397.36
8192	2779.25	4683.25	4718.95

[aes-192-ecb]

bs	main	this PR	unsafe
16	272.28	328.13	474.28
64	746.33	884.63	1049.79
256	2177.49	2613.92	2999.34
1024	3401.65	3542.78	3605.06
8192	2912.44	3824.85	3918.67

[aes-256-ecb]

bs	main	this PR	unsafe
16	270.12	294.16	419.64
64	733.58	812.57	949.72
256	2304.86	2347.25	2719.95
1024	3016.61	3068.76	3239.69
8192	2301.45	3281.38	3360.81

[aes-128-cbc-e]

bs	main	this PR	unsafe
16	260.19	267.08	377.52
64	431.68	439.84	502.51
256	513.78	523.94	539.20
1024	537.34	542.21	556.79
8192	517.00	550.46	543.76

[aes-128-cbc-d]

bs	main	this PR	unsafe
16	268.24	258.82	391.45
64	759.02	808.07	1013.12
256	2110.42	2351.67	2626.71
1024	2788.09	3346.07	3492.57
8192	1769.80	3647.55	3685.81

[aes-128-ctr]

bs	main	this PR	unsafe
16	216.63	229.51	246.38
64	508.88	684.87	721.28
256	1729.31	1886.90	1994.78
1024	2267.69	2575.06	2698.66
8192	1747.91	3006.44	3081.07

[aes-128-gcm]

bs	main	this PR	unsafe
16	58.31	68.67	72.43
64	173.41	214.31	234.06
256	616.69	755.22	810.40
1024	1148.37	1497.08	1582.37
8192	1050.54	2168.48	2083.73

[aes-128-ghash]

bs	main	this PR	unsafe
16	61.65	71.84	80.57
64	209.28	239.58	264.93
256	826.37	951.58	1072.18
1024	2338.49	2641.48	2959.12
8192	4963.99	5253.94	5580.50

[aes-128-ccm]

bs	main	this PR	unsafe
16	46.57	55.75	60.79
64	94.37	119.26	126.62
256	129.75	163.78	172.17
1024	144.46	181.62	188.59
8192	147.02	185.53	195.41

[d3des-ecb]

bs	main	this PR	unsafe
16	20.39	20.09	20.98
64	20.89	20.93	21.28
256	21.48	20.89	21.54
1024	21.33	21.13	21.49
8192	21.36	21.05	21.58

hannesm · 2024-06-09T14:27:45Z

Review and feedback is welcome, given the numbers I plan to include the unsafe_ functions, but don't have them show up in the generated documentation (using the (**/**) notation).

Any opinions on the API are welcome.

hannesm · 2024-06-09T14:53:18Z

I'm pretty sure there's more that can be done for fewer allocations, esp. in the Poly1305 (chacha20/poly1305) and Ghash (GCM) code - as well as in CCM. But I'd defer further optimizations to another PR (unless there's something obvious we can do right now, or we have some performance pressure for a specific use case).

Also provide unsafe_{en,de}crypt_into for further performance.

This may avoid buffer allocations. There are as well unsafe functions for those feeling bounds checks are unnecessary.

reynir

I found there are insufficient checks especially if the offsets are valid (non-negative).

src/cipher_block.ml

src/ccm.ml

src/cipher_block.ml

src/mirage_crypto.mli

src/poly1305.ml

Co-authored-by: Reynir Björnsson <reynir@reynir.dk>

dinosaure · 2024-06-27T10:32:04Z

Good to merge for me from a quick review.

@hannesm

CHANGES: ### Breaking changes * mirage-crypto: Poly1305 API now uses string (mirage/mirage-crypto#203 @hannesm) * mirage-crypto: Poly1305 no longer has type alias "type mac = string" (mirage/mirage-crypto#232 @hannesm) * mirage-crypto: the API uses string instead of cstruct (mirage/mirage-crypto#214 @reynir @hannesm) * mirage-crypto: Hash module has been removed. Use digestif if you need hash functions (mirage/mirage-crypto#213 @hannesm) * mirage-crypto: the Cipher_block and Cipher_stream modules have been removed, its contents is inlined: Mirage_crypto.Cipher_block.S -> Mirage_crypto.Block Mirage_crypto.Cipher_stream.S -> Mirage_crypto.Stream Mirage_crypto.Cipher_block.AES.CTR -> Mirage_crypto.AES.CTR (mirage/mirage-crypto#225 @hannesm, suggested in mirage/mirage-crypto#224 by @reynir) * mirage-crypto-pk: s-expression conversions for private and public keys (Dh, Dsa, Rsa) have been removed. You can use PKCS8 for encoding and decoding `X509.{Private,Public}_key.{en,de}code_{der,pem}` (mirage/mirage-crypto#208 @hannesm) * mirage-crypto-pk: in the API, Cstruct.t is no longer present. Instead, string is used (mirage/mirage-crypto#211 @reynir @hannesm) * mirage-crypto-rng: the API uses string instead of Cstruct.t. A new function `generate_into : ?g -> bytes -> ?off:int -> int -> unit` is provided (mirage/mirage-crypto#212 @hannesm @reynir) * mirage-crypto-ec: remove NIST P224 support (mirage/mirage-crypto#209 @hannesm @Firobe) * mirage-crypto: in Uncommon.xor_into the arguments ~src_off and ~dst_off are required now (mirage/mirage-crypto#232 @hannesm), renamed to unsafe_xor_into (98f01b14f5ebf98ba0e7e9c2ba97ec518f90fddc) * mirage-crypto-pk, mirage-crypto-rng: remove type alias "type bits = int" (mirage/mirage-crypto#236 @hannesm) ### Bugfixes * mirage-crypto (32 bit systems): CCM with long adata (mirage/mirage-crypto#207 @reynir) * mirage-crypto-ec: fix K_gen for bitlen mod 8 != 0 (reported in mirage/mirage-crypto#105 that P521 test vectors don't pass, re-reported mirage/mirage-crypto#228, fixed mirage/mirage-crypto#230 @Firobe) * mirage-crypto-ec: zero out bytes allocated for Field_element.zero (reported mirleft/ocaml-x509#167, fixed mirage/mirage-crypto#226 @dinosaure) ### Data race free * mirage-crypto (3DES): avoid global state in key derivation (mirage/mirage-crypto#223 @hannesm) * mirage-crypto-rng: use atomic instead of reference to be domain-safe (mirage/mirage-crypto#221 @dinosaure @reynir @hannesm) * mirage-crypto, mirage-crypto-rng, mirage-crypto-pk, mirage-crypto-ec: avoid global buffers, use freshly allocated strings/bytes instead, avoids data races (mirage/mirage-crypto#186 mirage/mirage-crypto#219 @dinosaure @reynir @hannesm) ### Other changes * mirage-crypto: add {de,en}crypt_into functions (and unsafe variants) to allow less buffer allocations (mirage/mirage-crypto#231 @hannesm) * mirage-crypto-rng-miou: new package which adds rng support with miou (mirage/mirage-crypto#227 @dinosaure) * PERFORMANCE mirage-crypto: ChaCha20/Poly1305 use string instead of Cstruct.t, ChaCha20 interface unchanged, performance improvement roughly 2x (mirage/mirage-crypto#203 @hannesm @reynir) * mirage-crypto-ec, mirage-crypto-pk, mirage-crypto-rng: use digestif for hashes (mirage/mirage-crypto#212 mirage/mirage-crypto#215 @reynir @hannesm) * mirage-crypto-rng: use a set for entropy sources instead of a list (mirage/mirage-crypto#218 @hannesm) * mirage-crypto-rng-mirage: provide a module type S (for use instead of mirage-random in mirage) (mirage/mirage-crypto#234 @hannesm)

@hannesm

CHANGES: ### Breaking changes * mirage-crypto: Poly1305 API now uses string (mirage/mirage-crypto#203 @hannesm) * mirage-crypto: Poly1305 no longer has type alias "type mac = string" (mirage/mirage-crypto#232 @hannesm) * mirage-crypto: the API uses string instead of cstruct (mirage/mirage-crypto#214 @reynir @hannesm) * mirage-crypto: Hash module has been removed. Use digestif if you need hash functions (mirage/mirage-crypto#213 @hannesm) * mirage-crypto: the Cipher_block and Cipher_stream modules have been removed, its contents is inlined: Mirage_crypto.Cipher_block.S -> Mirage_crypto.Block Mirage_crypto.Cipher_stream.S -> Mirage_crypto.Stream Mirage_crypto.Cipher_block.AES.CTR -> Mirage_crypto.AES.CTR (mirage/mirage-crypto#225 @hannesm, suggested in mirage/mirage-crypto#224 by @reynir) * mirage-crypto-pk: s-expression conversions for private and public keys (Dh, Dsa, Rsa) have been removed. You can use PKCS8 for encoding and decoding `X509.{Private,Public}_key.{en,de}code_{der,pem}` (mirage/mirage-crypto#208 @hannesm) * mirage-crypto-pk: in the API, Cstruct.t is no longer present. Instead, string is used (mirage/mirage-crypto#211 @reynir @hannesm) * mirage-crypto-rng: the API uses string instead of Cstruct.t. A new function `generate_into : ?g -> bytes -> ?off:int -> int -> unit` is provided (mirage/mirage-crypto#212 @hannesm @reynir) * mirage-crypto-ec: remove NIST P224 support (mirage/mirage-crypto#209 @hannesm @Firobe) * mirage-crypto: in Uncommon.xor_into the arguments ~src_off and ~dst_off are required now (mirage/mirage-crypto#232 @hannesm), renamed to unsafe_xor_into (98f01b14f5ebf98ba0e7e9c2ba97ec518f90fddc) * mirage-crypto-pk, mirage-crypto-rng: remove type alias "type bits = int" (mirage/mirage-crypto#236 @hannesm) ### Bugfixes * mirage-crypto (32 bit systems): CCM with long adata (mirage/mirage-crypto#207 @reynir) * mirage-crypto-ec: fix K_gen for bitlen mod 8 != 0 (reported in mirage/mirage-crypto#105 that P521 test vectors don't pass, re-reported mirage/mirage-crypto#228, fixed mirage/mirage-crypto#230 @Firobe) * mirage-crypto-ec: zero out bytes allocated for Field_element.zero (reported mirleft/ocaml-x509#167, fixed mirage/mirage-crypto#226 @dinosaure) ### Data race free * mirage-crypto (3DES): avoid global state in key derivation (mirage/mirage-crypto#223 @hannesm) * mirage-crypto-rng: use atomic instead of reference to be domain-safe (mirage/mirage-crypto#221 @dinosaure @reynir @hannesm) * mirage-crypto, mirage-crypto-rng, mirage-crypto-pk, mirage-crypto-ec: avoid global buffers, use freshly allocated strings/bytes instead, avoids data races (mirage/mirage-crypto#186 mirage/mirage-crypto#219 @dinosaure @reynir @hannesm) ### Other changes * mirage-crypto: add {de,en}crypt_into functions (and unsafe variants) to allow less buffer allocations (mirage/mirage-crypto#231 @hannesm) * mirage-crypto-rng-miou: new package which adds rng support with miou (mirage/mirage-crypto#227 @dinosaure) * PERFORMANCE mirage-crypto: ChaCha20/Poly1305 use string instead of Cstruct.t, ChaCha20 interface unchanged, performance improvement roughly 2x (mirage/mirage-crypto#203 @hannesm @reynir) * mirage-crypto-ec, mirage-crypto-pk, mirage-crypto-rng: use digestif for hashes (mirage/mirage-crypto#212 mirage/mirage-crypto#215 @reynir @hannesm) * mirage-crypto-rng: use a set for entropy sources instead of a list (mirage/mirage-crypto#218 @hannesm) * mirage-crypto-rng-mirage: provide a module type S (for use instead of mirage-random in mirage) (mirage/mirage-crypto#234 @hannesm)

This was referenced Jun 7, 2024

WIP aead into, WIP #204

Closed

Roadmap for 1.0 release #205

Closed

palainp reviewed Jun 7, 2024

View reviewed changes

src/cipher_block.ml Outdated Show resolved Hide resolved

src/cipher_block.ml Show resolved Hide resolved

palainp reviewed Jun 7, 2024

View reviewed changes

src/mirage_crypto.mli Show resolved Hide resolved

hannesm mentioned this pull request Jun 9, 2024

minor updates #232

Merged

hannesm force-pushed the into branch from 217957b to 67e1280 Compare June 9, 2024 14:19

hannesm changed the title ~~(WIP) provide ciphers with {de,en}crypt_into functionality~~ provide ciphers with {de,en}crypt_into functionality Jun 9, 2024

hannesm added 7 commits June 11, 2024 10:34

Mirage_crypto.Block.ECB with {de,en}crypt_into

04c1315

Also provide unsafe_{en,de}crypt_into for further performance.

Mirage_crypto.Block.CBC now has {de,en}crypt_into functionality

e8614b0

This may avoid buffer allocations. There are as well unsafe functions for those feeling bounds checks are unnecessary.

counters: add an offset parameter

5cbad0a

Mirage_crypto.Block.CTR with {de,en}crypt_into

148b4d4

GCM and ChaCha have {de,en}crypt_into now

0316c43

CCM16 with {de,en}crypt_into

5243e87

minor adjustments to speed

4482eeb

hannesm force-pushed the into branch from 67e1280 to 4482eeb Compare June 11, 2024 08:35

reynir self-requested a review June 11, 2024 09:00

reynir reviewed Jun 18, 2024

View reviewed changes

hannesm and others added 7 commits June 18, 2024 14:13

Apply suggestions from code review

3399544

Co-authored-by: Reynir Björnsson <reynir@reynir.dk>

revise bounds checks (cc @reynir @palainp), also check off >= 0

08a8b16

revise block_size check

acf74f4

update documentation, esp off < 0

829ceb5

poly1305: mac_into appropriate bounds checks, also unsafe_mac_into

7805a7c

ccm: remove maclen argument, and ensure tag_size = block_size

332890e

add tailcall annotations, remove an argument from ccm's loop

52e5105

reynir approved these changes Jun 28, 2024

View reviewed changes

hannesm merged commit ba299d8 into mirage:main Jun 29, 2024
9 of 13 checks passed

hannesm deleted the into branch June 29, 2024 05:00

hannesm mentioned this pull request Jun 29, 2024

[new release] mirage-crypto (9 packages) (1.0.0) ocaml/opam-repository#26163

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provide ciphers with {de,en}crypt_into functionality #231

provide ciphers with {de,en}crypt_into functionality #231

hannesm commented Jun 7, 2024

palainp commented Jun 7, 2024 •

edited

Loading

hannesm commented Jun 9, 2024 •

edited

Loading

hannesm commented Jun 9, 2024 •

edited

Loading

hannesm commented Jun 9, 2024

reynir left a comment

dinosaure commented Jun 27, 2024

provide ciphers with {de,en}crypt_into functionality #231

provide ciphers with {de,en}crypt_into functionality #231

Conversation

hannesm commented Jun 7, 2024

palainp commented Jun 7, 2024 • edited Loading

hannesm commented Jun 9, 2024 • edited Loading

[chacha20-poly1305]

[aes-128-ecb]

[aes-192-ecb]

[aes-256-ecb]

[aes-128-cbc-e]

[aes-128-cbc-d]

[aes-128-ctr]

[aes-128-gcm]

[aes-128-ghash]

[aes-128-ccm]

[d3des-ecb]

hannesm commented Jun 9, 2024 • edited Loading

hannesm commented Jun 9, 2024

reynir left a comment

Choose a reason for hiding this comment

dinosaure commented Jun 27, 2024

palainp commented Jun 7, 2024 •

edited

Loading

hannesm commented Jun 9, 2024 •

edited

Loading

hannesm commented Jun 9, 2024 •

edited

Loading