search

strophe / libstrophe

A simple, lightweight C library for writing XMPP clients
http://strophe.im/libstrophe
Other
400 stars 162 forks source link

SCRAM-SHA-1-PLUS + SCRAM-SHA-256-PLUS + SCRAM-SHA-512-PLUS supports #133

Closed Neustradamus closed 8 months ago

Neustradamus commented 5 years ago

After:

EDIT done by @sjaeckel and @pasis:

Commits:

Can you add supports of:

You can add too:

"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]".

https://xmpp.org/extensions/inbox/hash-recommendations.html

-PLUS variants:

IMAP:

LDAP:

HTTP:

2FA:

IANA:

Linked to:

sjaeckel commented 5 years ago

FYI I've added SCRAM-SHA-256 and SCRAM-SHA-512 support in https://github.com/sjaeckel/libstrophe/tree/refactor-scram

Waiting for #138 to open a PR

pasis commented 4 years ago

I pushed @sjaeckel implementation of SCRAM-SHA-256/512 to a branch: https://github.com/strophe/libstrophe/tree/refactor-scram . Can anyone check it, because I don't have access to a server with these mechanisms?

Neustradamus commented 4 years ago

@pasis: Thanks a lot for SCRAM-SHA-256 and SCRAM-SHA-512 from @sjaeckel.

Now it is a good point for other SCRAM too.

For test, there is:

cc @maranda

sjaeckel commented 4 years ago

@pasis thanks a lot for the heavy lifting!

Do you want to continue on the PLUS variants or should I?

pasis commented 4 years ago

@sjaeckel , I don't have time in the near future, so you can proceed with it if you want. The thing is that you will need support from TLS modules. You can implement it for openssl and leave a stub for other modules. PLUS variants should fail if TLS module doesn't support required functionality.

pasis commented 4 years ago

I fixed few bugs in current implementation. SCRAM-SHA-256 seems working, however, SCRAM-SHA-512 still fails.

Neustradamus commented 4 years ago

@maranda: What do you think about SCRAM-SHA-512? Maybe linked to QXmpp, @jlaine have spoken here: https://github.com/qxmpp-project/qxmpp/issues/177

Neustradamus commented 4 years ago

@pasis: Can you test with jackal.im? It is based on Jackal XMPP server: https://github.com/ortuman/jackal/ cc: @ortuman

pasis commented 4 years ago

@Neustradamus , when I checked last time, jackal.im didn't have support of SCRAM-SHA-512. Maximum SCRAM-SHA-256.

As an option, I can disable SCRAM-SHA-512 for now.

Neustradamus commented 4 years ago

@ortuman: Can you look the problem? It is very important!

ortuman commented 4 years ago

šŸ‘‹ Sorry for the late reply!

After implementing SCRAM-SHA-512 i forgot to enable it on jackal.im šŸ¤¦ā€ā™‚

Now it's ready šŸ‘‡ 

<stream:features version="1.0">
<mechanisms xmlns="urn:ietf:params:xml:ns:xmpp-sasl">
<mechanism>PLAIN</mechanism>
<mechanism>DIGEST-MD5</mechanism>
<mechanism>SCRAM-SHA-1</mechanism>
<mechanism>SCRAM-SHA-1-PLUS</mechanism>
<mechanism>SCRAM-SHA-256</mechanism>
<mechanism>SCRAM-SHA-256-PLUS</mechanism>
<mechanism>SCRAM-SHA-512</mechanism>
<mechanism>SCRAM-SHA-512-PLUS</mechanism>
</mechanisms>
<register xmlns="http://jabber.org/features/iq-register"/>
</stream:features>
pasis commented 4 years ago

I found why SHA512 failed - there were 2 errors:

  1. Bug in sha512 implementation
  2. HMAC implementation was made according to RFC2104 where blocksize was 64 for all hashes. But SHA384 and SHA512 appeared after the RFC publication and blocksize for them must be 128.

After fixing the issues I managed to log into jackal.im with SCRAM-SHA-512.

However, lightwitch.org authentication still fails. When I try to set blocksize to 64 (which is incorrect), I can log into lightwitch.org. So, there must be a bug in HMAC implementation there. I created a PR to fix it: maranda/metronome#497

Neustradamus commented 4 years ago

@pasis: Have you progressed? Metronome has been solved :) Thanks a lot.

pasis commented 4 years ago

SCRAM-SHA-256 and SCRAM-SHA-512 are in master. Thanks to @sjaeckel for the implementation.

sjaeckel commented 4 years ago

Thanks a lot for finishing up!

Neustradamus commented 4 years ago

@pasis, @sjaeckel: It is possible to add -PLUS variants and SCRAM-SHA-224(-PLUS) and SCRAM-SHA-384(-PLUS) too?

sjaeckel commented 4 years ago

I had a look today for the 224&384 variants and I'm strongly opposing adoption of those, and I kindly ask you to stop pushing others for their adoption, since they are not standardized!

pasis commented 4 years ago

I agree with @sjaeckel, we don't need 224&384 at this point. I tried to read about -PLUS variants, but not ready to implement them yet - need more time to find info. Unfortunately, I can't effort much time now.

Neustradamus commented 4 years ago

Thanks a lot to @sjaeckel and @pasis for SCRAM-SHA-256 and SCRAM-SHA-512 in libstrophe 0.10.0.

Hope the last part "-PLUS" variants one day...

Neustradamus commented 3 years ago

@sjaeckel and @pasis: For your informations:

SCRAM-SHA-512(-PLUS):

SCRAM-SHA3-512(-PLUS):

Neustradamus commented 2 years ago

@sjaeckel, @pasis: What do you think? I have found it:

Links:

Neustradamus commented 2 years ago

Good news!

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

sjaeckel commented 11 months ago

@Neustradamus I'm currently implementing the -PLUS variants.

Do you remember maybe already having a discussion in the past about the ranking on which should be preferred over another mechanism, besides what's stated in RFC8600:

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

FMU this means SHA256-PLUS > SHA256 > SHA1-PLUS > SHA1 but shouldn't the ranking better be SHA256-PLUS > SHA1-PLUS > SHA256 > SHA1?

Side-note: after reading the comments on the SHA[3]-512 drafts I changed my opinion on those drafts from neutral to strongly opposing their adoption as an RFC and I'm considering the removal of support for SHA512.

Neustradamus commented 11 months ago

@sjaeckel: Nice! :)

Little details, to know easily:

To have a full compatibility...

The problem is that there is an expired I-D draft:

SCRAM-SHA-512(-PLUS) is already used by several projects and SCRAM-SHA3-512(-PLUS) too.

It will be published soon... and it is in SCRAM-BIS and SCRAM-2FA too.

sjaeckel commented 11 months ago

The problem is that there is an expired I-D draft:

Thanks, that's the pointer that I needed and @samwhited also put the order as I expected.

SCRAM-SHA-512(-PLUS) is already used by several projects and SCRAM-SHA3-512(-PLUS) too.

It will be published soon... and it is in SCRAM-BIS and SCRAM-2FA too.

I'll wait until they're in an RFC.

Neustradamus commented 11 months ago

@sjaeckel: I do not know when it will be released... but if you want compatibility with others...

SamWhited commented 11 months ago

Pretty much everyone that supports SCRAM supports SCRAM-SHA-1 and SCRAM-SHA-256, that is all you need for near-universal compatibility. Furthermore, there is no known benefit to SCRAM-SHA-512 over the SHA-256 variant at this time. Stop spamming every issue tracker trying to get people to support SCRAM-SHA-512 without thinking things through properly first. A bigger number is not "better", and it's a perfectly reasonable choice for the author to wait for the RFC to be reviewed before implementing it.

Neustradamus commented 8 months ago

@sjaeckel: Good job! :)

Only one remark, can you add "tls-server-end-point" support too?

It is needed for security and for example here (In more RFCs):

There is a talk about it on Standards ML:

Thanks in advance.