Make CKM_AES_GCM usable.

Make CKM_AES_GCM usable.

This requires fixing bugs in the underlying AES GCM code and
adding the PK11_EncryptWithSymKey and PK11_DecryptWithSymKey
functions to provide access to CKM_AES_GCM, which does not
support multi-part operations.

R=agl@chromium.org,rsleevi@chromium.org
BUG=none
TEST=to be covered by Aes128GcmEncrypterTest and
Aes128GcmDecrypterTest in net_unittests

[wtc, 2013-03-26 18:24:46]

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/free...
File mozilla/security/nss/lib/freebl/gcm.c (right):

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/free...
mozilla/security/nss/lib/freebl/gcm.c:233: gcm_reverse(T, tmp_buf, blocksize);

The original code passes |X| as the second argument. Note that
|X| may be incremented on line 225, which is necessary for
the mp_to_unsigned_octets call on line 228 but would be wrong
for the gcm_reverse call.

If both the AAD and plaintext are zero-length, we will
encounter this bug. For other inputs, it seems that we have
a probability of at least 1/256 to encounter this bug (if
the logical first byte of ghash->X is 0).

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/free...
mozilla/security/nss/lib/freebl/gcm.c:582: }

This change is not necessary (because ghash->counterBuf is
initialized to all zeros on line 568, which happens to be
the encoding of a zero length), but I think it makes the
code easier to understand.

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/free...
File mozilla/security/nss/lib/freebl/rijndael.c (right):

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/free...
mozilla/security/nss/lib/freebl/rijndael.c:1224: if (cx == NULL || output ==
NULL || (input == NULL && inputLen != 0)) {

These changes allow an empty input to be represented as
input=NULL inputLen=0.  This allows us to call the default
StringPiece constructor and pass string_piece.data(),
string_piece.size() as input and inputLen.

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/pk11...
File mozilla/security/nss/lib/pk11wrap/pk11mech.c (right):

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/pk11...
mozilla/security/nss/lib/pk11wrap/pk11mech.c:227: case CKM_AES_GCM:

This function (PK11_GetKeyType) is called by PK11_ImportSymKey().
Without adding these cases to the switch statement, the
imported symmetric key will have the wrong type
(CKK_GENERIC_SECRET) and can't be used with CKM_AES_GCM
later.

I suspect we can work around this bug by padding
CKM_AES_ECB to PK11_ImportSymKey.  Since there are other
problems that can't be worked around, I didn't test this
workaround.

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/pk11...
File mozilla/security/nss/lib/pk11wrap/pk11obj.c (right):

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/pk11...
mozilla/security/nss/lib/pk11wrap/pk11obj.c:826:
PK11_EncryptWithSymKey(PK11SymKey *symKey,

The two new functions are very similar to the PK11_SignWithSymKey
function above and the PK11_PrivDecryptPKCS1 and
PK11_PubEncryptPKCS1 functions below. Unfortunately
they use different conventions (SECItems vs.
unsigned char* buffer plus unsigned int length)
to represent inputs and outputs. I chose the latter.

[agl, 2013-03-26 18:35:03]

LGTM.

This probably means that we can't use QUIC (assuming that it's using AES-GCM) on
Linux with old system NSS libraries?

[Ryan Sleevi, 2013-03-26 18:39:14]

lgtm

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/free...
File mozilla/security/nss/lib/freebl/rijndael.c (right):

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/free...
mozilla/security/nss/lib/freebl/rijndael.c:1224: if (cx == NULL || output ==
NULL || (input == NULL && inputLen != 0)) {
On 2013/03/26 18:24:46, wtc wrote:
> 
> These changes allow an empty input to be represented as
> input=NULL inputLen=0.  This allows us to call the default
> StringPiece constructor and pass string_piece.data(),
> string_piece.size() as input and inputLen.

I checked the PKCS#11 2.30 specs, and this seems valid according to Section 11.8
and notes from 11.1/11.2, and nothing from 6.11 conflicts, but it might be worth
double checking with Bob just to make sure there's not any assumptions here
about |input| being a valid pointer even when inputLen == 0 (eg: if there's some
assumption like that spelled out for memcpy in the C spec, where pointers must
be valid even when len == 0)

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/pk11...
File mozilla/security/nss/lib/pk11wrap/pk11obj.c (right):

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/pk11...
mozilla/security/nss/lib/pk11wrap/pk11obj.c:826:
PK11_EncryptWithSymKey(PK11SymKey *symKey,
On 2013/03/26 18:24:46, wtc wrote:
> 
> The two new functions are very similar to the PK11_SignWithSymKey
> function above and the PK11_PrivDecryptPKCS1 and
> PK11_PubEncryptPKCS1 functions below. Unfortunately
> they use different conventions (SECItems vs.
> unsigned char* buffer plus unsigned int length)
> to represent inputs and outputs. I chose the latter.

I don't have strong feelings about this, although i will note that
PK11_SignWithSymKey is the new function we added following Lucky 13, so it seems
weird to add a sym equivalent that follows a different API pattern (the SECItem
vs unsigned char asymmetry that you noted)

[wtc, 2013-03-26 18:41:55]

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/free...
File mozilla/security/nss/lib/freebl/gcm.c (right):

https://codereview.chromium.org/12668022/diff/1/mozilla/security/nss/lib/free...
mozilla/security/nss/lib/freebl/gcm.c:233: gcm_reverse(T, tmp_buf, blocksize);

If this bug in gcm_getX() only affects the zero-length AAD
and plaintext case, then I can look into bypassing the
pk11wrap layer and using the NSS softoken directly.

But I am worried that for other inputs we may still have a
1/256 probability of encountering this bug. If so, we
won't be able to use QUIC's AES GCM encrypter and decrypter
on any Linux computer (because NSS 3.15 hasn't been released
yet).

[Ryan Sleevi, 2013-03-26 18:42:51]

On 2013/03/26 18:35:03, agl wrote:
> LGTM.
> 
> This probably means that we can't use QUIC (assuming that it's using AES-GCM)
on
> Linux with old system NSS libraries?

ChromeOS is on NSS 3.14.x now
Ubuntu Precise is on 3.14.3 . Once Ubuntu drops Lucid support, I think we're
comfortable rolling our minimum NSS version forward.

[commit-bot: I haz the power, 2013-03-26 18:46:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wtc@chromium.org/12668022/1

[commit-bot: I haz the power, 2013-03-26 18:47:06]

Change committed as 190723