128144 - PK11_PQG_ParamGen is stubbed out

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Jamie Nicolson]

Several of the functions in pk11pqg.c have been ifdef'd out. JSS calls these
functions. Why have they been removed?

Comment 1

[Robert Relyea]

Sigh.

This is a complete oversight. What is missing is the PK11 wrapper functions to
call C_GenerateKey with the CKM_DSA_PARAMETER_GEN flag and the companion
implementation in pkcs11c.c to deal with it.

bob

Comment 2

[Wan-Teh Chang]

We should add a test that uses this function to the NSS or JSS
test suite.  It is a shame that we found out about this after
having passed NSS and JSS QA for more than a month.

Comment 3

[Robert Relyea]

Attachment: Implement PQG Generation and Verification functions.

Generation is handled as specified by PKCS #11 v2.11, except the verification
parameters are also available as attributes (using CKM_DSA_GEN_PARAMETERS and
C_GenterateKey).

Verification is handled as a side effect of creating the DSA CKO_KG_PARAMTERS
object with C_CreateObject().

Comment 4

[Robert Relyea]

Attachment: Changes to makepqg so that it binds with the DLL and uses the PK11_ Interfaces

This program was used as a rudimentary smoke test for the new pqg code.

Comment 5

[Robert Relyea]

This patch should fix the problem.

Comment 6

[Jamie Nicolson]

There's a problem in PK11_PQG_VerifyParams. If the params verify successfully,
*result is left undefined instead of being assigned the value SECSuccess. The
function specification implies *result will be set to SECSuccess if the params
are valid, and SECFailure if they are invalid. I will attach a patch.

Comment 7

[Jamie Nicolson]

Attachment: patch to fix bug in VerifyParams

If the PKCS #11 function returns crv==CKR_OK, we should set
*result==SECSuccess.

Comment 8

[Robert Relyea]

Good catch Jamie. I wanted to check what the original VerifyParams did before I
proposed a patch. The original does not return a meaningful *result value on
failure (it is possible to have result == SECSuccess even of the whole function
failed). I would propose the patch should be :

+   *result = SECSuccess;
    if (crv == CKR_ATTRIBUTE_VALUE_INVALID) {
        *result = SECFailure;
    } else if (crv != CKR_OK) {
        PORT_SetError( PK11_MapError(crv) );
        rv = SECFailure;
    }

We should check this in rather than have apps work around it.

bob

Comment 9

[Jamie Nicolson]

Either patch is fine with me. Do you want to check it in, or shall I?

Comment 10

[Robert Relyea]

New patch should be checked in.