Optional support for padded g in M1 (RFC5054 variants)

[davidspiess]

Hi Matt , first of all, thanks a lot for this awesome library. It’s been really helpful so far!

I’m currently using it to implement the iCloud SRP login flow. During integration I noticed that Apple’s implementation slightly deviates from the SRP-6a standard.

One difference is in the calculation of x, where the identity is not included. Apple appears to use: x = H(s | H(":" | P))
Since the library fortunately allows overriding the x calculation, I was able to adapt this part cleanly.

However, there is another deviation regarding the computation of M1. Apple expects g to be padded when calculating M1:
M1 = H(H(N) XOR H(PAD(g)) | H(U) | s | A | B | K)

While researching this, I found that the definition of M1 in older SRP specifications wasn't entirely clear, which led to multiple implementations that use a padded g. secure-remote-password/srp.net#19

At the moment, the library does not provide a way to influence the M1 calculation. My first thought was to introduce something like a setM1 hook (similar to the existing ones). However, the current implementation also relies on the internal helper function xorBytes, which is not accessible from the outside. Also given that this is only a very small variation in the formula, introducing a full customization hook might be maybe an overkill.

I looked for other implementations and noticed that the Python library pysrp introduced a dedicated RFC5054 compatibility mode that explicitly enables padding of g in M1:
https://github.com/cocagne/pysrp/blob/2f0211d7965290664af1537a0164dbb82a7270ff/srp/_pysrp.py#L208

Would you consider adding optional support for this kind of RFC5054-style compatibility, for example via a similar configuration flag or hook, or do you have any other ideas?

Also i completely understand if this feature request is out of scope, but since similar variants seem to exist in the wild, it might help others facing the same interoperability issues. :) Thank you!

[davidspiess]

I now created implementations for both ideas:

• RFC5054 compatibility flag: main...davidspiess:srp:rfc5054-compatibility
• M1 hook and a dedicated iCloud package (similar to cognito): main...davidspiess:srp:icloud-implementation

Let me know what you think :)

[bodgit]

Sorry, I didn't spot you had raised this issue. Let me take a look at your two branches.

[davidspiess]

No worries :)

[bodgit]

I've had a look at your changes and I think the RFC5054 compatibility flag is useful by itself for interoperability so please create a PR for that.

I'm also happy to include an iCloud wrapper similar to the Cognito code that sets all the correct options. Presumably if implemented in terms of your compatibility flag PR it becomes much simpler and you don't need to export any currently-internal functions?

I don't think you need to duplicate the group as the iCloud 2048 group uses the same parameters as the standard one so you can just use srp.GetGroup(2048). The Cognito 3072 group is slightly different to the standard one as it uses generator 2 with the 3072-bit prime whereas the RFC uses generator 5.

Feel free to send a combined or separate PRs and I'll be happy to review and merge those. I'll update some of the infrastructure and tests to get them current too as I haven't given this package much attention lately.

Interested to know how you're using this for iCloud.

[davidspiess]

I've had a look at your changes and I think the RFC5054 compatibility flag is useful by itself for interoperability so please create a PR for that.

Awesome, happy to do that 👍 But maybe read on before making a final decision.

I'm also happy to include an iCloud wrapper similar to the Cognito code that sets all the correct options. Presumably if implemented in terms of your compatibility flag PR it becomes much simpler and you don't need to export any currently-internal functions?

Yes, exactly. The implementation would become simpler, as setting the compatibility flag and using the srp.X callback would be sufficient.

I don't think you need to duplicate the group as the iCloud 2048 group uses the same parameters as the standard one so you can just use srp.GetGroup(2048). The Cognito 3072 group is slightly different to the standard one as it uses generator 2 with the 3072-bit prime whereas the RFC uses generator 5.

Thank you for pointing that out. I think i was a bit too focused on the cognito implementation, when creating the iCloud package. Dropped it in davidspiess@18a3345

Interested to know how you're using this for iCloud.

Sure, here's a bit of background :)

There is currently an open issue with iCloud authentication in rclone. The existing implementation does not yet use the iCloud SRP flow and was likely disabled or blocked by Apple.

Out of interest, I tried to implement the SRP authentication flow. However, I didn’t want to implement SRP from scratch, so I started looking for a suitable library. Since Apple does not support the SRP-6a standard, this turned out to be a bit tricky. Most iCloud SRP implementations I found during my research are copy pasted variations of the kong/srp implementation. That’s how I came across your SRP library and the dedicated Cognito implementation. Then i thought, an iCloud variant could be a useful addition to your project.

Here is my attempt to implement the SRP flow using your library: https://github.com/davidspiess/rclone/tree/fix-8587-invalid-session-token-error-srp-bodgit.

At the moment it doesn't fully work, as g isn't padded correctly.

In the meantime, another developer has implemented a pull request with a complete SRP solution. Still, i think the compatibility flag/iCloud extension could be useful for others.

[bodgit]

I'm still happy to accept a PR for iCloud. If you want to raise one first for the compatibility option and then raise the main iCloud PR in terms of that feature once merged then I think that would be fine.

I hadn't seen the rclone project, thanks for the info.