cmsutil will not work in FIPS mode

RESOLVED FIXED in 3.4

Status

NSS
Libraries
P1
major
RESOLVED FIXED
17 years ago
16 years ago

People

(Reporter: Ian McGreer, Assigned: Julien Pierre)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

17 years ago
The new S/MIME library does often passes NULL as the wincx variable in calls
below the API.  This prevents the passing of a password handler down to PKCS#11,
which will cause problems in FIPS mode.

There may be other problems.  For fun, I tried using cmsutil while in FIPS mode
and watched it crash all over.

Comment 1

17 years ago
Target this for NSS 3.4.
Priority: -- → P1
Target Milestone: --- → 3.4

Comment 2

17 years ago
Assigned the bug to Julien.
Assignee: wtc → jpierre
(Assignee)

Comment 3

17 years ago
Ian,

Will this affect the browser as well, if running in FIPS mode ?
(Reporter)

Comment 4

17 years ago
It depends on PSM's implementation.  I'm fairly sure that the problems are a
result of the FIPS token not receiving the key database password.  PSM, in
general, does a much better job of handling passwords than our own tools.

However, there may be other bugs.  The following patch is for tests/smime.sh,
once you apply the patch all the databases used in the S/MIME test will be in
FIPS mode during the tests.  You can see what happens - all but two fail.
(Reporter)

Comment 5

17 years ago
Created attachment 56876 [details] [diff] [review]
patch to reveal FIPS mode S/MIME failures

Comment 6

17 years ago
cc ddrinan.
David, would you please check how PSM deals with passwords in FIPS mode while
using the s/mime libraries.
(Reporter)

Comment 7

17 years ago
I thought I should look into this a little more.  Unfortunately, I bring only
bad news.

When Christian wrote the S/MIME libraries, he defined some of the object
initialization calls to take a PK11PasswordFunc callback and an argument (e.g.,
NSS_CMSDecoder_Start).  I presume this was to avoid having to propagate "void
*wincx" arguments throughout the API, but it does not work.  First of all, those
functions then set the *global* PKCS#11 password callback.  This is in no way
context-specific, or thread-safe, and will simply not work with PSM.

I don't know if you've come across this problem yet, David, but there is a
possible solution for part of it.  That is to always pass NULL to functions
looking for a PKCS11PasswordFunc, and let your global password handler take care
of things.  However, you need to pass the window context through the next argument.

For example, set:

PK11_SetPasswordFunc(myglobalfunc);

then call NSS_CMSEncoder_Start with pwfn=NULL and pwfn_arg=wincx.

Another problem is that NSS_CMSMessage_Create does not store pwfn_arg with the
NSSCMSMessage, which it needs to do.  A way to work around this is to call
NSS_CMSMessage_SetEncodingParams after creating the CMSMessage.

I will attach a patch I made to cmsutil that reflects these changes, and gets it
nearly working in FIPS mode. 
(Reporter)

Comment 8

17 years ago
Created attachment 56952 [details] [diff] [review]
patch that fixes many pw handling issues in cmsutil
(Assignee)

Comment 9

17 years ago
Is it an option to extend the APIs to take the needed wincx argument, in order
to make this work better, rather than patch and use the global callback ?
(Reporter)

Comment 10

17 years ago
I wouldn't think that would be a 3.X option, given that it breaks backward
compatibility.  OTOH, only PSM is using this library (that we know of).

I agree that the long term solution is to review the API as a whole.  But I
don't see how we could take that on now.

Using the global callback is fairly standard practice in NSS.  However, most
functions allows you to set the parameter passed through to that callback
(wincx).  None of the CMS functions do; you can only set it once, as I showed in
the patch.

Comment 11

17 years ago
Even though we can't change the existing functions to take
an extra wincx parameter, we can add new functions (with
different names) that take the needed wincx parameter.

Will that work, Ian?
(Assignee)

Comment 12

17 years ago
Wan-Teh,
You responded before I got a chance - this is in fact what I wanted to do, add
new APIs with the extra parameters. We can deprecate the existing ones in 3.4
without eliminating them. We can delete them in the next major release.
(Reporter)

Comment 13

17 years ago
IIRC, the S/MIME API was never really properly reviewed.  There are other minor
problems, like the fact that it doesn't quite use the NSS 4.0 naming scheme
(NSS_CMSSignedData_XXX instead of NSSCMSSignedData_XXX).

I would say that extending portions of the API for 3.X releases is overkill for
this bug, given that we know how to fix it within the current API spec.  I would
prefer to save API redefinition for a later time, when we have a more complete
idea of the changes we would like to make.  But that's just my preference, and
extending the API is certainly one way to fix this bug.
(Assignee)

Comment 14

17 years ago
Ian,

You mentioned in today's NSS meeting that this problem might be solved by your
patch. However, I only see a patch for cmsutil, nothing in the S/MIME library.
Should I assume that we need to have PSM do the same sort of thing as your patch
does to cmsutil in order to get FIPS mode to work with S/MIME in PSM ?
(Reporter)

Comment 15

17 years ago
That's correct.  At this time, the problem needs to be worked around by the
application.

However, currently the libraries are blocking this bug.  See 115660.
Depends on: 115660
(Assignee)

Comment 16

17 years ago
Wan-Teh,

As the release date for NSS 3.4 is nearing, do we want to simply let the
application work around this problem and move this bug to 4.0 ?

Comment 17

17 years ago
Bob, Nelson, Ian,

Do you think we need to fix this in NSS 3.4?  If
applications can work around this bug (see comment #15),
I suggest that we lower the priority of this bug and
move it to 4.0.
(Reporter)

Comment 18

17 years ago
I agree.
What exactly is the workaround?  

I don't think we should release NSS 3.4 unless S/MIME can be used in 
FIPS mode by PSM.
(Reporter)

Comment 20

17 years ago
The workaround is shown in the second attachment.  This patch shows what changes
had to be made to cmsutil for it to run in FIPS mode.

The problem was described in comment #7.  I don't remember exactly what
functions were problematic, but I found that the changes shown in the patch were
able to consistently provide the correct argument to the password callback function.

Comment 21

17 years ago
Ian,

1. Can we say that although the S/MIME library will not work in
FIPS mode, cmsutil now works in FIPS mode with your workaround?

2. Does the workaround rely on some global state to implicitly
pass the required info to the S/MIME functions?  If so, the
workaround may not work when multiple threads are doing S/MIME
simultaneously.

3. Is it a lot of work to implement the correct solution (new
functions that take the additional argument)?  Have we identified
all the S/MIME functions that won't work in FIPS mode?

Comment 22

17 years ago
Also, would fixing the listing of certs issue fix this problem for the FIPs
case. (I have a fix for that bug now as well).

BTW, we don't loose our FIPS certification if we allow certs, public keys, and
trust to be found in the FIPs token if the token hasn't been logged in do we?

bob
(Reporter)

Comment 23

17 years ago
> 1. Can we say that although the S/MIME library will not work in
> FIPS mode, cmsutil now works in FIPS mode with your workaround?

Perhaps the subject is misleading.  Because cmsutil will work in FIPS mode with
the changes in the patch, the S/MIME library *will* work in FIPS mode.  It's
just in a non-obvious way.  That is, the API purports to take the password
callback function as an argument to some functions, but then doesn't use it
correctly.  The patch shows how to force correct usage.  In other words, it's
not that the library won't work, but that the API is deficient.  Fixing the API
is certainly a Future issue, IMO.

> 2. Does the workaround rely on some global state to implicitly
> pass the required info to the S/MIME functions?  If so, the
> workaround may not work when multiple threads are doing S/MIME
> simultaneously.

No.

> 3. Is it a lot of work to implement the correct solution (new
> functions that take the additional argument)?  Have we identified
> all the S/MIME functions that won't work in FIPS mode?

I'm not sure.  I would have to start over by letting cmsutil fail without the
patch, and track down what functions failed.

I don't consider the patch to be that ugly of a workaround, as it is.  There are
simply two things to note:

1.  Don't pass an value to the PK11PasswordFunc argument when calling
NSS_CMSDecoderStart and NSS_CMSEncoderStart, but do pass a value to the pwfn_arg
argument.  The value to pass is the usual wincx variable.

2.  After creating an NSSCMSMessage to encode, call
NSS_CMSMessage_SetEncodingParams, and for the third argument pass the wincx
variable.

> BTW, we don't loose our FIPS certification if we allow certs, public keys, and
> trust to be found in the FIPs token if the token hasn't been logged in do we?

We received a validation with those operations as public functions.  They always
were public, therefore never required login.

Comment 24

17 years ago
Hmm. Actually they way Ian described the CMS calls is exactly how the calls
worked in PKCS7. The inline password function was a depricated function in
PKCS7. It sounds to me that there is nothing to do for NSS 3.4 here. 

bob

Comment 25

16 years ago
Changed the bug summary to "cmsutil will not work in FIPS mode".
Marked the bug fixed.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Summary: S/MIME library will not work in FIPS mode → cmsutil will not work in FIPS mode
You need to log in before you can comment on or make changes to this bug.