search

cocagne / pysrp

Python implementation of the Secure Remote Password protocol (SRP)
MIT License
113 stars 42 forks source link

Add option to specify length of generated salt #27

Closed dragomirecky closed 6 years ago

dragomirecky commented 6 years ago

Hello, is there any problem with this pull request? Sorry for leaving out the description of this pull request empty, I just thought that the name is self explanatory :).

The changes should be backward compatible and having the option to specify length of the salt might be useful in some cases. For example, I need it in my current project to establish connection via HAP protocol (HomeKit).

cocagne commented 6 years ago

Hey Alan. Sorry for the delayed reply. My primary concern with this pull request is whether backwards compatibility has been tested. Did you verify for sure that existing pysrp applications are compatible with your changes by default? The key here is to ensure that a pip upgrade of a pysrp server/client continues to function as before.

Your changes are very welcome. Previously all similar work has been done on a parallel branch because they make breaking changes. 5054 compatibility is definitely the right way to go moving forward but i don't want to hurt existing users.

Tom

On 17 Jan 2018 6:14 AM, "Alan Dragomirecký" notifications@github.com wrote:

Hello, is there any problem with this pull request? Sorry for leaving out the description of this pull request empty, I just thought that the name is self explanatory :).

The changes should be backward compatible and having the option to specify length of the salt might be useful in some cases. For example, I need it in my current project to establish connection via HAP protocol (HomeKit).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/cocagne/pysrp/pull/27#issuecomment-358286739, or mute the thread https://github.com/notifications/unsubscribe-auth/ABrLfF-JZNzt9iB3iwxqB35OhWtnGj6Gks5tLeQcgaJpZM4PPkbM .

dragomirecky commented 6 years ago

Hi Tom, I believe you wanted to post your reply to my second pull request regarding rfc5054 compatibility. To briefly answer here: I understand your concerns about backward compatibility – please have a look how I changed the pull request. Now, the rfc5054 compatibility is fully optional (tests are passing).

This pull request does not affect existing users in any way.

Alan

cocagne commented 6 years ago

Thanks for pinging me on this again Alan. I completely missed that your updates. Merged and published to pypi. Thanks for the contribution! That's a feature I've received requests for for years :)

On Sun, Feb 11, 2018 at 3:54 AM, Alan Dragomirecký <notifications@github.com

wrote:

Hi Tom, I believe you wanted to post your reply to my second pull request <#m_6471184473819690417_26> regarding rfc5054 compatibility. To briefly answer here: I understand your concerns about backward compatibility – please have a look how I changed the pull request. Now, the rfc5054 compatibility is fully optional (tests are passing).

This pull request does not affect existing users in any way.

Alan

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/cocagne/pysrp/pull/27#issuecomment-364738589, or mute the thread https://github.com/notifications/unsubscribe-auth/ABrLfOe5HzcEKBO0XI1Qec7jGW68cNuzks5tTrjggaJpZM4PPkbM .