pycryptoki should promote safer practices

[freedge]

There are some improvements that could be done in the code and the examples to promote safer practices.

One point is the default templates have CKA_EXTRACTABLE=True which looks like a big concern

pycryptoki/pycryptoki/default_templates.py

Line 3601 in ae98be3

CKA_EXTRACTABLE: True,

since partition policies by default allow to wrap and export those (also baffling). The default example is making use of the default template and generates extractable private keys (but the default partition policy prevent to wrap those so I guess it's not that bad)

Another point is that there is no example on how to securely provide the token pin, this is left as an exercise to the reader. The slot pin is provided either in the code in the example, or as a parameter on the command line which is also insecure since it is displayed on the process table which is visible by any locally authenticated user on the system. This remarks also applies for example to the "cmu" tool that allows a password on the command line which is bad.

Another point is that there is no check made on the permissions on private keys or Chrystoki.conf that contains an auth secret. This might not be a pycryptoki issue per se, but I'm bothered, as a user of pycryptoki, that the client just works when the setup is insecure. (if you compare with the ssh client, ssh client will refuse to load a private key that is world readable on the system, see https://github.com/openssh/openssh-portable/blob/1ec0a64c5dc57b8a2053a93b5ef0d02ff8598e5c/authfile.c#L90 You could argue that this is not the responsibility of the client but the benefit for the user is just evident).

A last thing is the default example makes use of c_initialize_ex/c_open_session_ex/login_ex and I think they should all be wrapped with try/finally structures or there is a risk that the first mistake in the code leaves a session opened on the system. The finally would reduce that risk.

---

Although while the password is hidden and replaced with " password: * ", the data to encrypt is shown in exceptions:

pycryptoki.exceptions.LunaCallException:
        Function: c_encrypt
        Error: CKR_DATA_LEN_RANGE
        Error Code: 0x21
        Arguments:
                h_session: 1
                h_key: 2
                data: b'1234567812345678123456781234567'
        mechanism:
                NullMech(mech_type: CKM_AES_ECB (0x00001081))
                output_buffer: None

[astraw38]

Another point is that there is no example on how to securely provide the token pin, this is left as an exercise to the reader. The slot pin is provided either in the code in the example, or as a parameter on the command line which is also insecure since it is displayed on the process table which is visible by any locally authenticated user on the system. This remarks also applies for example to the "cmu" tool that allows a password on the command line which is bad.

Correct, left to the reader. There's innumerable ways to get passwords, or even not use passwords at all (see: Luna's salogin or lunacm's set appid; or PED-authenticated devices).

A last thing is the default example makes use of c_initialize_ex/c_open_session_ex/login_ex and I think they should all be wrapped with try/finally structures or there is a risk that the first mistake in the code leaves a session opened on the system. The finally would reduce that risk.

Fair point - we do have helpers that implement __enter__ and __exit__ to use with with constructs.

And thanks for the exception report! I'll see about getting that updated.

[freedge]

I'm bothered because if you play a bit with pycryptoki (or the other SDKs, of course it's not an issue specific to Luna or pycryptoki) without giving much care you might have the impression that your keys are stored securely on an HSM while there are probably exportable/derivable and all the pins and secrets have not been secured along the way.

I realize it's unfair to blame the library for mistakes you could also attribute to sysadmins or developers but, considering the HSM will seal secrets you really really want to keep secret, I think it's also the role of the library, developed by the people who know best, to enforce those best practices whenever possible (and not come with "traps"). Not having world readable files holding secrets seems that it would make the overall ecosystem more secure and it's a check that could be made by the library so I'd love to see it there.

I find along the way that derivable AES keys are still vulnerable to CVE-2015-5464 on Utimaco HSM so I'd hope to see the default template update to see non modifiable/non derivable/non exportable keys as much as possible.

Thanks for your feedback and thanks for this awesome module!