Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

mirage-crypto-pk: remove s-expression converters and sexplib0 dependency #208

Merged
merged 1 commit into from
Feb 28, 2024
Merged

mirage-crypto-pk: remove s-expression converters and sexplib0 dependency #208

merged 1 commit into from
Feb 28, 2024

Conversation

hannesm
Copy link
Member

@hannesm hannesm commented Feb 28, 2024

No description provided.

@hannesm hannesm mentioned this pull request Feb 28, 2024
Closed
23 tasks
@hannesm
Copy link
Member Author

hannesm commented Feb 28, 2024

I used sherlocode to search for users of this functionality and didn't find any.

@reynir
Copy link
Member

reynir commented Feb 28, 2024

I would be in favor of removing this. However, I found a few users of the sexp converters by searching GitHub. Most notable are jackline and opam-health-check:

https://github.com/hannesm/jackline/blob/a7acd19bd8141b842ac69b05146d9a63e729230d/src/persistency.ml#L203

https://github.com/OnofreTZK/Blockchain4Me/blob/9d0ee58f51725b29bbbf8e65e977427ba7b34d2c/DSAExample/cr.ml#L12

https://github.com/ocurrent/opam-health-check/blob/6aba82fd0cddaddfc966ace83940007284059ac7/server/backend/admin.ml#L21

Those two use the sexp serializers to store keys. Probably for those two cases deserializers could be implemented manually.

@hannesm
Copy link
Member Author

hannesm commented Feb 28, 2024 •
edited
Loading

@reynir indeed, well spotted. I guess some serialisation format is sensible -- but then for private keys PKCS8 (via X509.Private_key.encode_{der,pem}) is best used -- it is as well interoperable (for DSA keys that is not (yet?) implemented).

For jackline, I would just embed the priv_of_sexp in that codebase and use (for newly generated OTR keys) PKCS8. In opam-health-check I guess it makes sense to open an issue about it. The Blockchain4Me doesn't look active, and based on the name I won't put much time into it.

This was referenced Feb 28, 2024
Merged
Merged
Merged
@hannesm
Copy link
Member Author

hannesm commented Feb 28, 2024

Incentive: decrease of binary size of bench/speed.exe (not stripped, build in "dune release mode"). main branch (9cb2ebd) 7718680 bytes, this branch 7193120 bytes (around 500kB).

@hannesm hannesm merged commit ec3e5ba into mirage:main Feb 28, 2024
29 checks passed
@hannesm hannesm deleted the pk-no-sexp branch February 28, 2024 20:25
hannesm added a commit to hannesm/opam-repository that referenced this pull request Jun 29, 2024
@hannesm
[new release] mirage-crypto (9 packages) (1.0.0)
2a90a5b 
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 hannesm mentioned this pull request Jun 29, 2024
Merged
avsm pushed a commit to avsm/opam-repository that referenced this pull request Sep 5, 2024
@hannesm @avsm
[new release] mirage-crypto (9 packages) (1.0.0)
1ee0136 
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hannesm @reynir