Wireshark-bugs: [Wireshark-bugs] [Bug 2495] New: Wireshark does not respect user's umask; incons
Date: Tue, 22 Apr 2008 13:43:31 -0700 (PDT)
http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2495 Summary: Wireshark does not respect user's umask; inconsistent file permissions when saving Product: Wireshark Version: SVN Platform: All OS/Version: All Status: NEW Severity: Normal Priority: Low Component: Wireshark AssignedTo: wireshark-bugs@xxxxxxxxxxxxx ReportedBy: vandry@xxxxxxxxx Build Information: [The following build can reproduce the problem but the code looks like it is the same in SVN] wireshark 0.99.6 Copyright 1998-2007 Gerald Combs <gerald@xxxxxxxxxxxxx> and contributors. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Compiled with GTK+ 2.12.0, with GLib 2.14.1, with libpcap 0.9.7, with libz 1.2.3.3, with libpcre 7.4, without Net-SNMP, with ADNS, without Lua, with GnuTLS 1.6.3, with Gcrypt 1.2.4, with MIT Kerberos, with PortAudio <= V18, without AirPcap. Running on Linux 2.6.22-14-server, with libpcap version 0.9.7. Built using gcc 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2). -- PROBLEM: In different circumstances, wireshark will save capture files with at least three different combinations of file permissions. This inconsistency leads to results that users cannot predict very well and will not expect. Furthermore, only one of these combinations of file permissions is actually correct: the one that respects the permissions mandated by the user's environment (umask). Since wireshark is often used in combination with utilities like "su" because it required privileged access to sniff, users often attempt to read capture files as regular users that were written as root. Wrong file permissions can easily interfere with being able to do this. BACKGROUND: When opening a live capture to a temporary file, the try_tempfile() function from file.c is used. This function manipulates the umask so the file permissions are always 0600 (owner can read&write, everybody else can neither read nor write). This is reasonable and correct: temporary files located in possibly public temporary directories should use restrictive permissions rather than respecting the process umask. But when saving a capture that is stored in a temporary file to a new file, the temporary file will be renamed if possible, thus preserving these file permissions. They aren't apropriate for a file which the user wants to keep. When saving a capture to a given file, the file might be copied with copy_binary_file() from file.c, which specifies permissions 0644 (which will be combined with the umask) if no processing is needed. Else, the file will be opened using wtap_dump_file_open() from wiretap/file_access.c which uses default unspecified) file permissions. RESULT: 1. If the result of a live capture is saved to a file without any filtering and renaming the temporary file was possible, the permissions will be -rw-------. 2. If the result of a live capture is saved to a file without any filtering and renaming the temporary file was impossible, the permissions will be (-rw-r--r--)&(umask). 3. In every other case, the permissions will be (-rw-rw-rw-)&(umask). Using a typical permissive umask of 022, this leads to (1) -rw-------, (2) -rw-r--r--, (3) -rw-r--r-- Using a typical restrictive umask of 077, this leads to (1) -rw-------, (2) -rw-------, (3) -rw------- Using a very permissive umask of 000, this leads to (1) -rw-------, (2) -rw-r--r--, (3) -rw-rw-rw- This last umask is not very common, but it demonstrates that three different combinations of permissions can be obtained. TO REPRODUCE: 1. set umask as desired (to 000 for best demonstration) 2. start wireshark 3. capture some packets then stop the capture 4. save the result to a file in a directory on the same filesystem as the temporary file. You will obtain case (1) above. 5. save the result again to another file. You will obtain case (2) above. 6. save the result again but this time specify some filtering (such as "Specified packet only"). You will obtain case (3) above. You now have three saved captures. If you used umask 000, they will have three different sets of file permissions. RESOLUTION: There are two bugs. First bug: copy_binary_files() should create files with permissions that respect the process umask. Here is a patch for the first bug: Index: file.c =================================================================== --- file.c (revision 25148) +++ file.c (working copy) @@ -3939,7 +3939,7 @@ may open the file in text mode, not binary mode, but we want to copy the raw bytes of the file, so we need the output file to be open in binary mode. */ - to_fd = eth_open(to_filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644); + to_fd = eth_open(to_filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666); if (to_fd < 0) { open_failure_alert_box(to_filename, errno, TRUE); eth_close(from_fd); Second bug: Saved capture files should not be given the same file permissions as temporary files, even if the saved capture file is just a renamed temporary file. There are a couple of options to solve this bug. I will implement the solution but I would like to present the options and find out which solution the wireshark developers prefer first. Second bug, option 1: At the moment the file is renamed, use chmod() to change the permissions to what they should have been if the file had been created with default permissions to begin with. Option 1 pro: no change in how temporary files are created and handled, so easiest, least intrusive. Option 1 con: reading the process umask() is racy, and explicitely manipulating file permissions is cumbersome. Second bug, option 2: Change the way temporary files are handled. Do not use mkstemp() anymore to create them. The software shall create a temporary directory with mode 0700 and place all temporary files inside this directory. The directory itself, just like the temporary files inside it, shall be deleted when the program exits. The temp files created inside this directory can be created with the correct permissions for saving right away, so there is no need to worry about changing them after rename(). Option 2 pro: clean, and no more playing with the umask. The umask does its thing and this is transparent to wireshark like it is to most software. Option 2 con: temporary files created and handled differently. Probably no users depend on Wireshark's temporary file creation scheme, but if they did, the change would break them. My recommendation is option 2. -Phil -- Configure bugmail: http://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the assignee for the bug.
- Follow-Ups:
- [Wireshark-bugs] [Bug 2495] Wireshark does not respect user's umask; inconsistent file permissions when saving
- From: bugzilla-daemon
- [Wireshark-bugs] [Bug 2495] Wireshark does not respect user's umask; inconsistent file permissions when saving
- From: bugzilla-daemon
- [Wireshark-bugs] [Bug 2495] Wireshark does not respect user's umask; inconsistent file permissions when saving
- From: bugzilla-daemon
- [Wireshark-bugs] [Bug 2495] Wireshark does not respect user's umask; inconsistent file permissions when saving
- From: bugzilla-daemon
- [Wireshark-bugs] [Bug 2495] Wireshark does not respect user's umask; inconsistent file permissions when saving
- Prev by Date: [Wireshark-bugs] [Bug 2494] New: Fix dissection of Location and Generic Number in CAMEL
- Next by Date: [Wireshark-bugs] [Bug 2493] Wireshark compile errors for cygwin flex (v 2.5.35) generated . c files
- Previous by thread: [Wireshark-bugs] [Bug 2494] Fix dissection of Location and Generic Number in CAMEL
- Next by thread: [Wireshark-bugs] [Bug 2495] Wireshark does not respect user's umask; inconsistent file permissions when saving
- Index(es):
- Get Wireshark
- Download
- Code of Conduct