430198 - certutil capability: generate CSR from orphan private key

Status: RESOLVED FIXED

(NSS :: Tools, enhancement, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

Even though bug 341371 was fixed in NSS 3.12, certutil still lacks a 
way to generate a CSR for an orphan private key.  An orphan private key is 
an existing private key in the key DB for which there is NO cert with the
corresponding public key in the cert DB.

When you want to generate a CSR for an existing private key in your key DB,
and you also have a certificate with the corresponding public key in your 
cert DB, you can specify the private key indirectly by giving the nickname 
of the cert with the corresponding public key.  NSS finds the cert by the 
nickname, and then finds the private key corresponding to that cert.  
That is the technique used to generate a CSR for an existing private key 
when you have a corresponding cert, which was implemented in bug 341371.

But Private keys do not have nicknames, so one cannot refer to a private key directly by nickname.  If there is no cert with a corresponding public key 
in the cert DB, you cannot reference the existing public key by a nickname.
A method is needed to refer to a private key on the command line that works
even for orphan private keys.  

All private keys accessible to NSS have PKCS#11 Key IDs (CKA_ID). These are 
binary numbers that may be expressed in hex, and may be up to 20 bytes long.
certutil -K will print out the CKA_IDs for the private keys in the users key 
DB.  (that was bug 291384.)

Bug 291383 proposes to allow users to enter private key CKA_IDs in hex as 
command line arguments, for the purpose of deleting those private keys.

This bug now proposes to use the same technique, entering the CKA_ID in 
hex as a command line option argument, for the purpose of specifying a 
private key for the generation of a CSR.  This will enable the generation
of multiple CSRs for the same private key, even while the private key is
still an orphan (before the first correpsonding cert is imported to the 
cert DB.

Related bugs:

Bug  67890: create self-signed cert with existing key that signed CSR
Bug 291383: certutil cannot delete orphan private keys
Bug 291384: certutil -K behavior doesn't match usage
Bug 341371: certutil lacks a way to request a certificate with an existing key

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Bug 67890 is very much like this bug, but it intends to skip the generation
of the CSR, and instead issue a self-signed cert using an existing private
key.  It proposes to add yet another way to specify the private key to be 
used, namely by supplying a CSR that was previously signed with that same
private key.  That is also another way to do it. 

So, there should be at least two ways to specify orphan keys:
- by CKA_ID inhex
- by naming a file that contains a CSR signed with the same key.

Comment 2

[Julien Pierre]

Nelson,

We probably don't want to require a CSR file to select the orphan private key. At least in the case of deletion, it's most likely that the CSR is lost.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

True, but having just generated a CSR, we ought to be able to use it to 
specify the private key for the generation of a second CSR.

Comment 4

[Julien Pierre]

Nelson,

The private key might be orphan in a token for other reasons than having just generated an initial CSR. Specifying by CKA_ID is a more generic way than passing a CSR. I would consider the second way (passing in existing CSR file) to be optional for resolving this bug. 

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Using a CSR is actually the easier of the two methods to implement, and 
IINM, there is already a patch that implements the find-key-by-CSR part 
attached to bug 67890.  

I'm sure that the find-key-by-CSR and find-key-by-CKA_ID methods, once 
developed for one purpose (such as _deleting_ orphan keys), will also 
be immediately useful for the other purposes (such generating CSRs, 
issuing self-signed certs).  

Comment 6

[John Dennis]

Any progress on this bug? I like the idea of using CKA_ID to identify the key in the database (but see below), it would be use for many purposes.

What's prompted our interest at the moment is we would like to be able to have a crypto agnostic utility (e.g. either OpenSSL or NSS) which follows the same basic sequence of steps, yet using the specified crypto provider. But currently when using the command line tools (e.g. certutil) there is no way to create a keypair in one step and then in a subsequent step generate a CSR from the key pair.

It would also be useful for things like reissuing certs when the original CSR is lost.

Is there any reason a nickname (or other user supplied identifier) can't be used to label (identify) the key pair generated with -G? And then use that identifier to specify the key used in the CSR? That would be ideal because then the user is in control of identifying the key generated with -G, otherwise you have to generate the key and then query the key database and *assume* the CKA_ID of the last listed key is the one you generated. Not always a good assumption and subject to races.

Comment 7

[Fraser Tweedale]

Attachment: nss-ftweedal-0000-SECU_HexString2SECItem-work-with-arena-NULL.patch

Comment 8

[Fraser Tweedale]

Attachment: nss-ftweedal-0001-certutil-R-interpret-k-argument-as-CKA_ID.patch

Comment 9

[Kai Engert (:KaiE:)]

Fraser, thanks for the patches. Ideally we should have a test. Could you suggest a series of certutil commands, with expected result, to test this functionality?

Comment 10

[Fraser Tweedale]

Kai, no worries.  The test procedure would be:

1. run `certutil -G ...` generate a new key
2. run `certutil -K ...` and read the CKA_ID from the output
3. run `certutil -R -k <CKA_ID> ...` to create a certificate request for the key

This is most easily performed in a clean NSSDB, otherwise you first need to
run `certutil -K` to read the existing CKA_IDs, then after key generation
run `certutil -K` and find the *new* key.

If it helps, here are some Python snippets for how I will use this feature.
First some methods for listing keys and generating a key:

    def list_private_keys(self):
        """
        Return list of hex-encoded private key CKA_IDs in the token.

        """
        cmd = [
            'certutil',
            '-d', self.directory,
            '-f', self.password_file,
            '-K',
        ]
        out = subprocess.check_output(cmd)

        # output contains list that looks like:
        #   < 0> rsa      b995381610fb58e8b45d3c2401dfd30d6efdd595 (orphan)
        #   < 1> rsa      dcd6cbc1226ede02a961488553b01639ff981cdd someNickame
        #
        # The hex string is the hex-encoded CKA_ID
        return re.findall(r'^<\s*\d+>\s+\w+\s+(\w+)', out, re.MULTILINE)

    def generate_key(
            self,
            key_type=None, key_size=None, curve=None,
            noise_file=None):
        """
        Generate a key of the given type and size.
        Returns the CKA_ID of the generated key.

        ``noise_file``
          Path to a noise file, or ``None`` to automatically
          generate a noise file.

        """
        ids_pre = set(self.list_private_keys())

        cmd = [
            'certutil',
            '-d', self.directory,
            '-f', self.password_file,
            '-G',
        ]
        if self.token:
            cmd.extend(['-h', self.token])
        if key_type:
            cmd.extend(['-k', key_type])
        if key_size:
            cmd.extend(['-g', str(key_size)])
        if curve:
            cmd.extend(['-q', curve])

        temp_noise_file = noise_file is None
        if temp_noise_file:
            fd, noise_file = tempfile.mkstemp()
            os.close(fd)
            size = key_size if key_size else 2048
            self.create_noise(noise_file=noise_file, size=size)
        cmd.extend(['-z', noise_file])

        try:
            subprocess.check_call(cmd)
        finally:
            if temp_noise_file:
                os.unlink(noise_file)

        ids_post = set(self.list_private_keys())
        return list(ids_post - ids_pre)[0]


And a fragment for creating the cert request:

            cmd = [
                'certutil',
                '-R',
                '-d', self.directory
            ]

            if self.token:
                cmd.extend(['-h', self.token])

            cmd.extend([
                '-f', self.password_file,
                '-s', subject_dn,
                '-o', binary_request_file,
                '-k', cka_id,
            ])

Comment 11

[Kai Engert (:KaiE:)]

Fraser, thanks again for your patch. I think the general approach is fine, but I'd like to ask for a few changes:

We should avoid changing the behavior of SECU_HexString2SECItem with null arenas, to avoid potential side affects for other users of the function. Therefore I think we should drop your small patch that changes it. You simply need a local "PLArenaPool *arena" variable, allocate an arena using "arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);", and error if it fails. When you're done, just call PORT_FreeArena(arena, PR_FALSE) and remove the call to SECITEM_FreeItem(&keyidItem, PR_FALSE);

Please check for a null/error SECU_HexString2SECItem.

All variable declarations must be at the start of blocks, for classic C compatibility.

Please use brackets {} for all statements, even one-line.

Don't re-declare rv when calling PK11_Authenticate, you're shadowing the variable in the other function (compiler warning).

Please update the help output of -R to mention that a key ID produced by -K is allowed.

Comment 12

[Kai Engert (:KaiE:)]

One more thing, while testing your patch, I ran into bug 1460284.

In certutil_main() after SECU_RegisterDynamicOids(), it might help to call SSL_OptionSetDefault(-1, 0) to work around that bug.

Comment 13

[Kai Engert (:KaiE:)]

Attachment: csr-test.patch

I've implemented a test for this functionality in the NSS test suite.

Comment 14

[Fraser Tweedale]

Attachment: csr.patch

Updated patch.  Obsoletes both previous patches 0000 and 0001 (BZ wouldn't let me mark those as obsoleted for some reason...)

Comment 15

[Kai Engert (:KaiE:)]

Comment on attachment 8976045 [details] [diff] [review]
csr.patch

r=kaie

I'd like to delay landing this patch just a little bit, until we have the test reviewed, too.

Comment 16

[Kai Engert (:KaiE:)]

Comment on attachment 8975083 [details] [diff] [review]
csr-test.patch

Fraser, any thoughts about this test?

Bob, could you please review?

Comment 17

[Robert Relyea]

Comment on attachment 8975083 [details] [diff] [review]
csr-test.patch

Review of attachment 8975083 [details] [diff] [review]:
-----------------------------------------------------------------

r+ rrelyea

Other then the issue with the certutil -K format which you documented really well, a pretty straightforward patch.

Comment 18

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/projects/nss/rev/54b4da2b8991
https://hg.mozilla.org/projects/nss/rev/bf3c9e2318aa

Comment 19

[Kai Engert (:KaiE:)]

This code fails to build on Windows, because it doesn't have strncasecmp. The obvious fix is to use the portable equivalent provided by NSPR, PL_strncasecmp.

https://hg.mozilla.org/projects/nss/rev/2f3ace03cbf89e9300c2dd09da2317a82e382791

Comment 20

[Fraser Tweedale]

Thanks Kai and Bob for your help getting this fix merged.