abbot / go-http-auth

Basic and Digest HTTP Authentication for golang http
Apache License 2.0
544 stars 121 forks source link

Implement SHA-256 and SHA-512 hashed passwords #75

Open pitpalme opened 3 years ago

pitpalme commented 3 years ago

Fix #72

abbot commented 3 years ago

This reimplements sha256/sha512 algorithm, which is available in the standard library (crypto/sha256 and crypto/sha512 respectively). I'm not sure I understand why this path was chosen instead of using the standard implementations?

pitpalme commented 3 years ago

Actually it's usage of standard library for sha* hashing, as far as I am aware. Couldn't find sha-crypt with it's 21 steps in my sdk. To be honest I couldn't even find sha-crypt compatible base64 encoding in standard lib. I'd be happy to get a hint, where to look for it.

pitpalme commented 3 years ago

Nevermind, please hold back this PR. I'll rearrange code. Just noticed "MD5Crypt" in md5crypt.go and will try to move implementation into a separate file and in a similar simplicity.

pitpalme commented 3 years ago

Here I am - again. I've moved implementation, so "basic.go" is kept slim.

Open to suggestions for improvement - and open for pointers to SHA crypt implementation in standard library. :)

pitpalme commented 3 years ago

OK, I guess I'm done. Sorry for not submitting one commit at one time. But I wanted to not miss the chance for general feedback, before tweaking the code.

Would be happy to hear about chances, this PR being merged - or reasons it can't.

andig commented 3 years ago

@abbot newer Shelly devices require SHA256 according to RFC7617: https://shelly-api-docs.shelly.cloud/gen2/Overview/CommonDeviceTraits/#authentication

Is there any chance to look at the open PRs and see what can be merged?

abbot commented 3 years ago

Thanks for additional context. I have some reservations about adding crypto code directly into this package, so instead I started work on moving crypto-related code to x/crypto (in fact there was an open proposal https://github.com/golang/go/issues/14274 about that).