336813 - NSC_GetTokenInfo does not return some applicable token information flags

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Wan-Teh Chang]

NSC_GetTokenInfo does not return all the token information
flags in PKCS #11 v2.20 Table 11 on p. 42 that are applicable
to the NSS softoken.

1. CKF_RNG is only returned in the "no db" mode.  This
flag should be returned in all cases.

2. CKF_WRITE_PROTECTED is only returned in the "no db" mode.
This should be returned when NSS is initialized in the "no db"
or "read only" mode.

3. CKF_TOKEN_INITIALIZED should be returned when there is a
key db.

4. CKF_THREAD_SAFE is returned in all cases, but this flag
is not defined in PKCS #11 v2.20.  We define this macro
in lib/softoken/pkcs11.c (note a C file), so even our
lib/pk11wrap layer is not using this flag.

5. I don't understand what CKF_RESTORE_KEY_NOT_NEEDED is,
so I don't know if we should return this flag.

Comment 1

[Wan-Teh Chang]

Attachment: First improvement

1. Make NSC_GetTokenInfo always sets CKF_RNG.  This flag
means the token has its own random number generator.  You
can verify this by looking at NSC_SeedRandom and NSC_GenerateRandom.
These two functions ignore the hSession argument, which means
any slot of the softoken can use the global RNG.

2. Set the utcTime member to 16 zeros "0000000000000000".
This member has the format "YYYYMMDDhhmmssxx".
It only makes sense for tokens with a clock.
So the softoken doesn't need to set this member, but it's
safer to set it to a default value.

3. Code cleanup. Factor out the common flag CKF_RNG and
CKF_THREAD_SAFE.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

What is the target for this fix?  3.11.5 ? or 3.12 ?

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 226738 [details] [diff] [review]
First improvement

r=nelson

Comment 4

[Robert Relyea]

Comment on attachment 226738 [details] [diff] [review]
First improvement

r+

The patch is correct, but incomplete. CKF_TOKEN_INITIALIZED still needs to be set (in the two places where we set CKF_USER_PIN_INITIALIZED).
It's ok not to set CKF_RESTORE_KEY_NOT_NEEDED. This is for C_SetOperationState and C_GetOperationState. some tokens will save the key encrypted in the OperationState. We don't do that so we shouldn't set the flag.

bob

Comment 5

[Wan-Teh Chang]

I checked in the patch "first improvement" on the NSS trunk (3.12)
and NSS_3_11_BRANCH (3.11.3).

Checking in fipstokn.c;
/cvsroot/mozilla/security/nss/lib/softoken/fipstokn.c,v  <--  fipstokn.c
new revision: 1.19; previous revision: 1.18
done
Checking in pkcs11.c;
/cvsroot/mozilla/security/nss/lib/softoken/pkcs11.c,v  <--  pkcs11.c
new revision: 1.129; previous revision: 1.128
done

Checking in fipstokn.c;
/cvsroot/mozilla/security/nss/lib/softoken/fipstokn.c,v  <--  fipstokn.c
new revision: 1.11.2.7; previous revision: 1.11.2.6
done
Checking in pkcs11.c;
/cvsroot/mozilla/security/nss/lib/softoken/pkcs11.c,v  <--  pkcs11.c
new revision: 1.112.2.14; previous revision: 1.112.2.13
done

Comment 6

[Wan-Teh Chang]

Attachment: Second improvement: CKF_THREAD_SAFE and CKR_WRITE_PROTECTED

1. Remove nonstandard flag CKF_THREAD_SAFE, which even NSS itself
doesn't use.

2. Set CKF_WRITE_PROTECTED if the token is read only.

I'm not sure exactly what CKF_TOKEN_INITIALIZED means for the softoken.
PKCS #11 allows token initialization and normal user PIN initialization
to be two separate steps.  Since the softoken creates the key and cert
databases automatically in C_Initialize, it seems to me that we should
always return the CKF_TOKEN_INITIALIZED flag.  At least, we should
always return the CKF_TOKEN_INITIALIZED flag for slot 1 (NETSCAPE_SLOT_ID).

Comment 7

[Robert Relyea]

In PKCS #11 2.0 there was no CKF_TOKEN_INITIALIZED, so there was no way to distinguish the case where the token has not yet been initialized (no password has been set) and the token has been initialized with a NULL password (the password was explicitly chosen to be null).

2.0 defined 2 other flags (CKF_LOGIN_REQUIRED and CKF_USER_PIN_INITIALIZED). We used the combination CKF_LOGIN_REQUIRED=1 and CKF_USER_PIN_INITALIZED=0 to
indicated that the module does not have any password initiallized yet.


CKF_LOGIN_REQUIRED CKF_USER_PIN_INITIALIZED     how CKF_TOKEN_INITIALIZED should 
                                                   be set..
        0                   0                           1
        1                   0                           0
        0                   1                           1
        1                   1                           1



bob

Comment 8

[Wan-Teh Chang]

Attachment: Second improvement v2: CKF_THREAD_SAFE, CKF_TOKEN_INITIALIZED, and CKF_DUAL_CRYPTO_OPERATIONS

Bob, thanks for explaining CKF_TOKEN_INITIALIZED.  This patch
sets CKF_TOKEN_INITIALIZED according to the truth table in your
comment.  I set it after we have set CKF_LOGIN_REQUIRED and
CKF_USER_PIN_INITIALIZED to make it clear that it is a function
of those two flags so that I can do without a comment that
summarizes your explanation.  Please let me know if you prefer
to set it in the four cases directly.

I am not sure about my change for CKF_WRITE_PROTECTED flag, so
I removed it from the patch.

I added the CKF_DUAL_CRYPTO_OPERATIONS because I found that the
softoken supports the dual-function crypgraphic functions in
PKCS #11 v2.20 Section 11.13.

Comment 9

[Robert Relyea]

Comment on attachment 234940 [details] [diff] [review]
Second improvement v2: CKF_THREAD_SAFE, CKF_TOKEN_INITIALIZED, and CKF_DUAL_CRYPTO_OPERATIONS

r+ rrelyea,

Nice to have: include the truth table before the if statement.

bob

Comment 10

[Wan-Teh Chang]

Attachment: Second improvement v3: CKF_THREAD_SAFE, CKF_TOKEN_INITIALIZED, and CKF_DUAL_CRYPTO_OPERATIONS

Bob, I thought the if statement is self-documenting.  But
since you asked, I included the truth table before the if
statement.

I'm requesting a second review for NSS_3_11_BRANCH checkin.
This patch is not required to obtain FIPS validation but
it will simplify the documentation.

I checked in the patch on the NSS trunk (NSS 3.12).

Checking in pkcs11.c;
/cvsroot/mozilla/security/nss/lib/softoken/pkcs11.c,v  <--  pkcs11.c
new revision: 1.132; previous revision: 1.131
done

Comment 11

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 235317 [details] [diff] [review]
Second improvement v3: CKF_THREAD_SAFE, CKF_TOKEN_INITIALIZED, and CKF_DUAL_CRYPTO_OPERATIONS

I don't know of that truth table is what we really want, but I confirm that the code correctly implements that truth table.  The table is good documentation.  If we later conclude that the results are not what we want, we will know it was not due to a coding error.

Comment 12

[Wan-Teh Chang]

I checked in the patch "second improvement v3" on the NSS_3_11_BRANCH
for NSS 3.11.3.

Checking in pkcs11.c;
/cvsroot/mozilla/security/nss/lib/softoken/pkcs11.c,v  <--  pkcs11.c
new revision: 1.112.2.17; previous revision: 1.112.2.16
done

This is as much as I wanted to fix on the NSS_3_11_BRANCH.  I will
create a new bug for the remaining work (CKF_WRITE_PROTECTED).