Last Comment Bug 876352 - certutil: (a) Warn if importing PEM file with private key (b) fail if user attempts to import cert with requested "u" trust
: certutil: (a) Warn if importing PEM file with private key (b) fail if user at...
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Tools (show other bugs)
: 3.14.3
: x86_64 Linux
: -- normal (vote)
: 3.15.1
Assigned To: Kai Engert (:kaie) (on vacation)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-05-27 00:44 PDT by Kai Engert (:kaie) (on vacation)
Modified: 2013-06-11 12:15 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1 (20.87 KB, patch)
2013-05-27 01:22 PDT, Kai Engert (:kaie) (on vacation)
no flags Details | Diff | Splinter Review
Patch v2 (20.90 KB, patch)
2013-05-27 01:29 PDT, Kai Engert (:kaie) (on vacation)
rrelyea: review+
Details | Diff | Splinter Review

Description Kai Engert (:kaie) (on vacation) 2013-05-27 00:44:37 PDT
certutil currently cannot import private keys from a PEM file, it silently skips over private keys.

We could improve usability by using two minor changes, which shouldn't cause any side effects:

(a) If a user attempts to import a certificate in the ASCII format,
    search if the file contains the phrase "PRIVATE KEY".
    If it does, print a warning message
      "certutil cannot import private keys in this format, please use pk12util"
    but proceed as usual (import cert, skip key).

(b) If a user explicitly requests to set the certificate to include "u" for
    any usage, then FAIL with an error like:
      "Usage u requires a private key, use pk12util to import a certificate with a private key."
Comment 1 Kai Engert (:kaie) (on vacation) 2013-05-27 01:00:11 PDT
I'm adjusting my (b) suggestion.

Even in that scenario, I want to print a warning only, to change the existing behaviour as little as possible.
Comment 2 Kai Engert (:kaie) (on vacation) 2013-05-27 01:22:07 PDT
Created attachment 754375 [details] [diff] [review]
Patch v1

This patch is slightly larger than expected, because we're operating on a file handle, which could be standard input, so we cannot read the file twice. This means, we must adjust the function that processes the input stream.

Luckily that function is in the tool code, so the function can easily be changed, without breaking any API/ABI promises of the core NSS library.
Comment 3 Kai Engert (:kaie) (on vacation) 2013-05-27 01:22:40 PDT
Output with this change:

$ certutil -A -d /tmp/bla/ -n test -a -i test.cert -t u,u,u
Warning: ignoring private key. Consider to use pk12util.
Notice: Trust flag u is set automatically if the private key is present.
Comment 4 Kai Engert (:kaie) (on vacation) 2013-05-27 01:29:00 PDT
Created attachment 754376 [details] [diff] [review]
Patch v2
Comment 5 Robert Relyea 2013-06-07 16:20:05 PDT
Comment on attachment 754376 [details] [diff] [review]
Patch v2

r+ Of the changes, the warning on trying to set the 'u' bits are probably the most useful.

bob
Comment 6 Kai Engert (:kaie) (on vacation) 2013-06-11 12:15:49 PDT
https://hg.mozilla.org/projects/nss/rev/edcb5af30559

Note You need to log in before you can comment on or make changes to this bug.