Oh-well, I suppose that we are all humans. ;)
Let me take a closer look on this and see if there are other ways to relax this constraint than adding env var which should be the very last resort in my opinion.
BTW, is there a bug filed for this?

Thanks for the feedback,
Valerie

On 2/9/2018 12:55 AM, Tomas Gustavsson wrote:
Hi,

Thanks for the answer. (sorry I was out with the flu for a week)

I am not too keen to add an env var/system property to accommodate this
kind of PKCS11 library bugs since this should be rare I hope.
Valerie
Unfortunately I don't see it as rare and the impact is huge due to the
slow turnaround of HSM firmware. Due to FIPS certification and whatnot
HSM vendors do not fix simple things like this sometimes in years. This
puts customers to a complete halt in the meantime. These are heavily
certified environments where re-certification alone takes at least 6 months.

Having worked with all major HSM vendors for many years I know that
PKCS11 library bugs are not rare at all. If we'd be using PKCS#11
natively, you can hack around by ignoring bad values, but when using
SunPKCS#11 you are stuck with Java's rules unless you patch SunPKCS11
which is not for everyone.

Due to the strictness of using SunPKCS11 compared to native PKCS#11,
some more flexibility in configuration would help a lot.

For many years SunPKCS11 have worked great. But also the HSM world is
moving faster than they did 5 years ago, and unfortunately this means
that we're seeing a huge rise in PKCS#11 issues in the last year,
requiring quite a lot of hacking in SunPKCS11 to workaround. In theory
it should not be needed, but in practice it is. Faster evolution = more
bugs.

I just showed two real world use cases that you really need to be able
to work around. And these will not be the last. PKCS#11 is a complex
standard and implementors will rarely get it exactly right.

Increased flexibility and more control around PKCS#11 will be needed in
the future imho, or users will be forced to move to other solutions like
JackNJI11. We'd much rather use SunPKCS11 but customers (end users)
can't get stuck between Java and HSM vendors in a fight who does PKCS#11
right or wrong.

Cheers,
Tomas

On 2018-02-01 01:07, Valerie Peng wrote:
Thanks for the feedback. I suppose we can ignore values which obviously
don't make sense such as 0 or max being less than min key size.
However, if the underlying PKCS11 library vendors forgot to update the
max value as in your comment#2, supposedly they should fix it.
I am not too keen to add an env var/system property to accommodate this
kind of PKCS11 library bugs since this should be rare I hope.
Valerie

On 1/30/2018 12:22 AM, Tomas Gustavsson wrote:
Hi,

At some revision in the PKCS#11 provider there was introduced checking
of C_GetMechanismInfo min and max sizes.

This has turned out to be a bit fragile. Let me give two real world
examples:

1. Amazon Cloud HSM report minSize and maxSize for EC keys to 0. The
Java PKCS#11 provider will happily take 0 as maxSize and refuse to
generate any EC keys at all. Needless to say, without the Java check it
would be no problem.

131: C_GetMechanismInfo
2018-01-30 07:52:20.740
[in] slotID = 0x1
   CKM_EC_KEY_PAIR_GEN
[out] pInfo:
CKM_EC_KEY_PAIR_GEN           : min:0 max:0 flags:0x10001 ( Hardware
KeyPair )
Returned:  0 CKR_OK

(we are reporting this to Amazon as well)

2. Thales HSMs (some?) report maxSize for RSA_PKCS key generation as
4096, but will happily generate 8192 bit keys. I.e. the reported maxSize
is not true.
We have customers who used to generate 8192 bit RSA keys, but after a
Java update can not do so anymore, because Java compares against this
value.


* Suggestions:

1. In the constructor of P11KeyPairGenerator where minKeyLen and
maxKeyLen are calculated, never allow maxKeyLen to be less than
minKeyLen.

I.e. change the part:
          // auto-adjust default keysize in case it's out-of-range
          if ((minKeyLen != -1) && (keySize < minKeyLen)) {
              keySize = minKeyLen;
          }
          if ((maxKeyLen != -1) && (keySize > maxKeyLen)) {
              keySize = maxKeyLen;
          }

To include something like:
          // auto-adjust default keysize in case it's out-of-range
          if ((minKeyLen != -1) && (keySize < minKeyLen)) {
              keySize = minKeyLen;
          }
          if ((maxKeyLen != -1) && (maxKeyLen < minKeyLen)) {
              maxKeyLen = minKeyLen;
          }
          if ((maxKeyLen != -1) && (keySize > maxKeyLen)) {
              keySize = maxKeyLen;
          }

2. Allow to ignore checking of maxKeyLen by some means, i.e. allow to
ignore checking against C_GetMechanismInfo if you know that the HSM does
not provide sane values. I.e. an environment variable for example
reverting back to the old behavior when these were ignored.

Regards,
Tomas Gustavsson


Reply via email to