pyca/pyopenssl

Add support for Secure Remote Password (SRP)

Closed this issue · 2 comments

munro commented

I noticed some bindings for OpenSSL are missing that I'm trying to use, namely:

  • BN_clear
  • BN_div
  • BN_mul
  • BN_rand
  • CRYPTO_free
  • RAND_seed

For context, I'm trying to fix this file so that it "just works":

https://github.com/ProtonMail/proton-python-client/blob/master/proton/srp/_ctsrp.py

There are a few issues with getting this to work, but after checking out pyOpenSSL (which does just work 💯), it seems like the best path is to swap out their FFI code with pyOpenSSL. I've also tried looking at using srp on PyPI, but that also doesn't run on my local machine.

What this code is trying to do is implement Secure Remote Password (SRP), and so I guess those extra functions from OpenSSL are needed.

My thoughts to getting this to work are:

  • Add those FFI functions to the pyOpenSSL project
  • Extend pyOpenSSL on my end
    • Create a fork of pyOpenSSL and add them--which is really not ideal, because then I'll have to maintain a fork.
    • Maybe some how try to figure out how to dynamically extend the FFI in Python--this would be ideal, but I can't seem to figure out how to
  • Find some other SRP PyPI library that already Just Works™ on my machine
  • If the KDF in SRP is not Good Crypto™, tell ProtonVPN that they need to replace their cryptography with something better (if I'm understanding correctly how SRP works)--but 1Password does seem to be using it, so it must be good ¯\_(ツ)_/¯
  • Refactor the KDF in this code to use functions available in pyOpenSSL
  • Add Secure Remote Password (SRP) to cryptography. This would probably be the most ideal solution, because after searching Github, I see a lot of people copying and pasting the varations of what looks to be similar SRP implementations. Like should this code be using RAND_add instead of RAND_seed?

Anyways, any thoughts are definitely appreciated!

OpenSSL directly supports TLS-SRP with a set of bindings: https://www.openssl.org/docs/man3.0/man3/SSL_CTX_set_srp_password.html

Of course, these are all deprecated as of 3.0 with no replacements, but that's likely because TLS-SRP only exists in TLS 1.2 and below. That makes this a bit of a weird thing to add in the sense that the world is transitioning to TLS 1.3 and eventually SRP users will need to change. I don't actually know whether there are viable PAKE choices for 1.3 handshakes, but adding something that is already effectively legacy is a bit strange.

1password's use of SRP is actually incompatible with the RFCs as well:

However, the hashing and padding scheme in this package is not interoperable with those specs.

I'm interested in how you think it could be added to cryptography. I would expect that just the bare PAKE would not be hugely useful and you'd need hooks to integrate it with a TLS handshake -- is that not the case?

Given that the requested bindings are for a feature that is effectively legacy, and no new comments have been added since Apr 2022, I suggest we close this one. cc @reaperhulk @mhils