After renaming basicauth to basic_auth directive Caddyfile fails to load if custom salt argument was used

[13xforever]

Basically, this was an undocumented change that removed support for custom salt. Old configuraitons using this feature can not be updated to use the new basic_auth directive without regenerating all the hashes (in some cases this is not even possible).

Please at the very least update the documentation, adding the migration guide/breaking changes section, but ideally restore the compatibility with old config format.

[francislavoie]

Scrypt support was deprecated a long time ago (about 2 years). We removed scrypt support. It doesn't have to do with renaming the directive.

[13xforever]

well it's hard to retrace the changes without links to older documentation version, my config is using default parameters, and it's not apparent that other algorithms didn't support salt parameter etc

I have a very simple issue: after upgrade and following the release note changes, my config doesn't work anymore and I have no easy way to migrate; I have no idea if scrypt was the default years ago, or that salt parameter was only applicable to scrypt either.

[francislavoie]

The default has always been bcrypt which has the salt integrated in the hash string. The only way you could need the salt is if you used scrypt which has been deprecated for two years. You would have seen warnings in your logs all this time about it. You should have heeded those warnings and migrated your password hashes.

[13xforever]

realistically no one would bother to look at the logs until everything is broken, unless you're a big corpo and actually care about proactive infra monitoring

anyway, I don't see any warnings in the systemd journal before the upgrade, and if what you're saying is true and salt was silently ignored all this time, I'll try simply removing it from config file and see if the old credentials still work

[francislavoie]

I disagree. You always should keep an eye on your logs. It's irresponsible to not look at your logs. That's the only avenue Caddy has to give you feedback about anything that might become a problem.

If you were using bcrypt hashes, then yes the salts in your config were never looked at, because the salt is built into the hash (delimited by $ characters). You may have hashes that were additionally base64 encoded - use any base64 decoder to try to reverse that and see if what you get starts with $2a$. If it does then you know you're using bcrypt. And you can replace your base64-encoded hashes with the $2a$ version which is slightly shorter and easier to understand. There's no reason to keep them base64-encoded anymore.

The reason we produced hashes with base64 before was because scrypt required it because it emitted the hashes in raw binary, so we allowed using base64 in the config. But that was redundant for bcrypt because it has its own textual format that doesn't require additional encoding. Now you can load both ways for bcrypt but the standard bcrypt format is preferred.

[13xforever]

Well you may disagree all you want, but the fact is it's hard to retrace what's broken after upgrade and why. As I said, at the very least there should be a note in the documentation that explains the configuration format change and migration notes, including the change of the default hashing algorithm, the removal of extra parameters, and how people can tell they're affected.

[francislavoie]

The note existed in the docs for two years that scrypt was deprecated. When we remove it, we also clean up the docs to remove references to it to get rid of irrelevant information.

If you tried to use salts with bcrypt, that was a user-error, because that doesn't make any sense (because bcrypt manages its own salt in the hash). Trying to use a custom salt doesn't make sense anyway, salts should always be randomly generated at the time of generating the password, and should always be different for all passwords, otherwise it's possible to produce a rainbow table using that salt if it's reused.

[13xforever]

That's a strange decision, to remove actually helpful note for people who will be struggling the most trying to figure out what needs to be fixed.

As for primer on salts, thanks, I have sufficient background in crypto, but it doesn't mean I can look at the legacy caddyfile and immediately understand what all the random parameters are meant to be especially when everything is using defaults without explicitly stating the configured algorithm anywhere. All I see as a user, is that first it fails to start because it wants basicauth to be renamed to basic_auth, and then it fails because there's an extra parameter in config, without further clarification.

Anyway, it was using scrypt or whatever default was years ago when the config file was generated, so I'll have to update all the passwords, which sucks, not gonna lie.

[francislavoie]

Scrypt was never the default. You would have had to explicitly choose to use it. And Caddy will have warned every time you load with the algorithm configured to scrypt that it was deprecated. You can revert to 2.7.6 if you need to keep using those for the time being, giving you time to migrate.

All I see as a user, is that first it fails to start because it wants basicauth to be renamed to basic_auth

No, that's a deprecation warning, not a reason that it would fail to start. Caddy will still work with both directive names.

You still haven't shown any of your logs nor your Caddyfile. So we might be talking past eachother, making assumptions about what eachother are seeing. Please help us by showing detail if you still think there's an actual problem.

[mholt]

The default has been bcrypt ever since there was a default before Caddy v2 was even released, all the way back in early 2020: 57c6f22

Maybe if you show us your config we can help you understand more about the changes in 2.8. Then we can decide whether this needs to stay open if there's anything actionable for us to do.

[13xforever]

my config file literally looks like this, so I have zero clue:

basicauth /path {
  user Base64Hash hexsalt
}

[francislavoie]

How do you know that it doesn't work if you remove the salt? How did you actually try that?

Scrypt was dropped in this PR #6091. If you were using scrypt you would have had to specify the algorithm like this:

basicauth /path scrypt {

}

But since you aren't doing that, you must be using bcrypt, in which case the salts never did anything (notice the code in the PR, the salt parameter was ignored with _ in the function signatures) and should be dropped from your config. And like I said, you should try base64-decoding the hashes to see if they contain $2a$ and if they do use the decoded version in your config.

[13xforever]

yeah, I've regenerated the password for one user that I know for sure is used in my ddns script, everything else I'll only know when something will break somewhere, (I can't find the old password hash values to see if it was double-b64 encoded or not; I had to remove the auth part of the config to make caddy work again for other parts of the server).

[francislavoie]

I can't find the old password hash values to see if it was double-b64 encoded or not

Wait, you've already lost your basicauth config? I'm not sure I follow.

If so, then obviously there's nothing else to do here and we should close this issue.

[13xforever]

Yes, I can live with the changes I had to make on the double. But closing the issue? I disagree.

The whole point I opened it is because I do not agree with your approach of releasing breaking changes without proper mention in the release notes and removing any helpful info on changes/migration from documentation.

Like, yes, you've deprecated it some time ago, but you're actually breaking the compatibility, and it's impossible to find anything useful anywhere right now.

[francislavoie]

It is in the release notes:

https://github.com/caddyserver/caddy/releases/tag/v2.8.0

If you were specifying salts when using bcrypt, that has never done anything useful as I've explained, and therefore was user error. We didn't warn about that in the past, you're right, but it doesn't make sense anyway so we didn't think to warn in that case.

I'll close this, there's nothing actionable for us to do here.

• If you were using scrypt, you've had warnings for a very long time; you could continue to use v2.7.6 until you've migrated the hashes to bcrypt.
• If you used invalid config (had config outside of our intended/documented API), then... fix it.

That's all.