Fix SChannel pointer mismatches and SCRAM fallback - #270
Conversation
|
forgot to run |
sjaeckel
left a comment
There was a problem hiding this comment.
Please separate those changes into at least two commits.
Regarding the SCRAM fallback I'm not so sure if that patch is correct, but I didn't invest too much time into reviewing it (yet).
|
@sjaeckel apologies for the delay, life kinda got in my way for a bit. hopefully that is what you were looking for. Like I said if its not a good patch don't worry about it, it just made it work for me, and i figured providing some kinda solution is probably better than a drive-by issue. |
|
I picked the schannel changes directly to master. Now we've only to figure out how the SCRAM fallback should work. |
|
As for } else {
char cb_flag = 'n';
const char *dummy_type;
size_t dummy_len;
/* determine if channel binding is supported before advertising it */
if (is_secured &&
tls_init_channel_binding(conn->tls, &dummy_type, &dummy_len) == 0) {
cb_flag = 'y';
}
l = strophe_snprintf(message, message_len, "%c,,n=%s,r=%s", cb_flag,
node, buf);
}this is just following the SCRAM spec on how to indicate that we aren't using channel binding, so that prosody would stop dropping the connection. Perhaps silently falling back to a lower security method is not optimal, however I'm not familiar enough with the codebase to really know what makes the most sense here. 🤷 |
Add a runtime check per TLS stack which returns whether channel binding is supported or not. This fixes #270 Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
|
I've pushed a potential alternative to the scram-channel-binding branch. Can you please check whether this fixes the issue for you? Could you please run two checks, once with only the first commit 419b616 ("Disable SCRAM PLUS variants when using schannel.") and see if that already fixes the issue? And the second check with both commits applied |
Heya, this is related to when I ~4 days ago asked in the profanity chat about a memory error in the tls library on windows. I really needed this working for a project, so I decided to try to fix it myself. This makes it work for me, I'm not an experienced c dev so its possible I've committed great sins lol.
The memory error was
tls_schannel.cpassingtls_t*instead ofstruct conn_interface*. I'm pretty confident of this fix.However fixing the pointer type brought me to another issue, where I was getting a "Memory allocation error" which came from
auth.cthrowing an error when the tls backend error-ed on the current authentication method rather than trying the next method. This seems to be caused by the schannel backend not supporting channel binding, however I'm not exactly sure if that is fixable (by me), so instead I fixed the falling back.Once falling back was fixed, prosody would throw an error because we indicate channel binding support if tls is enabled, rather than checking if the backend supports it. I simply added a check to see if the tls backend has channel binding, and using that to advertise support or not.