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

Fix distinguished name asn1. #140

Merged
merged 1 commit into from Apr 4, 2021

Conversation

NightBlues
Copy link
Contributor

No description provided.

@hannesm
Copy link
Member

hannesm commented Dec 17, 2020

thanks for your PR - this is a fine backwards-compatible way solving inconsistencies in respect to #69.

there are still some issues, though (repeating from NightBlues#2 (comment)): for a real CA system using ocaml-x509 it should ensure that reading the issuer used for signing is bitwise equal to the subject of the CA certificate. I guess this cannot be done in a backwards-compatible fashion, but instead requires the exposition of different string types (ia5string/utf8string/...) to the user in those parts of a DN where there's choice (and when there's no choice, we should be strict about the decoding and encoding -- and validate that with existing certificate chains / certificate corpusses: the specification/RFC may be more strict than the real world).

maybe @paurkedal has an opinion or some insights?

@paurkedal
Copy link
Contributor

paurkedal commented Dec 17, 2020

Not so much insights from me, as I haven't encountered string type ambiguity when using openssl, but I think it is better to be strict to the specification by default. Maybe it would be possible to provide a option to do lax comparison, which people would typically pass though a configuration file until they are ready to renew their CA?

I was too quick replying here, lacking some context. The approach you suggest seems reasonable.

Base automatically changed from master to main January 20, 2021 15:55
Earlier, each component was serialised to a UTF8String, which is wrong for
DomainComponent (IA5String), Serialnumber (PrintableString), CountryName
(PrintableString), DnQualifier (PrintableString), EMail (IA5String).

Reported in mirleft#69
@hannesm
Copy link
Member

hannesm commented Apr 4, 2021

Thanks. I looked into this PR again, rebased it to the main branch (and rewrote the commit message slightly). This fixes an actual issue, and makes X509 conformant with the specification, and thus is good to be merged.

@hannesm hannesm merged commit 3fd49d7 into mirleft:main Apr 4, 2021
1 check passed
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)
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

3 participants