Closed Bug 549952 Opened 14 years ago Closed 5 months ago

KU_DECIPHER_ONLY is missing

Categories

(NSS :: Libraries, defect, P5)

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: wtc, Unassigned)

Details

RFC 5280 defines the KeyUsage type for the key usage certificate
extension as:
      KeyUsage ::= BIT STRING {
           digitalSignature        (0),
           nonRepudiation          (1), -- recent editions of X.509 have
                                -- renamed this bit to contentCommitment
           keyEncipherment         (2),
           dataEncipherment        (3),
           keyAgreement            (4),
           keyCertSign             (5),
           cRLSign                 (6),
           encipherOnly            (7),
           decipherOnly            (8) }

Except for the comment after nonRepudiation, this definition has
not changed since the original RFC 2459.

NSS defines macros only for the first 8 named bits.  It is missing
a macro for decipherOnly:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/certdb/certt.h&rev=1.52&mark=570-577#569

Bob, Nelson, do you know why?

Alexei, in bug 390888 you added KU_ENCIPHER_ONLY.  Why didn't you also
add KU_DECIPHER_ONLY?
Maybe because public keys are seldom (if ever) used for deciphering?
Here are the definitions of encipherOnly and decipherOnly from RFC 5280:
      The meaning of the encipherOnly bit is undefined in the absence of
      the keyAgreement bit.  When the encipherOnly bit is asserted and
      the keyAgreement bit is also set, the subject public key may be
      used only for enciphering data while performing key agreement.

      The meaning of the decipherOnly bit is undefined in the absence of
      the keyAgreement bit.  When the decipherOnly bit is asserted and
      the keyAgreement bit is also set, the subject public key may be
      used only for deciphering data while performing key agreement.

If we are to define KU_DECIPHER_ONLY, I think it'll need to be defines as

    #define KU_DECIPHER_ONLY              (0x8000)  /* bit 8 */

and we need to do something like this Chrome patch to left-shift the
second byte (if any) of a decoded BIT STRING:
http://codereview.chromium.org/661241/diff/2008/3023#newcode701
if you use
#define KU_DECIPHER_ONLY 0x8000

then there is a conflict with
#define KU_NS_GOVT_APPROVED             (0x8000) /*don't make part of KU_ALL!*/

This appears to be relied upon in certdb.c line 477 (GetKeyUsage).
Wan-Teh, 
Why wouldn't we define KU_DECIPHER_ONLY as 0x100 and leave the bit field 
right justified in a 2-byte big-endian integer field?  
Seems like the obvious thing to do, consistent with what we already do 
elsewhere, no?
Nelson: my suggestion of using 0x8000 for bit 8 will be
clear if you look at how the existing bits are defined:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/certdb/certt.h&rev=1.53&mark=570-577#569

Note that they go from the most significant bit to the
least significant bit.  Following this pattern, the next
bit (bit 8) should be the most significant bit of a new
byte, hence 0x8000.
Sean: thanks for pointing out two bits in 0xFF00 are already
used for KU_NS_GOVT_APPROVED and KU_KEY_AGREEMENT_OR_ENCIPHERMENT.

We can avoid the 0xFF00 byte and use a new byte for
KU_DECIPHER_ONLY: 0x800000.
Here is how Windows CryptoAPI solves the problem.
From WinCrypt.h:

// certenrolld_begin -- CERT_*_KEY_USAGE
// Byte[0]
#define CERT_DIGITAL_SIGNATURE_KEY_USAGE     0x80
#define CERT_NON_REPUDIATION_KEY_USAGE       0x40
#define CERT_KEY_ENCIPHERMENT_KEY_USAGE      0x20
#define CERT_DATA_ENCIPHERMENT_KEY_USAGE     0x10
#define CERT_KEY_AGREEMENT_KEY_USAGE         0x08
#define CERT_KEY_CERT_SIGN_KEY_USAGE         0x04
#define CERT_OFFLINE_CRL_SIGN_KEY_USAGE      0x02
#define CERT_CRL_SIGN_KEY_USAGE              0x02
#define CERT_ENCIPHER_ONLY_KEY_USAGE         0x01
// Byte[1]
#define CERT_DECIPHER_ONLY_KEY_USAGE         0x80
// certenrolld_end

The CertGetIntendedKeyUsage function returns the
key usage in one or two bytes:
http://msdn.microsoft.com/en-us/library/aa376084%28VS.85%29.aspx
Severity: normal → S3
Severity: S3 → S4
Status: NEW → RESOLVED
Closed: 5 months ago
Priority: -- → P5
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.