Remove deprecated code and OLMKit dependency #1880
Conversation
There was a problem hiding this comment.
Great stuff 👏
A lot of this goes over my head but we had a chat in a call about it and I've left some summary comments. Definitely needs an approval by someone with more crypto knowledge before merging but looks decent to me.
(Its a shame so many tests are going but they're all clearly based around the legacy crypto stack).
| @@ -24,143 +24,3 @@ | |||
| #pragma mark - Constants | |||
|
|
|||
| NSString *const MXCrossSigningToolsErrorDomain = @"org.matrix.sdk.crosssigning.tools"; | |||
There was a problem hiding this comment.
Do we need all the imports above given this is all that is all that is left?
| { | ||
| NSString *threePidConcatenation = [NSString stringWithFormat:@"%@ %@ %@", threepid, medium, pepper]; | ||
|
|
||
| OLMUtility *olmUtility = [OLMUtility new]; | ||
| NSString *hashedSha256ThreePid = [olmUtility sha256:[threePidConcatenation dataUsingEncoding:NSUTF8StringEncoding]]; | ||
| hashedTreePid = [MXBase64Tools base64ToBase64Url:hashedSha256ThreePid]; | ||
|
|
||
| threePidArrayByThreePidConcatHash[hashedTreePid] = threepidArray; | ||
| } |
There was a problem hiding this comment.
I think where you have a removal of the plain OLMUtility in an existing class, we should find a replacement using system libraries. E.g. this looks like just a SHA256 which we should still be doing for the identity server.
| // Validate the secret before storing it | ||
| if ([self checkSecret:secret withSecretId:secretId]) | ||
| { | ||
| if (![secret isEqualToString:[self.dependencies.secretStore secretWithSecretId:secretId]]) | ||
| { | ||
| MXLogDebug(@"[MXRecoveryService] recoverSecrets: Recovered secret %@", secretId); | ||
|
|
||
| [updatedSecrets addObject:secretId]; | ||
| [self.dependencies.secretStore storeSecret:secret withSecretId:secretId]; | ||
| } | ||
| else | ||
| { | ||
| MXLogDebug(@"[MXRecoveryService] recoverSecrets: Secret %@ was already known", secretId); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| MXLogDebug(@"[MXRecoveryService] recoverSecrets: Secret %@ is invalid", secretId); | ||
| [invalidSecrets addObject:secretId]; | ||
| } |
There was a problem hiding this comment.
Are we sure that Rust will validate these secrets for us?
There was a problem hiding this comment.
We discussed in a call that we should check whether these MXOlm…GroupSession classes are used with Rust crypto or not.
This is better reviewed commit by commit.
This is a massive PR about removing legacy code, might split it in multiple PRs, but this is actually the first stable state in which I was able to make everything compile and work
What is done and what is missing: