ThalesGroup / crypto11

Implement crypto.Signer and crypto.Decrypter for HSM-protected keys via PKCS#11
MIT License
209 stars 81 forks source link

More coherent slot support #15

Closed optnfast closed 5 years ago

optnfast commented 5 years ago

Currently:

None of this is insurmountable but it does leave a lot of work for the callers of the key generation APIs.

(The situation is better for finding keys, but see #16).

I propose the following changes:

  1. GenerateDSAKeyPair, GenerateECDSAKeyPair, GenerateRSAKeyPair and GenerateSecretKey all get id and label arguments, consistent with the other Generate... functions. This is an incompatible change, but since the functions are almost useless I don't foresee much downstream fallout.
  2. withSession will become WithSession, allowing applications to play nicely with the library's own session management.
  3. WithSession will take a PIN argument rather than using the configured PIN. The present callers of withSession will use the configured PIN.
  4. The logic in Configure to find a slot given a token label/id will be exposed as a public function.
nickrmc83 commented 5 years ago

I'm down with exposing a key id. In general I wonder whether we need more of an interface around keys if we want to set labels as they're mutable. In addition I wonder if we should have a "factory" interface around tokens for generating, deleting and listing keys and managing said token.

dmjones commented 5 years ago

The scope of this library should remain small. Do one thing and do it well.

The goal was to provide PKCS#11 implementations for Go crypto interfaces. This would allow HSMs to be used in the "pluggable" parts of the Go crypto ecosystem (e.g. crypto.Signer).

The Go crypto packages provide no interfaces key generators and thus no way to approach this in a pluggable fashion. I see no benefit in providing a partial solution for key generation, when miekg/pkcs11 offers full access to the HSM.

The potential exception to this is session keys, which cannot currently be generated in separate calls to miekg/pkcs11 and used with our library. However, if people bump into that requirement, we should analyse it separately and consider all possible solutions.

IMO, key generation was scope creep and we will probably remove it in the future (with a suitable deprecation period). Consequently, I don't see a benefit in enhancing the existing support.

dmjones commented 5 years ago

I'm closing this. Comments on the new design are welcome at #36.