672828 - PK11_PubDeriveWithKDF does not call PORT_SetError when returning NULL due to unrecognized key type

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

+++ This bug was initially created as a clone of Bug #668397 +++

See:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/pk11wrap/pk11skey.c&rev=1.122&mark=1898,1905,1907,1912-1913,1918#1897

I believe the whole function body can be replaced with:

return privKey->keyType == ecKey
     ? pk11_PubDeriveECKeyWithKDF(privKey, pubKey, isSender, 
                                  randomA, randomB, derive,
                                  target, operation, keySize,
                                  kdf, sharedData, wincx)
     : PK11_PubDerive(privKey, pubKey, isSender, randomA,
                      randomB, derive, target, operation,
                      keySize, wincx);

Comment 1

[:Cykesiopka]

Attachment: bug672828_v1.patch

Patch for the change described in Comment 0.

Comment 2

[Ryan Sleevi]

Comment on attachment 8395443 [details] [diff] [review]
bug672828_v1.patch

Review of attachment 8395443 [details] [diff] [review]:
-----------------------------------------------------------------

Use PORT_SetError(SEC_ERROR_BAD_KEY) in the default case.

Comment 3

[Wan-Teh Chang]

Comment on attachment 8395443 [details] [diff] [review]
bug672828_v1.patch

Review of attachment 8395443 [details] [diff] [review]:
-----------------------------------------------------------------

::: lib/pk11wrap/pk11skey.c
@@ +2221,5 @@
> +                                     target, operation, keySize,
> +                                     kdf, sharedData, wincx)
> +        : PK11_PubDerive(privKey, pubKey, isSender, randomA,
> +                         randomB, derive, target, operation,
> +                         keySize, wincx);

Also, using the ? : operator for such a complicated expression
makes the code harder to read.

Comment 4

[:Cykesiopka]

(In reply to Wan-Teh Chang from comment #3)
> Comment on attachment 8395443 [details] [diff] [review]
> bug672828_v1.patch
> 
> Review of attachment 8395443 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: lib/pk11wrap/pk11skey.c
> @@ +2221,5 @@
> > +                                     target, operation, keySize,
> > +                                     kdf, sharedData, wincx)
> > +        : PK11_PubDerive(privKey, pubKey, isSender, randomA,
> > +                         randomB, derive, target, operation,
> > +                         keySize, wincx);
> 
> Also, using the ? : operator for such a complicated expression
> makes the code harder to read.

Noted, thanks.

Comment 5

[:Cykesiopka]

Attachment: bug672828_v2.patch

+ Use PORT_SetError(SEC_ERROR_BAD_KEY) in the default case

Comment 6

[Kai Engert [:KaiE:]]

https://hg.mozilla.org/projects/nss/rev/6f363b088c1e