288657 - pk11_AnyUnwrapKey does not process error condition correctly

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Steve Parkinson]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20040913
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20040913

In pk11_AnyUnwrapKey(), if C_UnwrapKey fails with CKR_DEVICE_ERROR, it   returns
a partially-constructued symKey object to the application. 

See code excerpt below.


3330        crv = PK11_GETTAB(slot)->C_UnwrapKey(rwsession,&mechanism,wrappingKey,
3331                    wrappedKey->data, wrappedKey->len, keyTemplate,
templateCount,
3332                                                             
&symKey->objectID);
3333        if (isPerm) {
3334            PK11_RestoreROSession(slot, rwsession);
3335        } else {
3336            pk11_ExitKeyMonitor(symKey);
3337        }
3338        if (param_free) SECITEM_FreeItem(param_free,PR_TRUE);
3339        if ((crv != CKR_OK) && (crv != CKR_DEVICE_ERROR)) {
(gdb) list
3340            /* try hand Unwrapping */
3341            PK11_FreeSymKey(symKey);
3342            symKey = pk11_HandUnwrap(slot, wrappingKey, &mechanism, wrappedKey,
3343                                     target, keyTemplate, templateCount,
keySize,
3344                                     wincx, NULL, isPerm);
3345       }
3346
3347       return symKey;
3348    }




Reproducible: Always

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch

Bob, is this the right fix?  I'm not sure if I
understood the intention of the code.

Comment 2

[Wan-Teh Chang]

Comment on attachment 179289 [details] [diff] [review]
Proposed patch

I forgot to describe what my patch does.

If C_UnwrapKey fails with CKR_DEVICE_ERROR, it destroys
the partially-constructued symKey object and returns
NULL to the application.

If C_UnwrapKey fails with any other error, it does the
same thing as before.

Comment 3

[Robert Relyea]

Comment on attachment 179289 [details] [diff] [review]
Proposed patch

Yup, that does exactly what it should.

Comment 4

[Wan-Teh Chang]

I checked in the fix on the NSS trunk (NSS 3.10).

Checking in pk11skey.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11skey.c,v  <--  pk11skey.c
new revision: 1.97; previous revision: 1.96
done