1373824 - Bug 1308027 (change 2163) breaks HMAC-SHA1 key imports

Status: RESOLVED FIXED

(JSS Graveyard :: Library, defect)

Description

[mmcclain]

Attachment: HmacTest.java

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.104 Safari/537.36

Steps to reproduce:

HmacTest code shows importing an HMAC-SHA1 key via an AES encrypt and AES unwrap call.  It works prior to change 2163 (or if I revert PK11MessageDigest.c).


Actual results:

After changeset 2163 (particularly change to PK11MessageDigest.c), it fails.
The failure is caused because PK11_CreateContextBySymKey(mech, CKA_SIGN,
newKey, &param) returns null.

[root@VTPFWSMUXG ~]# java -cp $CLASS_PATH:. HmacTest
mac: javax.crypto.Mac@126097b, Mozilla-JSS version 4.4
key: org.mozilla.jss.crypto.SecretKeyFacade@1468bd9
Exception in thread "main" java.security.InvalidKeyException: DigestException: Unable to initialize digest context
	at org.mozilla.jss.provider.javax.crypto.JSSMacSpi.engineInit(JSSMacSpi.java:53)
	at org.mozilla.jss.provider.javax.crypto.JSSMacSpi$HmacSHA1.engineInit(JSSMacSpi.java:93)
	at javax.crypto.Mac.init(Mac.java:413)
	at HmacTest.main(HmacTest.java:37)



Expected results:

Prior to changeset 2163, importing an HMAC-SHA1 key via encryption and then
unwrap worked.

[root@VTPFWSMUXG ~]# java -cp $CLASS_PATH:. HmacTest
mac: javax.crypto.Mac@30f1c0, Mozilla-JSS version 4.4
key: org.mozilla.jss.crypto.SecretKeyFacade@c77c2e
Done

Comment 1

[mmcclain]

Attachment: hg-revert-2163-PK11MessageDigest.patch

Patch that just reverts the change to PK11MessageDigest.c.

I don't know why the change was made, so I don't know if this
is the right thing to do.

Comment 2

[Matthew Harmsen]

(In reply to mmcclain from comment #1)
> Created attachment 8878679 [details] [diff] [review]
> hg-revert-2163-PK11MessageDigest.patch
> 
> Patch that just reverts the change to PK11MessageDigest.c.
> 
> I don't know why the change was made, so I don't know if this
> is the right thing to do.

mmcclain,

These changes cannot be reverted; the test will have to be fixed.

jmagne will provide a more detailed reason as to why these changes cannot be reverted.

-- Matt

Comment 3

[Matthew Harmsen]

Hello:

Thanks for the report. We can't revert the entire patch, because it is needed to provide some important sym key operations to your server code.

Having said that, I"m all for just figuring out what is wrong and fixing the problem that is causing issues with existing code.

Oh I see, the proposal is to revert the portion of the patch having to do with hmac. I will take a look at this and figure out why we made the change in the first place.. If you guys have a piece of test code that triggers the problem I would be happy to sort it out.

thanks,
jack

Comment 4

[jmagne]

Attachment: Proposed patch to solve issue and make the provided test work

Things done in this patch:

1. Integrated the provided test program into the jss test suite. The test was modified slightly to conform to JSS. It also tries a sample HMAC operation with the newly unwrapped key.

2. The test program also needed to be revised to import a key smaller than the provided 1024. NSS only allows sym keys to be so big.

3. Some internal code in JSS needed to be changed to get the test to work. There is a routine that maps a keyType to an algorithm object. The code as is, did not support SHA1_HMAC. Consequently, I was surprised at the fact that this test used to work as is.

4.Fixed the actual piece of code causing the problem. Originally we had commented out the "CopySymKeyForSigning" or some such call, in case we were dealing with hsms's or tokens that frown upon this sort of thing. Now the code first attempts the copy and if it fails , just uses the original key. This will work in situations when the secret is on an hsm and it will already have the proper capabilities built into the key, such as signing.

5.

Comment 5

[Matthew Harmsen]

Comment on attachment 8903315 [details] [diff] [review]
Proposed patch to solve issue and make the provided test work

I downloaded the patch, built it, tested it, and ran it as a smoke test for the PKI CA server.

Comment 6

[jmagne]

Commit:

changeset:   2197:eec15518fd61
tag:         tip
user:        Jack Magne <[e-mail]>
date:        Fri Sep 01 16:15:54 2017 -0700
files:       org/mozilla/jss/pkcs11/PK11KeyWrapper.java org/mozilla/jss/pkcs11/PK11MessageDigest.c org/mozilla/jss/tests/HmacTest.java org/mozilla/jss/tests/all.pl
description:
unwrapping of HMAC-SHA1 secret keys using AES wrapping and unwrapping
cfu on behalf of jmagne