115660 - cannot access certs as public objects while in FIPS mode

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Ian McGreer]

The way FC_FindObjects is constructed, it always requires login, no matter what
kind of search is being processed.  Now that certs are below PKCS#11, cert
objects (& trust objects & S/MIME profile objects) on the fips token need to be
accessible without login.

Comment 1

[Wan-Teh Chang]

Assigned the bug to Bob.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Is this bug valid?  

I thought that FIPS tokens were defined such that NO token objects are 
visible unless and until the user is logged in.

If that understanding is correct, then it sounds like this bug is invalid.

Or is the bug simply suggesting that the function should silently fail to 
find the objects when the FIPS token is not logged in?

Comment 3

[Robert Relyea]

I asked ian the same question when the bug was written.

While some many fips tokens do restrict the ability to read certs before
authenticating, it is not a requirement for FIPS certification. The requirement is
that secret and private keys are protected (and not visible). Our non-FIPS code
already does this.

bob

Comment 4

[Ian McGreer]

Login was always required to access token objects.  But prior to NSS 3.4, token
objects only included private keys.  The certificates were accessed by the CERT_
and SEC_ API's, which didn't even have a mechanism for authentication.

I wouldn't consider it bad policy for the FIPS token to require login before
accessing certs.  But making that change now would break a lot of functions that
depend on being able to access certs without authentication.

Comment 5

[Robert Relyea]

Fix and updated test cases have been checked in.

Comment 6

[Ian McGreer]

certutil is still logging into the FIPS token, and the QA is still providing the
password.  If you change that (remove the PK11_IsFIPS at line 645 of certutil),
you'll find that the QA fails.  The problem is that FC_GetAttributeValue needs
to be able to tell if the object handle is for a certificate.  Reopening bug.

Comment 7

[Ian McGreer]

Attachment: fix FC_GetAttributeValue

Bob, does this look correct to you?

Comment 8

[Robert Relyea]

OK, now certutil -L -d . works on fips tokens without a password.
FC_GetAttributeValue had to be changed to work without authentication.

Comment 9

[Robert Relyea]

Ooops need to mark it fixed...

Comment 10

[Ian McGreer]

I still don't consider this bug resolved, for a couple of reasons.

1.  The full set of object-related functions in the Crytoki API is:

C_CreateObject
C_CopyObject
C_DestroyObject
C_GetObjectSize
C_GetAttributeValue
C_SetAttributeValue
C_FindObjectsInit
C_FindObjects
C_FindObjectsFinal

Of those, when in FIPS mode all up to and including C_FindObjectsInit need to
detect whether the object is a private or secret key, and force authentication
if so.  I agree that the last two do not need this, because it is impossible to
continue a find without a previous call to C_FindObjectsInit.

At this point, we have only "resolved" C_FindObjectsInit and C_GetAttributeValue.

2.  fipstokn.c was created to enforce our FIPS policy on top of the softoken.  I
don't think we should *ever* rely on the softoken to maintain this policy.  The
change to C_GetAttributeValue that was checked in does exactly that.  For the
purposes of FIPS-compliance, I think we must keep our authentication policy
explicitly defined in fipstokn.c, which is why I proposed the patch I did.

Here is my full proposal for fipstokn.c:

Any Cryptoki function taking an object template as a parameter should search the
template for the object class, and require authentication is the object is a
private or secret key.  This is currently implemented in FC_FindObjectsInit, but
also applies to FC_CreateObject.

Any Cryptoki function taking an object handle as a parameter should use
NSC_GetAttributeValue to determine the object class, and require authentication
if it is a private or secret key.  This method is shown in attachment 1 [details] [diff] [review].  It
applies to FC_CopyObject, FC_DestroyObject, FC_GetObjectSize,
FC_GetAttributeValue, and FC_SetAttributeValue.

Comment 11

[Ian McGreer]

Attachment: all object functions should check for conditional login

It appears FC_CreateObject did check the template.  All of the functions taking
object handles as arguments needed the conditional check, this patch implements
it.

Comment 12

[Robert Relyea]

(Sigh). It is not true that the FIPS token was created strictly to enforce a
policy, it was created to augment the softoken's policy whenever necessary. In
fact it is impossible to inforce the FIPs token semantics without the softoken.
If you do not specify any class in C_FindObjectsInit, then you *ARE* depending
on the softoken to hide all the secret objects. Many of the FIPS checks are
already in the softoken (including the initial login check).

I purposefully did not add the write methods to the FIPs token. I believing we
should live with that regression rather than incur the risk changing the
database without a password incurs. On second thought, it's probably the wrong
semantic.

Looking at your patch: it would slow down the fips token, but it does look more
reliable (and easier to evaluate when FIPS evaluation time comes around again).

Go ahead and put your patch in. We still are depending on the softoken for
filtering the object id's when CLASS wasn't specified in the template, though.

Comment 13

[Ian McGreer]

Okay, I agree with you in terms of implementation.  That is, that we do rely on
the softoken to hide secret objects while in FIPS mode.

I guess I was coming from the angle of having to bury myself in the morass that
is our FIPS documentation.  From that angle, it does appear that fipstokn.c
enforces our authentication policy, because all of our flowcharts show
PK11_FIPSCHECK as the first step before entering the softoken.  So while I agree
that the code is correct as-is, I think the patch is needed to minimize the
changes to our stated FIPS policy.

Getting the object class should be fast, right?  Wouldn't the object be in a
hashtable after it has been found?

I'll check this in.  Bug fixed (again).

Comment 14

[Ian McGreer]

correcting subject