Replace PBKDF2 class by python stdlib implementation

[d0]

hashlib.pbkdf2_hmac has been backported to Python 2.7 (see PEP 466). In
order to avoid reinventing the wheel and to include less third party
code we should consider switching to the stdlib implementation.

[frankmorgner]

did you want to remove the local implementation of b64encode() as well?

[d0]

The b64encode() function was part of the PBKDF2 implementation we included from a third party as well, so I replaced it by using the equivalent stdlib function. It's not used anywhere else in the code.
Right now the only thing left from the borrowed PBKDF2 implementation are the crypt() and _makesalt() functions. Maybe I will do some more changes to these functions as well but I will have to think a bit more about it in order to avoid braking stuff.

[frankmorgner]

Thanks! I guess that some day we should move to python3 eventually...

Could you maybe have a look at the problem regarding pycrypto https://travis-ci.org/frankmorgner/vsmartcard/jobs/113253979#L3229? It's there since some time due to a change of the travis-ci environment. I was unsuccessful in resolving this so far, https://github.com/frankmorgner/vsmartcard/tree/test...

[d0]

I have opened up issues for fixing Travis CI (#70), switching to Python 3 (#71) and formating the code according to PEP 8 (#72) so we can track progress. I'll look into it but I'm not sure when I can find the time to work on these issues.

[d0]

BTW, there is a chance that this change breaks the deserialization of cards saved with CardGenerator.saveCard(). I didn't try to load a card serialized with the old implementation and the untit tests don't cover this scenario. I don't think anyone is actually using this feature so it probably is no big deal.
Furthermore, there are currently multiple ways to load a Card: CardGenerator.loadCard() and CardGenerator.readDatagroups(). They both cater to slightly different use cases. Do you want to keep both or switch to a unified way of serialization/deserialization?

[frankmorgner]

Well, maybe rewriting the python code from scratch is the fastest and easiest way of solving the python issues... Unfortunately I won't have time for that in the near future.

I know that CardGenerator.readDatagroups() is actually being used; serialization is most likely not used by anyone (though I think with the patch provided we don't loose backwards compatibility). I'd like to have a unified human readable way of initializing a card. However, I think that's a hard problem to do in a generic way (i.e. not for a specific card).

[d0]

Unfortunately, the change does indeed break backwards compatibility, see the new tests introduced in commits c8f2776 and 82e9248. I couldn't yet figure out what breaks exactly and how to fix it. Before I spend more time debugging I wanted to ask: do we care?

As I see it there are several options (in no order of preference):

1. Break backwards compatibility and keep the new implementation
2. Try to fix the new implementation in order to achieve backwards compatibility
3. Get rid of the serialization code altogether since it's not used by anyone anyway
4. Revert to 36970a2 and don't touch the code

I'd vote for options 3. or 1.

[d0]

On 03.03.2016 17:45, Frank Morgner wrote:

Well, maybe rewriting the python code from scratch is the fastest and
easiest way of solving the python issues... Unfortunately I won't have
time for that in the near future.

I concur that rewriting the python code from scratch is currently
unfeasible. Reformatting the code to PEP8 and achieving compatibility
with Python3 on the other hand should be feasible since there are good
tools which can be used to automate a lot of the work (even though we
don't have enough test coverage for me to feel really comfortable with
making major changes to the codebase). The biggest problem I see is the
OpenPACE swig wrapper which is currently not compatible with Python3.
We should discuss this in #71 though.

[frankmorgner]

No problem, we continue with the change as is.

[psytester]

This commit breaks my environment :-(
I was happy to easily use Ubuntu 14.04 but now I get Trouble again.
My Ubuntu is up-to-date but has Python 2.7.6 only and the pbkdf2_hmac was added with 2.7.8 via Issue #21304

I suggest to keep backwards compatibility for a longer time than on your systems!

[frankmorgner]

quoted from the Python 2.7.8 Release notes:

Python 2.7.8 was released on July 1, 2014. This release includes regression and security fixes over 2.7.7 including:

• The openssl version bundled in the Windows installer has been updated.
• A regression in the mimetypes module on Windows has been fixed.
• A possible overflow in the buffer type has been fixed.
• A bug in the CGIHTTPServer module which allows arbitrary execution of code in the server root has been patched.
• A regression in the handling of UNC paths in os.path.join has been fixed.

I'd advice you to upgrade your system to the latest version (anyway). Here is a description of how to install upgrade Python without breaking things https://renoirboulanger.com/blog/2015/04/upgrade-python-2-7-9-ubuntu-14-04-lts-making-deb-package/

[d0]

I'm pretty sure Ubuntu LTS backports the security fixes so asking all users of a pretty common and supported Linux distribution feels like the wrong way to go. We could do one of the following things:

1. Revert the commit and keep on using the old implementation.
2. Remove serialisation support since it's not used anyway and the code quality is questionable.
3. Move to a separate PBKDF2 (or other strong KDF) library managed by e.g. PIP

On 21 March 2016 16:41:42 CET, Frank Morgner [email protected] wrote:

quoted from the Python 2.7.8 Release notes:

Python 2.7.8 was released on July 1, 2014. This release includes
regression and security fixes over 2.7.7 including:

• The openssl version bundled in the Windows installer has been
  updated.
• A regression in the mimetypes module on Windows has been fixed.
• A possible overflow in the buffer type has been fixed.
• A bug in the CGIHTTPServer module which allows arbitrary execution
  of code in the server root has been patched.
• A regression in the handling of UNC paths in os.path.join has been
  fixed.

I'd advice you to upgrade your system to the latest version (anyway).
Here is a description of how to install upgrade Python without breaking
things
https://renoirboulanger.com/blog/2015/04/upgrade-python-2-7-9-ubuntu-14-04-lts-making-deb-package/

---

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#69 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

[psytester]

Frank, I know that blog entry, but you know I can not use every "fancy newest package" ;-)
My base is a long term distribution in this case 14.04 LTS and updates have to come via official package manager.
As long as Phyton 2.7.8 is not part of Ubuntu 14.04 I have to stop using newer vsmartcard Releases.

Why not using both? Can you separate the usage via such Determination?

try:
    from hashlib import pbkdf2_hmac
except ImportError:
    .......

If nothing helps, I have to wait some days until Ubuntu 16.04 LTS will be released in April and it contains Phyton 2.7.11

[frankmorgner]

Could you check whether our test suite works for you with 0415f32 applied? It should be executed with something like this:

cd virtualsmartcard/src/vpicc
python -m unittest discover -s virtualsmartcard.tests -p *_test.py

[d0]

IMHO that is the least desirable option with regard to simplicity, maintainability and code quality. I would be happy to nuke serialization support or migrate to a different KDF (like bcrypt or scrypt both of which have implementations available in PIP). However, I'm currently travelling and will only be able to do start doing that in 2.5 weeks.

On 24 March 2016 10:43:14 CET, Frank Morgner [email protected] wrote:

Could you check whether our test suite works for you with
0415f32
applied? It should be executed with something like this:

cd virtualsmartcard/src/vpicc
python -m unittest discover -s virtualsmartcard.tests -p *_test.py

---

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#69 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

[frankmorgner]

Agreed @d0 , we'll let this be as is for now...

[psytester]

Hello Frank, your current branch pbkdf2 with mentioned patch

try:  
    from hashlib import pbkdf2_hmac  
except ImportError:  

works fine at least for me.

[d0]

One of the problems with the pbkdf2 branch is that the hashlib implemenation and the fallback implementation are not interoperable. So if a user creates a serialized card with one implementation it cannot be loaded with the other.

I have prepared two branches:

1. https://github.com/d0/vsmartcard/tree/no_serialization (41bd7ab): Completely removes serialization support. Since the implementation of the serialization functionality is ugly, potentially insecure (unserializing untrusted python objects results in code execution, using the same key for encryption and authentication is not great) and not used by anybody, I prefer this solution.
2. https://github.com/d0/vsmartcard/tree/bcrypt_serialization (8a0b227): Switches the key derivation function from PBKDF2 to bcrypt. Users need to install the bcrypt library via PIP or from their distribution (python-bcrypt in Ubuntu) in order to use this solution.

[d0]

@frankmorgner Any feedback on this?

[frankmorgner]

I agree, we can remove serialization. Sorry for the late response. Will you make a PR?

[d0]

I created a PR to remove serialization (#78)