strophe / libstrophe

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

SCRAM-SHA-512 not -PLUS supported fails on login #241

Closed matthias-hmb closed 6 months ago

matthias-hmb commented 6 months ago

Situation

If the server supports SCRAM-SHA-512 and SCRAM-SHA-512-PLUS, but not SHA1/SHA256, libstrophe will choose SCRAM-SHA-512 Mechanism. (the plus variant for sha512 is not supported)

Problem

The sasl client first message will start with "y,,", (auth.c 602,...) because libstrophe supports plus variants in general. The server needs to reject, because plus is supported, but not used.

RFC5802:

If the flag is set to "y" and the server supports channel binding, the server MUST fail authentication. This is because if the client sets the channel binding flag to "y", then the client must have believed that the server did not support channel binding -- if the server did in fact support channel binding, then this is an indication that there has been a downgrade attack (e.g., an attacker changed the server's mechanism list to exclude the -PLUS suffixed SCRAM mechanism name(s)).

Solution

a) Exception for sha512 and report to not support plus variants (ugly) b) Support for sha512 plus variant.

As all the code for plus variants is already there, it is only a small patch/addition to add support for sha512:

diff -b -r src/common.h ../libstrophe-0.13.1/src/common.h                                                                                                                                                                                                                         
177a178                                                                                                                                                                                                                                                                           
> #define SASL_MASK_SCRAMSHA512_PLUS (1 << 9)                                                                                                                                                                                                                                     
180c181                                                                                                                                                                                                                                                                           
<     (SASL_MASK_SCRAMSHA1_PLUS | SASL_MASK_SCRAMSHA256_PLUS)                                                                                                                                                                                                                     
---                                                                                                                                                                                                                                                                               
>     (SASL_MASK_SCRAMSHA1_PLUS | SASL_MASK_SCRAMSHA256_PLUS | SASL_MASK_SCRAMSHA512_PLUS)
diff -b -r src/scram.c ../libstrophe-0.13.1/src/scram.c                                                                                                                                                                                                                           
80a81,89                                                                                                                                                                                                                                                                          
> const struct hash_alg scram_sha512_plus = {                                                                                                                                                                                                                                     
>     "SCRAM-SHA-512-PLUS",                                                                                                                                                                                                                                                       
>     SASL_MASK_SCRAMSHA512_PLUS,                                                                                                                                                                                                                                                 
>     SHA512_DIGEST_SIZE,                                                                                                                                                                                                                                                         
>     (void (*)(const uint8_t *, size_t, uint8_t *))sha512_hash,                                                                                                                                                                                                                  
>     (void (*)(void *))sha512_init,                                                                                                                                                                                                                                              
>     (void (*)(void *, const uint8_t *, size_t))sha512_process,                                                                                                                                                                                                                  
>     (void (*)(void *, uint8_t *))sha512_done};                                                                                                                                                                                                                                  
>                                                                                                                                                                                                                                                                                 
86a96,97                                                                                                                                                                                                                                                                          
>     /* *1 */                                                                                                                                                                                                                                                                    
>     &scram_sha512_plus, 
sjaeckel commented 6 months ago

c) Or simply remove the non-RFC compliant SCRAM method, which I never should have implemented in the first place.

sjaeckel commented 6 months ago

Can you please check whether #243 fixes your issue?

I thought about it and though I would prefer c) I went with b) since breaking stuff isn't really what we want to do.

matthias-hmb commented 6 months ago

Thanks, yes it works. I agree with you, but I got lured into it by a server, that already supports that draft...