Skip to content

Commit 6e8eeac

Browse files
Check order of SCRAM attributes and abort auth on wrong order
Also fix SASL2 error type extraction when receiveing <failure/> responses.
1 parent e64ebfd commit 6e8eeac

File tree

2 files changed

+13
-7
lines changed

2 files changed

+13
-7
lines changed

Monal/Classes/SCRAM.m

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ -(NSString*) clientFirstMessageWithNoMatchingChannelBindingFound:(BOOL) noMatchi
8787
MLAssert(!_finishedSuccessfully, @"SCRAM handler finished already!");
8888
MLAssert(!_serverFirstMessageParsed, @"SCRAM handler already parsed server-first-message!");
8989
//no matching channel binding could be found, but server advertised channel-binding capability
90-
//--> tell the server we DON'T support channel-binding and abort the authentication later on,
91-
// if the server doesn't support SSDP to sign this fact
90+
//--> tell the server we DON'T support channel-binding and, if the server doesn't support SSDP
91+
// to sign this fact, abort the authentication later on,
9292
//NOTE: XEP-0440 makes tls-server-end-point mandatory and we support this
9393
//NOTE: ==> not having a match almost always means a MITM attacker tampered with the XEP-0440 list
9494
if(noMatchingChannelBindingFound)
@@ -109,7 +109,7 @@ -(MLScramStatus) parseServerFirstMessage:(NSString*) str
109109
{
110110
MLAssert(!_finishedSuccessfully, @"SCRAM handler finished already!");
111111
MLAssert(!_serverFirstMessageParsed, @"SCRAM handler already parsed server-first-message!");
112-
NSDictionary* msg = [self parseScramString:str];
112+
NSDictionary* msg = [self parseScramString:str withKeysRegex:@"^m?rsi.*$"];
113113
_serverFirstMessageParsed = YES;
114114
//server nonce MUST start with our client nonce
115115
if(![msg[@"r"] hasPrefix:_nonce])
@@ -140,7 +140,7 @@ -(MLScramStatus) parseServerFirstMessage:(NSString*) str
140140
-(NSString*) clientFinalMessageWithChannelBindingData:(NSData* _Nullable) channelBindingData
141141
{
142142
MLAssert(!_finishedSuccessfully, @"SCRAM handler finished already!");
143-
MLAssert(_serverFirstMessageParsed, @"SCRAM handler did not parsed server-first-message yet!");
143+
MLAssert(_serverFirstMessageParsed, @"SCRAM handler did not parse server-first-message yet!");
144144
//calculate gss header with optional channel binding data
145145
NSMutableData* gssHeaderWithChannelBindingData = [NSMutableData new];
146146
[gssHeaderWithChannelBindingData appendData:[_gssHeader dataUsingEncoding:NSUTF8StringEncoding]];
@@ -180,7 +180,7 @@ -(MLScramStatus) parseServerFinalMessage:(NSString*) str
180180
{
181181
MLAssert(!_finishedSuccessfully, @"SCRAM handler finished already!");
182182
MLAssert(_serverFirstMessageParsed, @"SCRAM handler did not parsed server-first-message yet!");
183-
NSDictionary* msg = [self parseScramString:str];
183+
NSDictionary* msg = [self parseScramString:str withKeysRegex:@"^(e|v).*$"];
184184
//wrong v-value
185185
if(![HelperTools constantTimeCompareAttackerString:msg[@"v"] withKnownString:_expectedServerSignature])
186186
return MLScramStatusWrongServerProof;
@@ -245,15 +245,21 @@ -(NSData*) hash:(NSData*) data
245245
return nil;
246246
}
247247

248-
-(NSDictionary* _Nullable) parseScramString:(NSString*) str
248+
-(NSDictionary* _Nullable) parseScramString:(NSString*) str withKeysRegex:(NSString*) keyRegex
249249
{
250+
NSRegularExpression* regex = [NSRegularExpression regularExpressionWithPattern:keyRegex options:0 error:nil];
251+
NSMutableString* keys = [NSMutableString new];
250252
NSMutableDictionary* retval = [NSMutableDictionary new];
251253
for(NSString* component in [str componentsSeparatedByString:@","])
252254
{
253255
NSString* attribute = [component substringToIndex:1];
254256
NSString* value = [component substringFromIndex:2];
255257
retval[attribute] = [self unquote:value];
258+
[keys appendString:attribute];
256259
}
260+
//check order of attributes in accordance to RFC 5802
261+
if([regex numberOfMatchesInString:keys options:0 range:NSMakeRange(0, [keys length])] == 0)
262+
return @{}; //return empty dictionary to make sure the wrong order doesn't lead to successful authentications
257263
return retval;
258264
}
259265

Monal/Classes/xmpp.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2716,7 +2716,7 @@ -(void) processInput:(MLXMLNode*) parsedStanza withDelayedReplay:(BOOL) delayedR
27162716
}
27172717
else if([parsedStanza check:@"/{urn:xmpp:sasl:2}failure"])
27182718
{
2719-
NSString* errorReason = [parsedStanza findFirst:@"{urn:ietf:params:xml:ns:xmpp-streams}!text$"];
2719+
NSString* errorReason = [parsedStanza findFirst:@"{urn:ietf:params:xml:ns:xmpp-sasl}*$"];
27202720
NSString* message = [parsedStanza findFirst:@"text#"];
27212721
DDLogWarn(@"Got SASL2 %@: %@", errorReason, message);
27222722
if([errorReason isEqualToString:@"not-authorized"])

0 commit comments

Comments
 (0)