#7911 [installer] audit & fix temp file management due to fs.protected_regular=1
Opened 3 years ago by fcami. Modified 3 years ago

Fedora 30's systemd sets fs.protected_regular=1.

As per the kernel documentation:


This protection is similar to protected_fifos, but it
avoids writes to an attacker-controlled regular file, where a program
expected to create one.

When set to "1" don't allow O_CREAT open on regular files that we
don't own in world writable sticky directories, unless they are
owned by the owner of the directory.

There are 44 calls to os.chown in ipaserver/install and some lead to recent issues.

Goal: audit all calls to os.chown and make sure these happen after the file is created and closed.
Secondary goals: avoid harcoding temporary file names.

I propose that we take this opportunity to refactor how IPA handles temporary files and directories. In my opinion any code that uses tempfile.mkstemp() followed by os.close(fd) is broken. Some may even be a potential security issue. Even tempfile.NamedTemporaryFile(delete=False) is problematic, because it potentially leaves the file behind.

A propose to have one temporary directory for all files private to the installer and an additional directory for each service that runs under a different user account, e.g. Dogtag with pkiuser. The directory is automatically removed by an atexit handler so we don't leave any directory behind.

For shared file the code should no longer set both user and group. For example pkiuser:pkiuser with 0640 files are no longer accessible by root except if root has DAC override permission. It's better to give it pkiuser:root with permissions 0660 so both the Dogtag process and root user can still read and write to the file. The same is true for directories except the directory needs 0770.

We don't need to pass the directory name around. A process global temp file handler is easier to use.

Login to comment on this ticket.