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

Feature Request: support modules implementing http.authentication.hashes #6670

Open
marvinlwenzel opened this issue Nov 1, 2024 · 1 comment

Comments

@marvinlwenzel
Copy link

marvinlwenzel commented Nov 1, 2024

Hello.

Caddy Name Spaces lists http.authentication.hashes as a namespace for custom implementations for hashing in use with basic_auth and caddy-hash-password.

As of 2.8.4, there appears to be no check for modules implementing http.authentication.hashes. Just hard coded cases going "bcrypt or bust".

switch algorithm {
case "bcrypt":
hash, err = BcryptHash{}.Hash(plaintext)
hashString = string(hash)
default:
return caddy.ExitCodeFailedStartup, fmt.Errorf("unrecognized hash algorithm: %s", algorithm)
}

case "bcrypt":
cmp = BcryptHash{}
default:
return nil, h.Errf("unrecognized hash algorithm: %s", hashName)
}

The basic_auth Docu states

<hash_algorithm> is the name of the password hashing algorithm (or KDF) used for the hashes in this configuration. Default: bcrypt

caddy-hash-password states

--algorithm may be bcrypt or any installed hash algorithm. Default is bcrypt.

Those seem misleading in the first case and false in the second.

I would like to change that.

I have started working on this to get an understanding of challenges and impact to get a proper discussion started. (So far, I was able to have both bcrypt and a custom alternative working without observing bugs. No tests, performance tests, or anything done yet. If you want to have a look: https://github.com/marvinlwenzel/caddyCrypt, my current diff master...marvinlwenzel:caddyCrypt:master and the custom hash impl https://github.com/marvinlwenzel/fvttcrypt)

  1. Given that the doc suggests that adding a custom hasher should be possible and the necessary interfaces offer the needed abstraction, I assume that would be a feature the project is generally interested in.
  2. I guess the impact on non-test-code would be relatively small from what I have been able to test manually and from looking into the code base.
  3. Right now I have no idea what the scope of test code would be. Before I dive into that (and ask for guidance as my golang-integ-test-experience is zero), I wanted to start the discussion to avoid wasted effort.

Should http.authentication.hashes modules be supported, or is this too niche of a scenario to be relevant? Is there more to be considered when this is tackled than what I stated thus far (more config, guidelines on hash formats, idk...)?

@marvinlwenzel marvinlwenzel changed the title Feature Request: scan modules for all implementations of http.authentication.hashes Feature Request: support modules implementing http.authentication.hashes Nov 1, 2024
@francislavoie
Copy link
Member

Yeah you're right it should use https://pkg.go.dev/github.com/caddyserver/caddy/v2#GetModules instead. PRs are certainly welcome to fix it!

FYI we used to have scrypt support but we removed it because it needed to use multiple parameters instead of having a batteries included string format like bcrypt, and doesn't have as great security properties.

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