Calculation of B

[dvj]

It seems there is a difference between the python and C implementation of how B is calculated, notably that in the C version:

            # B = kv + g^b
            BN_mul(self.tmp1, k, self.v, self.ctx)
            BN_mod_exp(self.tmp2, g, self.b, N, self.ctx)
            BN_add(self.B, self.tmp1, self.tmp2)

the modulo differs from the python version:

    self.B = (k*self.v + pow(g, self.b, N)) % N

Should that be BN_mod_add() instead?

[cocagne]

Possibly. Are you asking because you're trying to root-cause a problem? There have been a few bugs related to mod operations in the past but I haven't heard of one in quite a while. That would suggest that the current implementation is correct but it's far from a guarantee...

Tom

On Mon, Mar 21, 2016 at 12:05 AM, Doug Johnston notifications@github.com wrote:

It seems there is a difference between the python and C implementation of how B is calculated, notably that in the C version:

        # B = kv + g^b
        BN_mul(self.tmp1, k, self.v, self.ctx)
        BN_mod_exp(self.tmp2, g, self.b, N, self.ctx)
        BN_add(self.B, self.tmp1, self.tmp2)

the modulo differs from the python version:

self.B = (k*self.v + pow(g, self.b, N)) % N

Should that be BN_mod_add() instead?

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/cocagne/pysrp/issues/13

[dvj]

I'm trying to interop with another SRP implementation, which wasn't working, and debugging. Specifically, I was seeing different key lengths for A and B. This doesn't happen with the default ng of 2048, but using 4096 as an example:

    salt, vkey = srp.create_salted_verification_key('testuser', 'testpassword', ng_type=srp.NG_4096)
    usr = srp.User('testuser', 'testpassword', ng_type=srp.NG_4096)
    uname, A = usr.start_authentication()
    svr = srp.Verifier(uname, salt, vkey, A, ng_type=srp.NG_4096)
    s, B = svr.get_challenge()

which yields a key length of 512 for A, but a key length of 531 for B using the C implementation.

[cocagne]

Okay, that makes sense. pysrp & csrp were not originally designed to interoperate with other SRP implementations. So far as I know, there is no official "this is the standard way to implement SRP in general" but apparently there are several libraries that adhere to the SSL-defined implementation approach (not sure which ones those are but there's definitely more than one). There's an rfc5054_compat branch in both the pysrp & csrp git repositories that match the RFC5054 implementation requirements. Several people have had good luck interoperating with other implementations when using that branch so it's probably what you're looking for. At some point I'll probably merge that branch into the mainline and make it the default implementation.

Tom

On Mon, Mar 21, 2016 at 11:22 AM, Doug Johnston notifications@github.com wrote:

I'm trying to interop with another SRP implementation, which wasn't working, and debugging. Specifically, I was seeing different key lengths for A and B. This doesn't happen with the default ng of 2048, but using 4096 as an example:

salt, vkey = srp.create_salted_verification_key('testuser', 'testpassword', ng_type=srp.NG_4096)
usr = srp.User('testuser', 'testpassword', ng_type=srp.NG_4096)
uname, A = usr.start_authentication()
svr = srp.Verifier(uname, salt, vkey, A, ng_type=srp.NG_4096)
s, B = svr.get_challenge()

which yields a key length of 512 for A, but a key length of 531 for B using the C implementation.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/cocagne/pysrp/issues/13#issuecomment-199364016

[dvj]

I've now gotten things to work with the other SRP implementation using the rfc5054_compat branch. It does appear as if that line is now using a mod_add call, amongst a couple hashing changes.

I would highly encourage merging that with master, as the branch has several bugs and missing features. There was some hesitation in #11 to make it the default for backwards compatibility reasons, but I think looking at the long-term view of this module, using RFC5054 is the right thing to do. Besides, I doubt too many people have been waiting with baited breath since the last pypi release for a point release. :)

Now that I've become intimately familiar with the code, I may have a feature or two to add, including variable salt length, separating the private key generation and adding in other standard Ng options; all of which I needed to implement to use this module with the scheme I'm implementing against.

In the meantime, I would also suggest adding a note to the readthedocs page indicating that it's not RFC5054, as that point is not clear currently.

Thanks for the work.

[cocagne]

Thanks Doug. I agree with you on every point. The only reason I haven't already merged the rfc5054 branch over into the mainline is that it lacks backwards compatibility support. I agree that hardly anyone is eagerly awaiting the next release but I do suspect that quite a few people would be irked if new pip installs suddenly started breaking applications. It's not a difficult problem to solve, just time-consuming. Which is rather pathetic to say since time-consuming in this case probably amounts to about 8 hours of total effort between pysrp & csrp. When I originally wrote py/csrp, I had several use-cases in mind for production deployment. Unfortunately, I wound up changing jobs at approximately the same time as the original release and I have yet to find a good reason for using py/csrp at my current position. So I'm left maintaining these packages but not personally benefiting from them, other than helping out the OSS community that is. That's a plus, certainly, but the evidence would suggest it's also a lackluster motivator. Between family time, sleep, and srp... Well, I suppose it's obvious where my time has been going.

That said, I'd be very interested in any changes you'd like to contribute up stream. It's a lot easier to spend time on this kind of project when you know it's of real benefit to an actual person, not just some hypothetical user that might someday be interested.

Tom

On Mon, Mar 21, 2016 at 10:11 PM, Doug Johnston notifications@github.com wrote:

I've now gotten things to work with the other SRP implementation using the rfc5054_compat branch. It does appear as if that line is now using a mod_add call, amongst a couple hashing changes.

I would highly encourage merging that with master, as the branch has several bugs and missing features. There was some hesitation in #11 https://github.com/cocagne/pysrp/issues/11 to make it the default for backwards compatibility reasons, but I think looking at the long-term view of this module, using RFC5054 is the right thing to do. Besides, I doubt too many people have been waiting with baited breath since the last pypi release for a point release. :)

Now that I've become intimately familiar with the code, I may have a feature or two to add, including variable salt length, separating the private key generation and adding in other standard Ng options; all of which I needed to implement to use this module with the scheme I'm implementing against.

In the meantime, I would also suggest adding a note to the readthedocs page indicating that it's not RFC5054, as that point is not clear currently.

Thanks for the work.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/cocagne/pysrp/issues/13#issuecomment-199607223