My first elm package: elm-hmac-sha1

Hello everyone. I’m following the guidelines and I wanted to ask you what do you think about this before publishing it.

Here is the repo: https://github.com/romariolopezc/elm-hmac-sha1
And here is the documentation: https://package.elm-lang.org/packages/romariolopezc/elm-hmac-sha1/latest/

  • What is the concrete problem you want to solve?
    HMAC-SHA1 computation

  • What would it mean for your API to be a success?
    Other people using it

  • Who has this problem? What do they want from an API?

  • What specific things are needed to solve that problem?
    Use SHA1 as the hash function for the HMAC

  • Have other people worked on this problem? What lessons can be learned from them? Are there specific weaknesses you want to avoid?

Thanks to ktonon, I reused the HMAC logic from it’s package (elm-crypto). I first asked if he was going to add sha-1 to the package (https://github.com/ktonon/elm-crypto/issues/3), and he pointed me to a sha-1 implementation, so I just connected both.

5 Likes

Three quick thoughts:

  1. Maybe add a more descriptive README as this is usually the first page that users see:
    • What is HMAC-SHA1 (with a link)?
    • When is it useful or usually used, some examples?
    • A small code example
    • What is the difference between SHA1 and HMAC-SHA1 as this may be confusing?
  2. Will people need to be able to run it on Bytes? Also maybe returning the digest as Bytes? I’m not sure, just asking. Also your toBytes function name may be ambiguous because of that.
  3. In toBytes example, shouldn’t it be a ==? Maybe use elm-verify-examples for consistency?
5 Likes

Thanks for the feedback dmy. I improved the package.

Update:

  • Using an opaque type with elm/bytes representation
  • Added README
  • Improved documentation
  • Using elm-verify-examples and elm-test
  • Everything returns a Result for consistency
3 Likes

I published the Package: https://package.elm-lang.org/packages/romariolopezc/elm-hmac-sha1/latest/ :smiley:

1 Like

I think the API is very nice! I can offer some feedback here specifically on the feature this package provides:

  • Packages that implement cryptographic algorithms should denote in the README whether they have been peer reviewed. ktonon/elm-crypto’s README includes such a section at the end.
  • SHA-1 is not considered cryptographically strong, and the strength of an HMAC depends in part on the strength of the hash. So the README should denote this as well to ensure people only use it for things like interoperating with systems that still use HMAC-SHA1 and not for implementing new integrity and authenticity checking in their own software.
9 Likes

Thanks for the feedback Luke! I updated the README with more information.

I only have doubts about the versioning. Why if I change just the README (no code), elm needs to bump the version?

Changing README: 1.0.1 -> 1.0.2

Thank you! Published Elm packages are linked to a specific git commit. The package website loads everything, including the README, from the tree at the commit for a particular version.

Hi,

Thanks for this nice package!

I have some feedback. If I understand it correctly, your encoder listToBytes and decoder bytesToMaybeList are almost inverses of each other. If you were to store the digest in type Digest = Digest (List Int), your toBytes and toIntList would have clearer API as you could return the representation directly without the Result wrapping.

1 Like

Ah yes that is clearer, and it only converts the list to bytes in that toBytes function instead of each of the representation to* functions.

I have doubts on the Result wrapping. Is clear that I don’t need to wrap it but I wanted to standardize the functions return Types, all returning Result String Representation, what do you think? @tzemanovic

I saw the PR thanks for that!

1 Like

I think baseX encoding (but not decoding) should be possible to implement in a way that it can never fail. I.e. it should be able to encode any arbitrary byte sequence.

It seems to me that in the upstream library elm-coder the encoding can fail because the implementation relies on List.Extra.getAt, which can fail when you try to access invalid index. If that’s possible to improve then all your toX can also avoid the Result wrapper.

Personally, even if that’s not technically correct or possible, I think in this case it would be preferable to have clearer API on those two functions rather than making them all consistent. They would most likely be consumed in different ways, so it doesn’t give you much benefit to have standard looking outputs (one is List Int, the other is Bytes and the other two are String)

2 Likes

This topic was automatically closed 10 days after the last reply. New replies are no longer allowed.