Description of problem: When an encrypted private key is imported into a slot in the PEM module, the module sets the slot's needsLogin flag to signal that a password is required. When the application attempts to log in to the token, the module attempts to decrypt the private key, but after decryption, it overwrites the encrypted key data with the decrypted data before verifying that the decrypted data isn't just the garbage that you get when you decrypt with an incorrect key. After that, there's no way to recover the data. It really should wait until after SEC_QuickDERDecodeItem() succeeds before modifying the contents of io->u.key.key.privateKey. Version-Release number of selected component (if applicable): nss-3.14.1-2.fc18
Created attachment 680373 [details] patch to only replace the encrypted data with decrypted data after it's vetted
Comment on attachment 680373 [details] patch to only replace the encrypted data with decrypted data after it's vetted > --- mozilla/security/nss/lib/ckfw/pem/psession.c > +++ mozilla/security/nss/lib/ckfw/pem/psession.c > @@ -230,6 +230,7 @@ pem_mdSession_Login > unsigned int len = 0; > NSSLOWKEYPrivateKey *lpk = NULL; > PLArenaPool *arena; > + SECItem plain; > int i; > > fwSlot = NSSCKFWToken_GetFWSlot(fwToken); > @@ -306,23 +321,31 @@ pem_mdSession_Login > lpk->keyType = NSSLOWKEYRSAKey; > prepare_low_rsa_priv_key_for_asn1(lpk); > > - nss_ZFreeIf(io->u.key.key.privateKey->data); > - io->u.key.key.privateKey->len = len - output[len - 1]; > - io->u.key.key.privateKey->data = > - (void *) nss_ZAlloc(NULL, io->u.key.key.privateKey->len); > - memcpy(io->u.key.key.privateKey->data, output, len - output[len - 1]); > + if (output[len - 1] > 8) { > + rv = CKR_GENERAL_ERROR; > + /* goto loser; */ > + } Why is the goto statement commented out? It makes the assignment redundant. It would also help to put there a comment saying which error case is being handled by the condition.
(In reply to comment #2) > Comment on attachment 680373 [details] > patch to only replace the encrypted data with decrypted data after it's > vetted > > > --- mozilla/security/nss/lib/ckfw/pem/psession.c > > +++ mozilla/security/nss/lib/ckfw/pem/psession.c > > @@ -230,6 +230,7 @@ pem_mdSession_Login > > unsigned int len = 0; > > NSSLOWKEYPrivateKey *lpk = NULL; > > PLArenaPool *arena; > > + SECItem plain; > > int i; > > > > fwSlot = NSSCKFWToken_GetFWSlot(fwToken); > > @@ -306,23 +321,31 @@ pem_mdSession_Login > > lpk->keyType = NSSLOWKEYRSAKey; > > prepare_low_rsa_priv_key_for_asn1(lpk); > > > > - nss_ZFreeIf(io->u.key.key.privateKey->data); > > - io->u.key.key.privateKey->len = len - output[len - 1]; > > - io->u.key.key.privateKey->data = > > - (void *) nss_ZAlloc(NULL, io->u.key.key.privateKey->len); > > - memcpy(io->u.key.key.privateKey->data, output, len - output[len - 1]); > > + if (output[len - 1] > 8) { > > + rv = CKR_GENERAL_ERROR; > > + /* goto loser; */ > > + } > > Why is the goto statement commented out? It makes the assignment redundant. My early assumption that the size of the padding would always be smaller than the cipher block size proved to be incorrect. Feel free to drop that part.
Created attachment 696984 [details] the same patch, with redundant assignment removed
nss-3.14.3-1.fc18,nss-softokn-3.14.3-1.fc18,nss-util-3.14.3-1.fc18,nspr-4.9.5-2.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/nss-3.14.3-1.fc18,nss-softokn-3.14.3-1.fc18,nss-util-3.14.3-1.fc18,nspr-4.9.5-2.fc18
Package nss-3.14.3-1.fc18, nss-softokn-3.14.3-1.fc18, nss-util-3.14.3-1.fc18, nspr-4.9.5-2.fc18: * should fix your issue, * was pushed to the Fedora 18 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing nss-3.14.3-1.fc18 nss-softokn-3.14.3-1.fc18 nss-util-3.14.3-1.fc18 nspr-4.9.5-2.fc18' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2013-2929/nss-3.14.3-1.fc18,nss-softokn-3.14.3-1.fc18,nss-util-3.14.3-1.fc18,nspr-4.9.5-2.fc18 then log in and leave karma (feedback).
nss-3.14.3-1.fc17,nss-softokn-3.14.3-1.fc17,nss-util-3.14.3-1.fc17,nspr-4.9.5-2.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/nss-3.14.3-1.fc17,nss-softokn-3.14.3-1.fc17,nss-util-3.14.3-1.fc17,nspr-4.9.5-2.fc17
nss-3.14.3-1.fc18, nss-softokn-3.14.3-1.fc18, nss-util-3.14.3-1.fc18, nspr-4.9.5-2.fc18 has been pushed to the Fedora 18 stable repository. If problems still persist, please make note of it in this bug report.
nss-3.14.3-1.fc17, nss-softokn-3.14.3-1.fc17, nss-util-3.14.3-1.fc17, nspr-4.9.5-2.fc17 has been pushed to the Fedora 17 stable repository. If problems still persist, please make note of it in this bug report.