4 ms·

Heads up to anyone considering using this: the author wrote their own crypto code[1]. I would recommend against using this until that is fixed... I've already

Heads up to anyone considering using this: the author wrote their own crypto code[1]. I would recommend against using this until that is fixed... I've already spotted a few vulnerabilities. [1] https://github.com/FedericoCeratto/owefs/blob/master/pycryptoenc.py https://github.com/FedericoCeratto/owefs/blob/master/pycrypt...

Sir_Cmpwn 11y ago
I don't necessarily disagree, but at some point the buck has to stop, right? How would you implement this any other way? The author didn't implement AES or so on himself, he uses standard library encryption and applies it as appropriate. You should probably report the issues you find to federico.ceratto-at-gmail.com (from Github).
- lotyrin 11y ago
  You could make this a frontend to an existing system like GPG
  - marbu 11y ago
    It looks like OP has that in the roadmap. Unfortunately it also seems that the last work has been done in 2013.
- timmclean 11y ago
  The author should use a library that provides a simple "encryptWithPublicKey" method, so that any choices about RSA key size, AES mode of operation, etc are all taken care of. NaCl[1] would probably be best, since it's written and audited by prominent cryptographers. [1] http://nacl.cr.yp.to/ http://nacl.cr.yp.to/
- xrcltr 11y ago
  There are a tremendous number of other ways this could be implemented. Authenticated encryption? GCM? XTS? Salt the CFB? Guard against interblock attacks? The crypto needs to be completely reworked. This is an asymmetric kek around symmetric encryption, which is done in many other projects. Half-backed crypto such as this is worse than no crypto at all, as it lulls people into believing they are using a valid cryptographic system. But, the project implements (poorly) a subset of what is needed and pushes the rest into application code - but app writers don't know this and wouldn't know what to implement even if they know of the shortcomings. Cryptographers see this all the time. People think they invented a new concept but only implemented a well-known design but did it incompletely and with well-known flaws in the crypto. Then, people defend the system, when it would be far easier to use better primitives.
FiloSottile 11y ago
For a start, it's not authenticating ciphertexts: it uses plain hybrid RSA-AES-CFB
- kentonv 11y ago
  To be fair, most full-disk encryption schemes do not authenticate.
  - xrcltr 11y ago
    This is not disk encryption. This is file encryption.
    - StavrosK 11y ago
      It looks like the file is replaced every write, too, which removes most of the hard use cases. It really seems to me that he could just use PyNaCl to encrypt the files and not have to bother with all the custom crypto. I don't know what the intentions and tradeoffs are, though, so I can't be sure.
    - kentonv 11y ago
      Yeah, good point. You could make similar threat-model arguments as are made about FDE, but that's not really a good excuse when authentication would be technically easy in this case.
eliteraspberrie 11y ago
It uses the PyCrypto package to generate random numbers. The Random class is a Fortuna generator seeded with 8 bytes from the system RNG, the PID, and time: https://github.com/dlitz/pycrypto/blob/master/lib/Crypto/Random/_UserFriendlyRNG.py#L81 https://github.com/dlitz/pycrypto/blob/master/lib/Crypto/Ran... I doubt eight bytes is enough for cryptography... If you need random bytes in Python, use os.urandom: secret = os.urandom(32) https://docs.python.org/2/library/os.html#os.urandom https://docs.python.org/2/library/os.html#os.urandom
- technion 11y ago
  It says something if you're using a package called PyCrypto for RNG and that happens to be an insecure approach. You would think with that name it was the right way to do it.
- michwill 11y ago
  Uhh. I'd think, Crypto.Random is supposed to be secure: """Return the specified number of cryptographically-strong random bytes.""" Pretty sad if it is not the case! Interestingly enough, RNG in pycryptodome which I was using for zerodb is urandom. https://github.com/Legrandin/pycryptodome/blob/master/lib/Crypto/Random/__init__.py https://github.com/Legrandin/pycryptodome/blob/master/lib/Cr... Would be interesting to see similar gotchas about that library (though everybody uses PyCrypto, that makes me feel a little paranoid!)
StavrosK 11y ago
> the author wrote their own crypto code That doesn't immediately mean that the library is useless. > until that is fixed I disagree with the word "fixed", as if it's broken. He probably used the highest-level primitives he could to achieve the requirements. > I've already spotted a few vulnerabilities. It'd probably be more constructive to open an issue detailing the vulnerabilities rather than saying "I've spotted some, use NaCl" and leaving it at that. What makes you so sure that NaCl is even a suitable replacement without knowing all the considerations that went into the project?
federico3 11y ago
Author here. Please be aware that the project has no releases, it's not on PyPI and it's little more than a proof of concept. I have no intention to mislead any user into running it so I'll remove the repository for the time being.

Sir_Cmpwn 11y ago

I don't necessarily disagree, but at some point the buck has to stop, right? How would you implement this any other way? The author didn't implement AES or so on himself, he uses standard library encryption and applies it as appropriate. You should probably report the issues you find to federico.ceratto-at-gmail.com (from Github).

lotyrin 11y ago
You could make this a frontend to an existing system like GPG
- marbu 11y ago
  It looks like OP has that in the roadmap. Unfortunately it also seems that the last work has been done in 2013.
timmclean 11y ago
The author should use a library that provides a simple "encryptWithPublicKey" method, so that any choices about RSA key size, AES mode of operation, etc are all taken care of. NaCl[1] would probably be best, since it's written and audited by prominent cryptographers. [1] http://nacl.cr.yp.to/ http://nacl.cr.yp.to/
xrcltr 11y ago
There are a tremendous number of other ways this could be implemented. Authenticated encryption? GCM? XTS? Salt the CFB? Guard against interblock attacks? The crypto needs to be completely reworked. This is an asymmetric kek around symmetric encryption, which is done in many other projects. Half-backed crypto such as this is worse than no crypto at all, as it lulls people into believing they are using a valid cryptographic system. But, the project implements (poorly) a subset of what is needed and pushes the rest into application code - but app writers don't know this and wouldn't know what to implement even if they know of the shortcomings. Cryptographers see this all the time. People think they invented a new concept but only implemented a well-known design but did it incompletely and with well-known flaws in the crypto. Then, people defend the system, when it would be far easier to use better primitives.

FiloSottile 11y ago

For a start, it's not authenticating ciphertexts: it uses plain hybrid RSA-AES-CFB

kentonv 11y ago
To be fair, most full-disk encryption schemes do not authenticate.
- xrcltr 11y ago
  This is not disk encryption. This is file encryption.
  - StavrosK 11y ago
    It looks like the file is replaced every write, too, which removes most of the hard use cases. It really seems to me that he could just use PyNaCl to encrypt the files and not have to bother with all the custom crypto. I don't know what the intentions and tradeoffs are, though, so I can't be sure.
  - kentonv 11y ago
    Yeah, good point. You could make similar threat-model arguments as are made about FDE, but that's not really a good excuse when authentication would be technically easy in this case.

eliteraspberrie 11y ago

It uses the PyCrypto package to generate random numbers. The Random class is a Fortuna generator seeded with 8 bytes from the system RNG, the PID, and time: https://github.com/dlitz/pycrypto/blob/master/lib/Crypto/Random/_UserFriendlyRNG.py#L81 https://github.com/dlitz/pycrypto/blob/master/lib/Crypto/Ran... I doubt eight bytes is enough for cryptography... If you need random bytes in Python, use os.urandom: secret = os.urandom(32) https://docs.python.org/2/library/os.html#os.urandom https://docs.python.org/2/library/os.html#os.urandom

technion 11y ago
It says something if you're using a package called PyCrypto for RNG and that happens to be an insecure approach. You would think with that name it was the right way to do it.
michwill 11y ago
Uhh. I'd think, Crypto.Random is supposed to be secure: """Return the specified number of cryptographically-strong random bytes.""" Pretty sad if it is not the case! Interestingly enough, RNG in pycryptodome which I was using for zerodb is urandom. https://github.com/Legrandin/pycryptodome/blob/master/lib/Crypto/Random/__init__.py https://github.com/Legrandin/pycryptodome/blob/master/lib/Cr... Would be interesting to see similar gotchas about that library (though everybody uses PyCrypto, that makes me feel a little paranoid!)

StavrosK 11y ago

> the author wrote their own crypto code That doesn't immediately mean that the library is useless. > until that is fixed I disagree with the word "fixed", as if it's broken. He probably used the highest-level primitives he could to achieve the requirements. > I've already spotted a few vulnerabilities. It'd probably be more constructive to open an issue detailing the vulnerabilities rather than saying "I've spotted some, use NaCl" and leaving it at that. What makes you so sure that NaCl is even a suitable replacement without knowing all the considerations that went into the project?

federico3 11y ago

Author here. Please be aware that the project has no releases, it's not on PyPI and it's little more than a proof of concept. I have no intention to mislead any user into running it so I'll remove the repository for the time being.