488350 - NSPR-free freebl interface need to do post tests only in fips mode.

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Robert Relyea]

The NSSLOW_ interface to freebl should only do POST when the OS is in FIPS mode.

Comment 1

[Robert Relyea]

Attachment: freebl patch

Comment 2

[Robert Relyea]

Attachment: Friendlier version of this patch.

Comment 3

[Wan-Teh Chang]

Comment on attachment 372682 [details] [diff] [review]
Friendlier version of this patch.

r=wtc.

It's strange for nsslow_GetFIPSEnabled() to return 1 by default.
It seems that nsslow_GetFIPSEnabled() should return 0 by default.
In other words, the code at the end of the function should look
like this:

>+    if (d == '1')
>+        return 1;
>+#endif
>+    return 0;
>+}

Also, on Linux distributions that don't have the special file
/proc/sys/crypto/fips_enabled, nsslow_GetFIPSEnabled() returns 0.
Is that what you want?

Comment 4

[Robert Relyea]

re: default.

on non-linux, I would think that you would want the fips check if you don't have a way of turning it on. It's moot since only linux uses this mode right now. I'd be happy to change the default.

re missing /proc....

That's a better question. I thought about it to and I lean toward turning it on if the /proc doesn't exist, but I could go either way. Thanks for the review wan-Teh..

Comment 5

[Wan-Teh Chang]

Yeah, the default return value doesn't matter much to me,
but the code would be more self-consistent if it treats
a non-Linux OS and a Linux distro without /proc/sys/crypto/fips_enabled
the same way.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

This is obviously P1.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 372682 [details] [diff] [review]
Friendlier version of this patch.

/proc seems like an odd place for this.  It's not a process...  but I don't really care.  If Linux wants it there...

Is there any sort of attack possible here?  
Might an attacker want to disable FIPS mode for some reason?  Or force it to be enabled for some reason?  
Might an attacker be able to effect the change he wants by playing some game with the directories in that path?  
If so, is there a way for this to "fail safe"?
In the event of an attack, should it fail by going into FIPS mode by default? or by not going into FIPS mode by default?

I don't have answers to these questions.  I'm just passing them on for you to think about.

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Might an attacker be able to effect this change by doing a chroot 
in a parent process before running the code that uses NSS?

Comment 9

[Robert Relyea]

I know /proc is a mounted filesystem. It's used to present kernel variables (not just processes). On linux you can find out all sorts of things, and privileged users can even change kernel behavior on the fly.

chroot is an interesting issue. I think that says we should probably default to FIPS on as wtc suggests.

bob

Comment 10

[Robert Relyea]

Checking in nsslowhash.c;
/cvsroot/mozilla/security/nss/lib/freebl/nsslowhash.c,v  <--  nsslowhash.c
new revision: 1.3; previous revision: 1.2
done

Checked in. default for failure to open changes to '1' (use fips mode).

Comment 11

[Robert Relyea]

fixed

Comment 12

[Wan-Teh Chang]

Your checkin looks good, Bob.  Nit: to go all the
way, you may want to do something like this so
that only a value of '0' in /proc/sys/crypto/fips_enabled
can make nsslow_GetFIPSEnabled return 0:

+    if (size != 1)
+        return 1;
+    if (d == '0')
+        return 0;

Comment 13

[Kai Engert (:KaiE:)]

(In reply to comment #12)
> Your checkin looks good, Bob.  Nit: to go all the
> way, you may want to do something like this so
> that only a value of '0' in /proc/sys/crypto/fips_enabled
> can make nsslow_GetFIPSEnabled return 0:
> 
> +    if (size != 1)
> +        return 1;
> +    if (d == '0')
> +        return 0;


Looks like this comment was missed.

Wan-Teh, if you propose this to be done, would it make sense to file a new bug?