Skip to content

put CERTIFICATE_UNKNOWN back #516

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

Merged
merged 1 commit into from
Aug 22, 2025
Merged

put CERTIFICATE_UNKNOWN back #516

merged 1 commit into from
Aug 22, 2025

Conversation

copy
Copy link
Contributor

@copy copy commented Jun 18, 2025

This alert is sent by chromium on self-signed certificates, so it probably makes sense to have a useful error message, as opposed to "TLS alert from peer: unknown 46"

This alert is sent by chromium on self-signed certificates, so it
probably makes sense to have a useful error message, as opposed to
"TLS alert from peer: unknown 46"
@hannesm
Copy link
Member

hannesm commented Jun 19, 2025

I'm undecided. I agree that this is a more useful error message. At the same time, my reasoning in 52ee03e was to reduce the binary size of the tls library - and only provide these constructors that we actually use.

Now, where should we draw the line? There are likely more alerts being used by other TLS implementations. Adding all the alerts (e.g. all from the TLS 1.3 spec) would mean to add:

          unsupported_certificate(43),
          certificate_revoked(44),
          certificate_expired(45),
          illegal_parameter(47),
          unknown_ca(48),
          access_denied(49),
          decrypt_error(51),
          insufficient_security(71),
          internal_error(80),
          bad_certificate_status_response(113),
          unknown_psk_identity(115),
          certificate_required(116),

and not only these constructors and integer values, but as well something printable (a string).

As said, I'm undecided about that. We can of course do this case-by-case -- and now add the certificate_unknown, and when someone else comes along, add their favourite observed alert.

@copy
Copy link
Contributor Author

copy commented Jun 19, 2025

As said, I'm undecided about that. We can of course do this case-by-case -- and now add the certificate_unknown, and when someone else comes along, add their favourite observed alert.

That sounds reasonable to me.

@hannesm hannesm merged commit 3a0dc34 into mirleft:main Aug 22, 2025
1 check passed
@hannesm
Copy link
Member

hannesm commented Aug 22, 2025

thanks. sorry for the delay. I cut a release, 2.0.2, including your PR.

hannesm added a commit to hannesm/opam-repository that referenced this pull request Aug 22, 2025
CHANGES:

* put CERTIFICATE_UNKNOWN alert back (mirleft/ocaml-tls#516 @copy)
* improve README (mirleft/ocaml-tls#514 @dinosaure)
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.

2 participants