Skip to content

Add support for SRP Login#638

Merged
MattKiazyk merged 4 commits intoXcodesOrg:matt/SRPLoginfrom
abiligiri:SRPLogin_fix
Oct 29, 2024
Merged

Add support for SRP Login#638
MattKiazyk merged 4 commits intoXcodesOrg:matt/SRPLoginfrom
abiligiri:SRPLogin_fix

Conversation

@abiligiri
Copy link
Contributor

  • Switch to use https://github.com/adam-fowler/swift-srp with some modifications that are local
    • Pad g value to equal size of N while calculating clientProof
  • Use SHA256(plain-text-password) while computing key using PBKDF2
  • Added a unit test with some sample values

Fixes #630

- Switch to use https://github.com/adam-fowler/swift-srp with some modifications
  that are local
  - Pad g value to equal size of N while calculating clientProof
- Use SHA256(plain-text-password) while computing key using PBKDF2
- Added a unit test with some sample values
@fiznool
Copy link

fiznool commented Oct 28, 2024

Personally, I like the approach of using a well-tested library for SRP, and the upstream library seems to provide this. So I would be in favour of this implementation vs #639

If I can make a suggestion, forking adam-fowler/swift-srp and publishing as a separate package with the necessary changes would be my preference for the following reasons:

  • any changes to the upstream code can be more easily incorporated into a fork
  • the shared package can be consumed by the Xcodes app and cli

@abiligiri
Copy link
Contributor Author

Note that both options use a library for SRP. This uses https://github.com/adam-fowler/swift-srp/blob/main/Sources/SRP/client.swift and #639 fixes the srp code that was already in repo most likely from https://github.com/Bouke/SRP. I see that Adam Fowler implementation is still active. Maybe that is another reason to use it.

@MattKiazyk I was able to download and install Xcode 16.1.0. Not sure what you mean by download doesn't work.

@abiligiri
Copy link
Contributor Author

May I also request merging this for the release #632

- Use from https://github.com/abiligiri/swift-srp, version 1.1.0
  This is based on latest from upstream with changes required
- Remove local copy of swift-srp
@abiligiri
Copy link
Contributor Author

I cleaned this up a bit. Using patched swift-srp from https://github.com/abiligiri/swift-srp and removed local copy. I have also opened MR for upstream adam-fowler/swift-srp#13

@abiligiri abiligiri mentioned this pull request Oct 28, 2024
@MattKiazyk
Copy link
Contributor

@abiligiri hope you don't mind I clean up some extra prints that were in there as well as moved the swift-srp dependency to an xcodes fork. I copied your changes from your branch into main on our fork.

@MattKiazyk MattKiazyk merged commit 29bf770 into XcodesOrg:matt/SRPLogin Oct 29, 2024
@MattKiazyk MattKiazyk changed the title SRP Login works now Add support for SRP Login Oct 29, 2024
@MattKiazyk MattKiazyk added the bugfix Fixes a bug label Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants