Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should the Generator (g) be padded in the M calculation for SrpServer? #19

Closed
mrhappyasthma opened this issue Apr 14, 2022 · 4 comments
Closed

Comments

@mrhappyasthma
Copy link
Contributor

I'm trying to port an SRP implementation over to C# from python. The client is legacy and I cannot change it, so I'm just implementing a compatible server side of the protocol.

It was failing using this library until I noticed that the library doesn't Pad the g value here:
https://github.com/secure-remote-password/srp.net/blob/master/src/srp/SrpServer.cs#L130

(It doesn't pad in the SrcClient either. So it's consistent. But I'm wonder if we should update both?)

The python SRP library does pad this value when enabling support for rfc5054:
https://github.com/cocagne/pysrp/blob/master/srp/_pysrp.py#L205-L208

I tried jumping through the rfc5054 docs, but it doesn't even mention the M proof calculation so it's unclear if the padding should also apply there. It mentions padding k and u (which this library does) but doesn't claim one way or the other for M:
https://datatracker.ietf.org/doc/html/rfc5054

I was able to workaround this issue by doing the following:

SrpParameters parameters = SrpParameters.Create<SHA256>(N, g);
parameters.Generator = parameters.Pad(parameters.Generator);  // WORKAROUND
SrpServer srpServer = new SrpServer(parameters);
SrpSession serverSession = srpServer.DeriveSession((.........);

It works for my needs. But I guess the larger question for this issue is: Should this library actually be padding g in the M calculation? Or is the python srp implementation incorrect?

@yallie
Copy link
Member

yallie commented Apr 17, 2022

Hello Mark, thanks for posting your workaround.

This library aims to be interoperable with other standard-compliant implementations.
You're right, RFC5054 doesn't specify how exactly M value is calculated.
But looks like most implementations don't pad g when calculating M proof value.

Srp.net is compatible with Swift SRP which is able to connect to Apple's servers.
I don't think that we should change the current behavior, but we can add a note about
compatibility workaround for your python client implementation, if it's open source.

@mrhappyasthma
Copy link
Contributor Author

The python API is open source, it's the main srp implementation there. But the client I'm working with is actually using some C++ library. So presumably there's more than just 1 client doing things this way.

We could consider making it easier to configure the padding of g in the M calculation ,but since a workaround exists I don't think it's necessary. Perhaps we can just make a comment for future folks who may run into the same problem working with an alternate srp implementation.

What do you think?

@yallie
Copy link
Member

yallie commented Apr 19, 2022

Hello Mark, sorry for my late response.

SRP protocol has several obsolete revisions, and of course it makes sense to support them
if they are still in use somewhere. I don't think however that the library should encourage
the use of old revisions because they are less secure than the latest SRP-6a, so I believe
that your workaround is just fine.

Perhaps we can just make a comment for future folks who may run into the same problem working with an alternate srp implementation.

It would be great if you add a couple of lines to the readme about your workaround and when
it should be needed. There is a compatibility section that already has a few tips on that matter.

Or maybe implementations repo is a better place to mention partially-compatible libraries.

Thank you!

@mrhappyasthma
Copy link
Contributor Author

Sounds like a plan. When I get some downtime I'll update the docs.

yallie pushed a commit to secure-remote-password/implementations that referenced this issue May 26, 2022
I migrated from `pysrp` to `srp.net` and confirmed interoperability (assuming the caveats in the footnote).

For more details, see secure-remote-password/srp.net#19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants