137506 - Only internal token's keys can be exported

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, defect, P2)

Description

[Petr Kostka]

Certificates stored on other token than the internal cannot be exported. PSM 
does not even try to export the certificate, it just assumes that it is not 
possible when it sees the cert is not on the internal token. The disabling 
check was added in 1.10 revision of security/manager/ssl/src/nsPKCS12Blob.cpp, 
method nsPKCS12Blob::ExportToFile() as fix for <A 
HREF="http://bugzilla.mozilla.org/show_bug.cgi?id=83675">83675</A>. Is there 
any serious reason why not even try to export the cert? There are also 
software only implementations of PKCS #11 module that can perform the 
operation without any problem. This was also possible in Navigator 4.x.

Comment 1

[Stephane Saux]

cc javi and Bob Relyea.
Question to Bob: can we query a pkcs11 module for its ability to export certs?
The code currently reads:
    if (nssCert->slot && !PK11_IsInternal(nssCert->slot)) {

so the reporter is right that we'll never get a cert out of a non-internal module.

Comment 2

[Julien Pierre]

Stephane,

I investigated this with Bob.

The short answer is that no, we cannot query a token for its ability to export 
keys (I'm referring to keys here because they are the rela problem when 
exporting, not the certs). This is a limitation of PKCS#11. The only reliable 
way to tell is to try it and look for a failure.

Long answer :

There are some checks that could be performed to tell if the token can't export 
the key. One that Bob mentioned is to check for the CKM_3DES_CBC_PAD mechanism 
on the token, and whether it supports CK_WRAP. If not, then the token won't be 
able to extract keys. If yes, it might work, but there is no guarantee until it 
is actually tried.

Another possible check would be on the key object itself, and check for whether 
it can be wrapped or not. But this requires that you already have a key object - 
you can't query the token for that information if you don't have a key object 
already to try.

Both additional checks require new high-level NSS functions to expose the 
functionality. I have opened two RFEs against NSS 4.0 , 142160 and 142161. If 
you want them moved to 3.5, please comment on these bugs.

Because there is no good short-term solution, I would recommend that you remove 
the check for the internal slot (PK11_IsInternal())and let the export fail if 
the token doesn't support it, hopefully by displaying a meaningful error 
message in that case.

Comment 3

[Kim Meyrick]

I was so embarassed i use Mozilla all the time and don't touch IE with a barge
poll ( yes I *WAS* smug about this )

i phoned up a techie about some issue to do with exporting a certificate -
saying i couldn't do it.

they said hmmm well I'm using IE and it works fine

I'm ,like, excuse me while i fall on my sword ! IE beats Mozilla ? how can this be!
Please restore the Universal Equilibrium before BG is installed at God's right
hand! Fix the bug - cheers

Comment 4

[Julien Pierre]

Kim,

IE does not support smartcard PKCS#11 libraries, so if it is able to export
certs from it, it is using completely unrelated code - a CSP library - than the
one used by Mozilla - a PKCS#11 library. It's actually possible that the vendor
alows export in his CSP code but not in the PKCS#11 code, and if that's the
case, there is nothing that Mozilla could do about it.

Also, you need to distinguish between the ability to export the cert and the
key. You can always export the certs from any token, since they are public
information. The Mozilla PSM code, however, does not have a function for
exporting certificates alone, which is a shame. Strict cert-only export would be
done to DER or PKCS#11 format. Instead, the export function in the Mozilla
certificate manager exports both certificates and private keys together to a
PKCS#12 file. For that reason, I have changed the summary of this bug to mention
keys instead of certs.

Only some tokens are capable of exporting keys, mostly software PKCS#11
implementations.

The code in PSM assumes that only NSS' software PKCS#11 implementation can
export keys, and does not try to export keys from other tokens.
A one-line fix in PSM has already been proposed. It is up to the PSM module
owner to decide whether to take it or not.

Comment 5

[Robert Relyea]

OK, so the problem is UI was added to PSM which prevents attempts to load a key from the token since most tokens do not allow it.

I have a patch which does not check if the token can export the keys, but instead whether or not the particular key is exportable using the PKCS #11 CKA_EXTRACTABLE attribute.

This patch has the side effect that if the token claims that the key is extractable, but does not use the same wrapping mechanism that we use, the user will again get a rather gross error. I believe that this case is not the common case (the common case is still the key cannot be extracted and the token tells us so).

There is also a bug in NSS which prevents direct extraction of the key. That will be addressed in a bug.

Comment 6

[Robert Relyea]

Attachment: Only display a problem if the key itself is not extractable

Comment 7

[John G. Myers]

Does it make sense to still test PK11_IsInternal()?  Shouldn't the internal token report the key as extractable?

Comment 8

[Robert Relyea]

It's an optimization. Yes the internal token will report that the key is extractable, but the test requires looking up the key and then fetching the attribute. Since the internal token is a common case, it seems reasonable to skip those steps in that case.

bob

Comment 9

[Kai Engert [:KaiE:]]

Comment on attachment 203142 [details] [diff] [review]
Only display a problem if the key itself is not extractable

Bob, does that patch contain a leak? If FindKey returned a non-null privKey, but !isExtractable(privKey), then privKey is not being destroyed.

Comment 10

[Robert Relyea]

kaie, yes.

we should either free the key before the continue, or change the code to have the following logic

if (privKey) {
   PRBool isExtractable = isExtractable(...);

   SECKEY_FreePrivateKey(privKey);
   if (!isExtractable) {
      .
      .
      .
   }
}

Comment 11

[Kai Engert [:KaiE:]]

Attachment: Patch with Bob's new code added

Bob, thanks for posting this new code.
I have added your new snippet to your patch.

Comment 12

[Kai Engert [:KaiE:]]

Patch checked in.