Code Monkey home page Code Monkey logo

Comments (16)

francislavoie avatar francislavoie commented on July 24, 2024

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.

from caddy.

13xforever avatar 13xforever commented on July 24, 2024

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.

from caddy.

francislavoie avatar francislavoie commented on July 24, 2024

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.

from caddy.

13xforever avatar 13xforever commented on July 24, 2024

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

from caddy.

francislavoie avatar francislavoie commented on July 24, 2024

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.

from caddy.

13xforever avatar 13xforever commented on July 24, 2024

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.

from caddy.

francislavoie avatar francislavoie commented on July 24, 2024

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.

from caddy.

13xforever avatar 13xforever commented on July 24, 2024

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.

from caddy.

francislavoie avatar francislavoie commented on July 24, 2024

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.

from caddy.

mholt avatar mholt commented on July 24, 2024

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.

from caddy.

13xforever avatar 13xforever commented on July 24, 2024

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

basicauth /path {
  user Base64Hash hexsalt
}

from caddy.

francislavoie avatar francislavoie commented on July 24, 2024

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.

from caddy.

13xforever avatar 13xforever commented on July 24, 2024

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).

from caddy.

francislavoie avatar francislavoie commented on July 24, 2024

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.

from caddy.

13xforever avatar 13xforever commented on July 24, 2024

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.

from caddy.

francislavoie avatar francislavoie commented on July 24, 2024

It is in the release notes:

image

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.

from caddy.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.