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.

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

Do not allow sloppy keys. #142

Merged
merged 2 commits into from Jan 21, 2021
Merged

Do not allow sloppy keys. #142

merged 2 commits into from Jan 21, 2021

Conversation

psafont
Copy link
Contributor

@psafont psafont commented Jan 20, 2021

They were not sloppy after all, and should be accepted at mirage-crypto
level.

Depends on mirage/mirage-crypto#99

Copy link
Member

@hannesm hannesm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, I looked at commit f4dd7dc which this PR basically reverts (but keeping the test case).

lib/private_key.ml Show resolved Hide resolved
lib/private_key.ml Outdated Show resolved Hide resolved
lib/private_key.ml Outdated Show resolved Hide resolved
lib/private_key.ml Outdated Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
@psafont psafont force-pushed the noslop branch 3 times, most recently from bed82d6 to ebf3a8d Compare January 20, 2021 11:37
lib/private_key.ml Outdated Show resolved Hide resolved
@psafont psafont force-pushed the noslop branch 2 times, most recently from fb0bd23 to 9505e2f Compare January 21, 2021 14:05
psafont and others added 2 commits January 21, 2021 18:56
They were not sloppy after all, and are accepted by mirage-crypto since 0.8.10

It reverts most of the changes in f4dd7dc while
keeping the regression test and adding a new one.

Signed-off-by: Pau Ruiz Safont <pau.safont@citrix.com>
@hannesm
Copy link
Member

hannesm commented Jan 21, 2021

thanks, I force-pushed to trigger CI -- which now looks fine.

@hannesm hannesm merged commit dee157e into mirleft:main Jan 21, 2021
@psafont psafont deleted the noslop branch January 22, 2021 09:09
avsm pushed a commit to ocaml/opam-repository that referenced this pull request Apr 6, 2021
CHANGES:

* FEATURE PKCS12 support (mirleft/ocaml-x509#114 by @hannesm)
* FEATURE ECDSA and EDDSA support via mirage-crypto-ec (mirleft/ocaml-x509#145 by @hannesm)
  This breaks some clients since the Private_key.t and Public_key.t variants
  are extended (may result in partial pattern matches of users of this library).
* CRL.is_revoked has `crls` as last parameter to avoid warning 16
  (4.12 compatibility) (mirleft/ocaml-x509#144 by @hannesm)
* Signing_request.sign: add optional labelled argument `~subject` to allow
  changing the subject when signing a signing request (mirleft/ocaml-x509#139 by @reynir)
* BUGFIX Encoding of Distinguished_name components (adhere to specification)
  DomainComponent and EMail are now serialised using a IA5String; Serialnumber,
  CountryName and DnQualifier as PrintableString (reported in mirleft/ocaml-x509#69, fixed mirleft/ocaml-x509#140
  by @NightBlues)
* BREAKING Remove `~sloppy` from Private_key.decode_{pem,der}. The seemingly
  bad RSA keys were valid and should have been accepted by mirage-crypto.
  (mirleft/ocaml-x509#142 by @psafont)
actionshrimp added a commit to imandra-ai/ocaml-gcloud that referenced this pull request Sep 17, 2021
From
ocaml/opam-repository@cf51d08 :
* BREAKING Remove `~sloppy` from Private_key.decode_{pem,der}. The seemingly
  bad RSA keys were valid and should have been accepted by mirage-crypto.
  (mirleft/ocaml-x509#142 by @psafont)
actionshrimp added a commit to imandra-ai/ocaml-gcloud that referenced this pull request Sep 20, 2021
From
ocaml/opam-repository@cf51d08 :
* BREAKING Remove `~sloppy` from Private_key.decode_{pem,der}. The seemingly
  bad RSA keys were valid and should have been accepted by mirage-crypto.
  (mirleft/ocaml-x509#142 by @psafont)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants