SQL DB creation does not set files protections to 0600

RESOLVED FIXED in 3.12.3

Status

NSS
Libraries
P1
critical
RESOLVED FIXED
9 years ago
4 years ago

People

(Reporter: Robert Relyea, Assigned: Robert Relyea)

Tracking

(Blocks: 1 bug)

3.12.2
3.12.3
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: FIPS SUN_MUST_HAVE)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 years ago
The old database explicitly protected the NSS database files with user rw and all other's no access. At least the key DB, and preferably the others need this protection as well.
(Assignee)

Updated

9 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

9 years ago
Blocks: 459298
Whiteboard: FIPS
Severity: normal → critical
OS: Linux → All
Priority: -- → P1
Hardware: PC → All
Whiteboard: FIPS → FIPS SUN_MUST_HAVE
Target Milestone: --- → 3.12.3
(Assignee)

Comment 1

9 years ago
Created attachment 346705 [details] [diff] [review]
preliminary patch, comments welcome

This patch has only been tested on Linux. It needs to be tested on a few other platforms, particularly since it is not using our normal porting layer (NSPR), since some of the required NSPR functions are missing.
(Assignee)

Comment 2

9 years ago
Created attachment 351481 [details] [diff] [review]
Set proper mode on NSS db files.

This patch works on both Linux and Windows (the latter doesn't actually have any modes).
Attachment #346705 - Attachment is obsolete: true
(Assignee)

Comment 3

9 years ago
Comment on attachment 351481 [details] [diff] [review]
Set proper mode on NSS db files.

patch is ready for review.
Attachment #351481 - Flags: review?(alexei.volkov.bugs)
Comment on attachment 351481 [details] [diff] [review]
Set proper mode on NSS db files.

Bob,  

I have two issues with this patch:

1. Windows does have an approximate equivalent to Unix modes.
NSPR knows how to set Windows' security descriptors when you pass Unix 
mode flags to PR_OpenFile, PR_OpenFileUTF16, or PR_MakeDir. See

http://mxr.mozilla.org/security/source/nsprpub/pr/src/md/windows/w95io.c#245
http://mxr.mozilla.org/security/source/nsprpub/pr/src/md/windows/w95io.c#1075
http://mxr.mozilla.org/security/source/nsprpub/pr/src/md/windows/w95io.c#1228

http://mxr.mozilla.org/security/source/nsprpub/pr/src/md/windows/ntio.c#2148
http://mxr.mozilla.org/security/source/nsprpub/pr/src/md/windows/ntio.c#3197

http://mxr.mozilla.org/security/source/nsprpub/pr/src/io/prfile.c#386
http://mxr.mozilla.org/security/source/nsprpub/pr/src/io/prfile.c#800
http://mxr.mozilla.org/security/source/nsprpub/pr/src/io/prdir.c#93

Now, it appears to be true that NSPR doesn't offer a chmod-like function,
but perhaps it should.  In any case, I think it is appropriate for NSS
to use similar logic to that used by NSPR for mapping unix file permissions
to Windows security descriptors, whether it does that by calling a function
in NSPR, or by doing the work itself.

2. On Windows, mode 0600 is frequently a problem.  
It's better to use mode 0660 as the default on Windows.
Summary: new Shared DB does not set database files protections to 0600 → SQL DB creation does not set files protections to 0600
Duplicate of this bug: 471756
(Assignee)

Comment 6

8 years ago
So I can go 2 ways here, I can use windows own posix mappings and call chmod and open directly with 0600, or we can say on windows, it's better to set 660, which is default.

I traced what we are doing in the old dbm code, and we pass 0600 to dbopen(), which on windows eventually maps to the normal libc open with 0600.

Which patch should I attach:

1) that never messes with mode at all on Windows neither pkcs11.txt nor cert & key databases.
--- or ----
2) one that uses windows libc mapping and passes 0600 for both pkcs11.txt and the cert & key databases.


bob
(Assignee)

Comment 7

8 years ago
Actually the current patch does option 2 above.
Comment on attachment 351481 [details] [diff] [review]
Set proper mode on NSS db files.

I guess there's no good solution for windows. :(
r=nelson
Attachment #351481 - Flags: review+
It appears that this checkin has broken the build on Windows.
Problem is: chmod is undeclared.
I have committed a trivial fix.

Checking in softoken/sdb.c;     new revision: 1.10; previous revision: 1.9
Checking in softoken/sftkmod.c; new revision: 1.4;  previous revision: 1.3
(Assignee)

Updated

8 years ago
Attachment #351481 - Flags: review?(alexei.volkov.bugs)
(Assignee)

Comment 10

8 years ago
patch is in  with nelsons correction for windows.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
when this change was pushed to m-c it broke the windows ce build.  See bug 487254 and bug 487255.

Comment 12

8 years ago
Comment on attachment 351481 [details] [diff] [review]
Set proper mode on NSS db files.

>+/* same as fopen, except it doesn't use umask, but explicit */

Nit: this comment is misleading because umask is still used.
The file access modes for the new file are 0600 & ~umask.'
See http://www.opengroup.org/onlinepubs/000095399/functions/open.html
under "O_CREAT":

  ... and the access permission bits (see <sys/stat.h>) of the
  file mode shall be set to the value of the third argument
  taken as type mode_t modified as follows: a bitwise AND is
  performed on the file-mode bits and the corresponding bits
  in the complement of the process' file mode creation mask.

Updated

4 years ago
Blocks: 900067
You need to log in before you can comment on or make changes to this bug.