Skip to content

Conversation

@estheruary
Copy link

@estheruary estheruary commented Feb 21, 2025

Addresses #494

I know, golang.org/x/crypto/pbkdf2 isn't in the standard library in Go 1.22 but x/crypto is maintained by the Go core devs and it is in the stdlib as of Go 1.24.

While it is true 3rd party packages will have to update their code, I've seen it already wrong in the wild.

@mperham
Copy link
Collaborator

mperham commented Feb 21, 2025

Is this backwards compatible with existing clients?

@mperham
Copy link
Collaborator

mperham commented Feb 21, 2025

We'll need to add backwards compatibility, maybe with a version or hash algo prefix on the hex, e.g. "sha256:<hex>" to signal the server. If it's not there, we just use the original algo.

@estheruary
Copy link
Author

Seems reasonable. I'll get on it.

If the client is updated and knows to use the new KDF it will send
sha256:<hash> and the server will ues the new algo. If it doesn't it
will simply send <hash> and the server will fall back to the old algo.
@estheruary
Copy link
Author

@mperham Is this about what you were thinking?

@mperham
Copy link
Collaborator

mperham commented Apr 3, 2025

Now what happens if a new client is talking to an old server?

@estheruary
Copy link
Author

estheruary commented Apr 3, 2025

woah woah woah there, you didn't ask for forwards compatibility :P

I'm not sure it's possible to do this in general because some 3rd party client wouldn't be required to implement a fallback and wouldn't keep sending the two hashes forever or peep the server version, is it enough that the Go client handles this gracefully?

@mperham
Copy link
Collaborator

mperham commented Apr 3, 2025

I think the server sends a protocol version in the initial hash to the client? We should bump that. I want to maximize compatibility as this wasn't a "hair on fire" fix, breaking old clients is not acceptable.

@estheruary
Copy link
Author

yep, I can do that!

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

Successfully merging this pull request may close these issues.

2 participants