TLS fingerprint shown as binary instead of hex

[andreasdotorg]

Currently, TLS fingerprint failures look like this:

12:54:37 * async error * TLS failure: (Error (AuthenticationFailure (InvalidFingerprint (CERTIFICATE "!i\ \n]\021\255D\151\236\017\213>\167L\204\181\196\210\247\145\186\246\212mx\147W\025\238\127\157\146"))))

It would be good if the fingerprint would be shown as hex here, for easier verification.

[cfcs]

Procedure for fixing this (for the up-and-coming jackline contributors out there):

$ fgrep -r 'TLS failure:' jackline/
bin/jackline.ml:      | Tls_lwt.Tls_failure f -> Some ("TLS failure: " ^ Tls.Engine.string_of_failure f)
$ fgrep -r 'string_of_failure' | fgrep -i 'engine'
lib/engine.mli:(** [string_of_failure failure] is [string], the string representation of the [failure]. *)
lib/engine.mli:val string_of_failure : failure -> string

Code in bin/jackline.ml:

      | Tls_lwt.Tls_failure f -> Some ("TLS failure: " ^Tls.Engine.string_of_failure f)

Look up the failure type in lib/engine.mli:

type failure = [
  | `Error of error
  | `Fatal of fatal
]

And the error type:

(** failures which can be mitigated by reconfiguration *)
type error = [
  | `AuthenticationFailure of X509.Validation.validation_error
  (* ... snippet cut ... *)

X509. let's go looking for that:

$ find ~/.opam -iname 'x509.mli'
/home/user/.opam/4.02.1/lib/x509/x509.mli

Open that file and search for type validation_error:

module Validation : sig
  (* ... snippet cut ... *)
  type validation_error = [
    (* ... snippet cut ... *)
    | `InvalidFingerprint of t
    (* ... snippet cut ... *)
  ]

So that gives us a module signature with a type ´t´. Now, we could have guessed that this was probably a Cstruct.t which is in use everywhere else in ocaml-tls, but just to be sure:

$ fgrep -r Validation ~/ocaml-x509/ | fgrep module
/home/user/ocaml-x509/lib/x509_certificate.ml:module Validation = struct (* <-- an implementation! *)
/home/user/ocaml-x509/lib/x509.mli:module Validation : sig (* signature declaration *)

Looking into x509_certificate.ml gives us:

type t = {
  asn : certificate ;
  raw : Cstruct.t
}

To display the fingerprint, we need to hash the raw field. Since we are at the frontline of a software engineering revolution, the X509 module does not currently expose the certificate, nor does it provide a way to get the fingerprint of the certificate. So we add that and submit a pull request: https://github.com/mirleft/ocaml-x509/pull/66

Now we can implement this in jackline:

      | Tls_lwt.Tls_failure f -> Some ("TLS failure: " ^
        begin match f with
        | `Error (`AuthenticationFailure (`InvalidFingerprint cert)) ->
            "AuthenticationFailure: InvalidFingerprint: " ^
            begin match
              Hex.of_cstruct (X509.fingerprint cert `SHA256)
            with `Hex s -> s end
        | _ ->
            Tls.Engine.string_of_failure f
        end )

And we observe the new error message which gives us the fingerprint of the offending certificate:

12:34:56 *** async error *** TLS failure: AuthenticationFailure: InvalidFingerprint: 12345678901234567890123456789012345678901234567890123456789012345678901234567890123456780123456

I submitted a pull request for the patch above: https://github.com/hannesm/jackline/pull/97

[sternenseemann]

@cfcs nice work.

[hannesm]

thx @cfcs -- but I decided to solve it slightly differently: having some more sensible error messages in X.509, esp when InvalidFingerprint occurs (which has been enriched with the seen and expected fingerprint as well), and actually use them in TLS when printing a TLS error... the upside is that jackline does not need any changes, and other applications which use Tls.Engine.string_of_failure benefit from this as well :)

[cfcs]

No objections to that :-) I guess that means this issue is resolved!

[hannesm]

subsequently fixed in other libraries, it will only be a matter of time (and releases) till this is fixed in jackline itself (for the adventurous use fc21d5989fc5dba47bb71d9aa2b59195729a1176 and tls + x509 master + prs)

thanks for reporting @andreas23, and for the extensive report how to fix such issues, @cfcs