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.

Sign up for GitHub

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

Jump to bottom

Add SCRAM-SHA-1-PLUS, SCRAM-SHA-224-PLUS, SCRAM-SHA-256-PLUS, SCRAM-SHA-384-PLUS, SCRAM-SHA-512-PLUS, SCRAM-SHA3-512(-PLUS) supports #552

Open
Neustradamus opened this issue Jan 20, 2019 · 18 comments
Open

Add SCRAM-SHA-1-PLUS, SCRAM-SHA-224-PLUS, SCRAM-SHA-256-PLUS, SCRAM-SHA-384-PLUS, SCRAM-SHA-512-PLUS, SCRAM-SHA3-512(-PLUS) supports #552

Neustradamus opened this issue Jan 20, 2019 · 18 comments

Comments

@Neustradamus
Copy link
Contributor

Neustradamus commented Jan 20, 2019 •
edited
Loading

"When using the SASL SCRAM mechanism, the SCRAM-SHA-256-PLUS variant SHOULD be preferred over the SCRAM-SHA-256 variant, and SHA-256 variants [RFC7677] SHOULD be preferred over SHA-1 variants [RFC5802]".

SCRAM-SHA-1(-PLUS):

SCRAM-SHA-256(-PLUS):

SCRAM-SHA-512(-PLUS):

SCRAM-SHA3-512(-PLUS):

In more than

  • SCRAM-SHA-1
  • SCRAM-SHA-256

It is possible to add?

  • SCRAM-SHA-1-PLUS
  • SCRAM-SHA-224(-PLUS)
  • SCRAM-SHA-256-PLUS
  • SCRAM-SHA-384(-PLUS)
  • SCRAM-SHA-512(-PLUS)

Linked to:
This was referenced Jul 25, 2019
Closed
Closed
Closed
@ksmurchison
Copy link
Contributor

We can add 224, 384, 512 easily since OpenSSL now appears to support them in the EVP API.
-PLUS will take more work. A PR would be appreciated.

@Neustradamus
Copy link
Contributor Author

Good if you can add a better SHA-2 family (SHA-224, SHA-384, SHA-512) :)

I think, it will be possible to have SHA-3 family (SHA3-224, SHA3-256, SHA3-384, SHA3-512) too?

Who could see for -PLUS variant part?

Have you looked for the documentation?

ksmurchison added a commit that referenced this issue Aug 10, 2019
@ksmurchison
SCRAM: added -224, -384, -512 (issue #552)
8a1f697
@Neustradamus
Copy link
Contributor Author

Neustradamus commented Aug 13, 2019 •
edited
Loading

@ksmurchison: Thanks for SCRAM-SHA-224, SCRAM-SHA-384, SCRAM-SHA-512!

Now there are SCRAM-SHA-1, SCRAM-SHA-224, SCRAM-SHA-256, SCRAM-SHA-384, SCRAM-SHA-512 :)

Have you looked for SCRAM-SHA3?

@dhawes dhawes mentioned this issue Sep 17, 2019
Closed
@Neustradamus
Copy link
Contributor Author

Neustradamus commented Sep 18, 2019 •
edited
Loading

@dhawes: Please look:

Thanks a lot to @aamelnikov and @ksmurchison too.

This was referenced Nov 6, 2019
Closed
Open
@Neustradamus
Copy link
Contributor Author

Neustradamus commented Nov 28, 2019 •
edited
Loading

@aamelnikov, @ksmurchison: There is a problem, can you look?

@SamWhited
Copy link

I'd like to push back on this a bit; adding the PLUS variants of SCRAM-SHA1 and SCRAM-SHA-256 would be great, but the other mechanisms that the OP mentions are not part of the SCRAM family of mechanisms, and are not defined in any IETF standard. That is to say, there is no such mechanism as SCRAM-SHA-512 or SCRAM-SHA-384. Since they are not supported by most SCRAM clients and do not provide any known security benefit over either of the other SCRAM mechanisms (since the hash is just used in an HMAC so only pre-image attacks would matter), please consider removing these mechanisms or at least disabling them by default.

If the mechanisms are left in, and eventually a SCRAM-SHA-512 mechanism is created by the IETF but it differs somehow from the other mechanisms, you will have an incompatible version. This also may encourage other developers to implement the non-standard mechanism and/or to not support the actual standardized mechanisms out of some misguided idea that bigger numbers means that it is somehow "more secure". We don't want to have to clean up a mess later, or encourage other SCRAM stacks to invent their own mechanisms which may only work with one or two clients and servers when safe, standardized, mechanisms have already been thought through by a group with expertise in these matters.

TL;DR let's not make up our own crypto, it's dangerous. Please trust the experts and wait until the IETF reviews and standardizes a new mechanism before implementing it.

Thanks for your consideration.

@Neustradamus
Copy link
Contributor Author

Neustradamus commented Oct 30, 2020 •
edited
Loading

SCRAM-SHA-512(-PLUS):

SCRAM-SHA3-512(-PLUS):

@SamWhited
Copy link

Those are I-Ds, not RFCs. They are IDs by a well respected author, and if they are ever accepted then we should use them. In the mean time let's not make up our own crypto, it's bad practice.

@Neustradamus
Copy link
Contributor Author

@SamWhited: It will be validated soon :)
A good job from @aamelnikov.
In several RFCs, there are some links to draft before RFC.

@SamWhited
Copy link

I certainly hope so, and when it is then we can use them, as I've said on every other issue where you've pushed people to make up their own auth mechanisms for some reason.

@Neustradamus
Copy link
Contributor Author

@SamWhited: Do not forget that the code for SCRAM in Cyrus SASL has been done by the author of the SCRAM RFCs himself @aamelnikov and by @ksmurchison too.

@Neustradamus
Copy link
Contributor Author

A lot of projects have already SCRAM-SHA-512: https://www.google.com/search?q=SCRAM-SHA-512

@SamWhited
Copy link

SamWhited commented Oct 30, 2020 •
edited
Loading

I'm sure the code is fine, and I'm very excited they're writing I-Ds to update the registry, I'm pushing back against implementing them and encouraging others to use non-standardized mechanisms before we're sure what the final version of those mechanisms looks like. Chances are they won't change, and I'll be very excited to see these mechanisms implemented once they are in the registry.

A lot of projects implementing it doesn't mean it's a good idea, and most mainstream services don't implement it, so you're not even getting any sort of compatibility befit from rushing an implementation.

@quanah
Copy link
Contributor

quanah commented Nov 1, 2020

I see no major issue here. Any pull request should be against the master (development) branch, and not against the 2.1 release branch. I would suggest putting the non-finalized RFC versions behind an IFDEF that can be flagged at compile time for people who want to enable/experiment with them. If the finalized form differs, we can fix and unifdef them for them to be officially supported. It is quite common for draft form RFCs to be implemented in software while the draft is finalized (For example, see password policy support found in most all LDAP projects, despite it still being in draft form).

@SamWhited
Copy link

I would suggest putting the non-finalized RFC versions behind an IFDEF that can be flagged at compile time for people who want to enable/experiment with them.

Putting them behind an IFDEF seems good enough to me.

@Neustradamus
Copy link
Contributor Author

It is official, it is here: RFC 9266: Channel Bindings for TLS 1.3:

Thanks @SamWhited.

@Neustradamus Neustradamus changed the title Add SCRAM-SHA-1-PLUS, SCRAM-SHA-224(-PLUS), SCRAM-SHA-256-PLUS, SCRAM-SHA-384(-PLUS), SCRAM-SHA-512(-PLUS) supports Add SCRAM-SHA-1-PLUS, SCRAM-SHA-224-PLUS, SCRAM-SHA-256-PLUS, SCRAM-SHA-384-PLUS, SCRAM-SHA-512-PLUS, SCRAM-SHA3-512(-PLUS) supports Aug 6, 2023
@Neustradamus
Copy link
Contributor Author

@aamelnikov, @ksmurchison, @quanah, @hyc, @bgermann: It is possible to add SCRAM-SHA3-512?

Can you see for Channel Binding too?

Thanks in advance.

This was referenced Nov 3, 2023
Open
Open
@Neustradamus
Copy link
Contributor Author

Dear @cyrusimap team, @aamelnikov, @ksmurchison, @quanah, @hyc, @bgermann, @dilyanpalauzov, @iboukris, @simo5,

Can you look for Channel Binding for TLS 1.3 support?

There is a recent history with jabber.ru MITM and SCRAM-SHA-*-PLUS is the security solution!

Some sources about jabber.ru:

Thanks in advance.

Linked to:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Neustradamus @SamWhited @quanah @ksmurchison