Last Comment Bug 463452 - SQL DB creation does not set files protections to 0600
: SQL DB creation does not set files protections to 0600
Status: RESOLVED FIXED
FIPS SUN_MUST_HAVE
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.12.2
: All All
: P1 critical (vote)
: 3.12.3
Assigned To: Robert Relyea
:
Mentors:
: 471756 (view as bug list)
Depends on:
Blocks: 900067 FIPS2008
  Show dependency treegraph
 
Reported: 2008-11-06 10:00 PST by Robert Relyea
Modified: 2013-07-31 09:40 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
preliminary patch, comments welcome (3.34 KB, patch)
2008-11-06 10:58 PST, Robert Relyea
no flags Details | Diff | Splinter Review
Set proper mode on NSS db files. (3.34 KB, patch)
2008-12-04 18:11 PST, Robert Relyea
nelson: review+
Details | Diff | Splinter Review

Description Robert Relyea 2008-11-06 10:00:15 PST
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.
Comment 1 Robert Relyea 2008-11-06 10:58:20 PST
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.
Comment 2 Robert Relyea 2008-12-04 18:11:55 PST
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).
Comment 3 Robert Relyea 2008-12-04 18:13:25 PST
Comment on attachment 351481 [details] [diff] [review]
Set proper mode on NSS db files.

patch is ready for review.
Comment 4 Nelson Bolyard (seldom reads bugmail) 2008-12-04 22:04:32 PST
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.
Comment 5 Nelson Bolyard (seldom reads bugmail) 2009-01-01 11:25:10 PST
*** Bug 471756 has been marked as a duplicate of this bug. ***
Comment 6 Robert Relyea 2009-03-09 16:00:30 PDT
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
Comment 7 Robert Relyea 2009-03-09 16:01:41 PDT
Actually the current patch does option 2 above.
Comment 8 Nelson Bolyard (seldom reads bugmail) 2009-03-09 18:17:07 PDT
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
Comment 9 Nelson Bolyard (seldom reads bugmail) 2009-03-12 21:00:13 PDT
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
Comment 10 Robert Relyea 2009-03-18 11:06:22 PDT
patch is in  with nelsons correction for windows.
Comment 11 Brad Lassey [:blassey] (use needinfo?) 2009-04-07 11:43:40 PDT
when this change was pushed to m-c it broke the windows ce build.  See bug 487254 and bug 487255.
Comment 12 Wan-Teh Chang 2009-04-08 16:16:10 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.