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

API usage and confusion #156

Open
hannesm opened this issue May 8, 2024 · 3 comments
Open

API usage and confusion #156

hannesm opened this issue May 8, 2024 · 3 comments

Comments

@hannesm
Copy link
Member

hannesm commented May 8, 2024

Hey,

I'm not entirely sure I understand the API design. We have two types, ctx and t -- but what is the fundamental difference?

A ctx we can create by either empty or init -- hold on, should the ctx returned by empty be immutable or better not being used?

A t we receive from get, but also from the functions that take care of computing the hash of a string / string list /...

Now, the nicely introduced get_into_bytes is defined on ctx, so if I computed the hash with digest_string, I can't extract it into an existing byte vector.

Would it be possible to simplify the API and only provide a single type t? Since it is private anyways, I'd expect no issues... What do you think?

@hannesm
Copy link
Member Author

hannesm commented May 8, 2024

I got confused when thinking about this code from mirage-crypto's hmac_drbg RNG:

    let rec go off k v = function
      | 0 -> v
      | 1 ->
        let v = H.hmac_string ~key:k v |> H.to_raw_string in
        let len =
          let rem = len mod H.digest_size in
          if rem = 0 then H.digest_size else rem
        in
        Bytes.unsafe_blit_string v 0 buf off len;
        v
      | i ->
        let v = H.hmac_string ~key:k v |> H.to_raw_string in
        Bytes.unsafe_blit_string v 0 buf off H.digest_size;
        go (off + H.digest_size) k v (pred i)

To avoid further allocations, also a ?len parameter for get_into_bytes would be great.

@dinosaure
Copy link
Member

#155 fixes your issue?

@hannesm
Copy link
Member Author

hannesm commented Jan 8, 2025

I think I'm still curious whether/why we need both ctx and t as types. I understand that this would introduce a breaking change.

As example, in the Poly1305 in mirage-crypto, we have (an incomplete interface in contrast to digestif) -- which avoids the type ctx:

module type S = sig
  type 'a iter = 'a Uncommon.iter

  type t
  val mac_size : int

  val empty : key:string -> t
  val feed : t -> string -> t
  val feedi : t -> string iter -> t
  val get : t -> string

  val mac : key:string -> string -> string
  val maci : key:string -> string iter -> string
  val mac_into : key:string -> (string * int * int) list -> bytes -> dst_off:int -> unit
  val unsafe_mac_into : key:string -> (string * int * int) list -> bytes -> dst_off:int -> unit
end

The second comment is about get_into_bytes -- where in some cases we want to truncate the output (and not take the entire output) -- and here the ?len argument would be helpful -- esp. for the presented DRBG implementation. I guess that's a separate issue, with:

  val get_into_bytes : ctx -> ?off:int -> bytes -> unit
  (** [get_into_bytes ctx ?off buf] writes the result into the given [buf] at
      [off] (defaults to [0]).

      It's equivalent to:

      {[
        let get_into_bytes ctx ?(off = 0) buf =
          let t = get ctx in
          let str = to_raw_string t in
          Bytes.blit_string str 0 buf off digest_size
      ]}

      except [get_into_bytes] does not allocate an intermediate string. *)

Where the "digest_size" provided to Bytes.blit_string would then be configurable. I'm not certain this complexity in Digestif pays off, though.

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

No branches or pull requests

2 participants