OK, I *think* I have done as you requested. I have to try the environment variable before falling back on stderr (there's a program we want to use this on that disables the ability to write to stderr). I have added the secure_getenv stuff as you requested. The fixed patch is attached.
Please review the patch and let me know if this is OK to commit. Thanks!
-- Caroline Tice
cmt...@google.com
2013-08-16 Caroline Tice <cmt...@google.com>
* configure.ac: Add check for __secure_getenv and secure_getenv.
* configure: Regenerate.
* vtv_utils.cc : Include stdlib.h
(HAVE_SECURE_GETENV): Add checks and definitions for secure_getenv.
(log_dirs): Remove file static constant.
(__vtv_open_log): Increase size of log file name. Add the user
and process ids to the file name. Do not put the log files in /tmp.
Instead try to get the directory name from an environment variable; if
that fails try to use stderr. Add O_NOFOLLOW to the flags
for 'open'. Update function comment.
* vtv_rts.cc (log_memory_protection_data): Remove %d from file name.
On Wed, Aug 14, 2013 at 8:33 AM, Florian Weimer <fwei...@redhat.com> wrote:
> On 08/12/2013 07:07 PM, Caroline Tice wrote:
>
>> The feature is supposed to be active in production code (like the
>> stack protector).
>
>
> Okay, and it makes sense to enable this in programs that run with different
> privileges than the invoking user.
>
> If a trust transition occurred during the past execve, libvtv must not use
> the current directory to store files, or derive file paths from environment
> variables. Using shared directories such as /tmp is also tricky. (The
> current libvtv version in trunk has an arbitrary file creation vulnerability
> because of the hardcoded path in /tmp.)
>
> Realistically, I think libvtv can only log to standard error (or perhaps
> syslog/the journal) if it detects it runs with elevated privileges.
>
> One way to achieve that is to return dup(2) as the file descriptor if
> logs_dir is NULL (after changing getenv to secure_getenv), instead of
> setting logs_dir to ".". This means that unless the environment variable is
> set, nothing would be logged to disk. I'm not sure if this what you want.
> But it follows the principle of least surprise, though. Creating log files
> in strange places because of a crash is unusual.
>
> If you insist on dumping stuff into the current directory by default, you'll
> have to use getauxval(AT_SECURE) (glibc 2.17 and later),
> __libc_enable_secure (some glibc systems, but not Fedora and the ELs),
> issetugid (Solaris and the BSDs) or an explicit comparison between
> getuid()/geteuid() and getgid()/getegid() (all the rest, slightly unsafe on
> Linux). I can write down this approach in more detail, it should probably
> go into the Fedora security guide anyway.
>
> I also noticed this in log_memory_protection_data:
>
> log_fd = __vtv_open_log ("vtv_memory_protection_data_%d.log");
>
> The "_%d" should probably be dropped because the argument is not a format
> string.
>
>
>
> --
> Florian Weimer / Red Hat Product Security Team
vtv-update-tmpdir.patch
Description: Binary data
