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

Mirage_git_http: When failing to parse authenticator report what the expected format is #582

Closed
reynir opened this issue Aug 25, 2022 · 0 comments · Fixed by #593
Closed

Comments

@reynir
Copy link
Member

reynir commented Aug 25, 2022

When writing a unikernel that uses git over https you may want to expose a https/tls-authenticator argument so the user can pin a key or certificate. The description of the format is somewhat long and not so nice to put in the argument doc string. However, leaving out the description offers a poor user experience because the user will just get an error message saying the authenticator is invalid if they try to use e.g. a key fingerprint without the prerequisite prefix and they will be none the wiser what the expected format is.

I suggest the error message explains what the expected format is.

let with_optional_tls_config_and_headers ?headers ?authenticator ctx =
let time () = Some (Ptime.v (Pclock.now_d_ps ())) in
let none ?ip:_ ~host:_ _ = Ok None in
let authenticator =
match authenticator with
| None -> (
match NSS.authenticator () with
| Ok authenticator -> authenticator
| Error (`Msg err) -> failwith err)
| Some str -> (
match String.split_on_char ':' str with
| "key" :: tls_key_fingerprint ->
let tls_key_fingerprint = String.concat ":" tls_key_fingerprint in
let hash, fingerprint = of_fp tls_key_fingerprint in
X509.Authenticator.server_key_fingerprint ~time ~hash ~fingerprint
| "cert" :: tls_cert_fingerprint ->
let tls_cert_fingerprint =
String.concat ":" tls_cert_fingerprint
in
let hash, fingerprint = of_fp tls_cert_fingerprint in
X509.Authenticator.server_cert_fingerprint ~time ~hash
~fingerprint
| "trust-anchor" :: certs ->
let certs = List.map Base64.decode certs in
let certs = List.map (Result.map Cstruct.of_string) certs in
let certs =
List.map
(fun cert -> Result.bind cert X509.Certificate.decode_der)
certs
in
let certs = List.filter_map Result.to_option certs in
X509.Authenticator.chain_of_trust ~time certs
| [ "none" ] -> none
| _ -> Fmt.failwith "Invalid TLS authenticator: %S" str)

hannesm added a commit to hannesm/opam-repository that referenced this issue Oct 5, 2022
CHANGES:

* Improve parse error message of Authenticator.of_string (mirage/ocaml-git#593
  by @dinosaure, mirage/ocaml-git#582 by @reynir)
dinosaure added a commit to dinosaure/opam-repository that referenced this issue Oct 20, 2022
CHANGES:

- Do not append a leading slash to path (mirage/ocaml-git#580, @reynir, @dinosaure)
- Fix some typos (mirage/ocaml-git#585, @hannesm, @dinosaure)
- Add the `main` reference (mirage/ocaml-git#586, @hannesm, @dinosaure)
- Upgrade `git-paf` with `paf.0.2.0` (mirage/ocaml-git#587, @dinosaure)
- Explain when the user give a bad argument about TLS authenticator (mirage/ocaml-git#593, reported by @reynir mirage/ocaml-git#582, @dinosaure)
- Use `x509.0.16.2` which raise an explanation if it's a bad argument (mirage/ocaml-git#594, @hannesm, @dinosaure)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant