9

In order to protect an application from begin used wrongly, I'm trying to check that its configuration files have correct permissions, so that the application can trust the content of the files not being modified by someone else.

I believe the following rules are corrects:

  • the file must not be writable by others
  • the file must be owned by a trusted user/group: root or
  • the file must be owned by the effective user/group running the application (think of setuid program)

Here an example:

#include <stdio.h>
#include <unistd.h>
#include <sys/stat.h>

#include <string.h>
#include <errno.h>

static
int is_secure(const char *name)
{
    struct stat st;

    uid_t euid = geteuid();
    gid_t egid = getegid();

    if (stat(name, &st) != 0) {
        int err = errno;
        fprintf(stderr, "can't stat() '%s': %d (%s)\n", name, err, strerror(err));
        return 0;
    }

    /* writable by other: unsecure */
    if ((st.st_mode & S_IWOTH) != 0) {
        return 0;
    }

    /* not owned by group root and not owned by effective group: unsecure */
    if (st.st_gid != 0 && st.st_gid != egid) {
        return 0;
    }

    /* not owned by user root and not owned by effective user: unsecure */
    if (st.st_uid != 0 && st.st_uid != euid) {
        return 0;
    }

    return 1;
}

int
main(int argc, char *argv[])
{
    int i;

    for(i = 1; i < argc; i++) {
        printf("'%s' : %s\n", argv[i], is_secure(argv[i]) ? "sure" : "unsure");
    }

    return 0;
}

Since I'm not sure about my assumptions, can someone check if I leave some loophole in the file permissions check.

Update

sudo has a function for that: sudo_secure_path, it only check for one uid/gid, but it take care of checking for group write bit.

Regards.

Yann Droneaud
  • 5,277
  • 1
  • 23
  • 39
  • 1
    Consider the information in [Secure access to files in a directory identified by an environment variable](http://stackoverflow.com/questions/196244/secure-access-to-files-in-a-directory-identified-by-an-environment-variable). The scenario there is more complex. However, some of the ideas, notably that the path leading to the file also needs to be secure, is valid. – Jonathan Leffler Jul 26 '13 at 04:21
  • 1
    Jonathan is right. Every element of the path leading to the file needs to be writable only by trusted users for full safety. Otherwise, strange scenarios like moving a component of the path and replacing it with a pointer to a different but similar directory structure could happen, bypassing your file entirely but having the same effect as rewriting the entire configuration file. Depending on what's in the configuration file, it could help attackers break in or at least do a more informed DOS attack, so it might even be good to have the file readable only by its owner / trusted users. – Joseph Myers Jul 26 '13 at 05:26
  • @JosephMyers Thanks. I should have produced a more complete example: this code is used to check the directory and each files located in it. – Yann Droneaud Jul 27 '13 at 07:31
  • @JonathanLeffler Thanks. I should have produced a more complete example: this code is used to check the directory and each files located in it. – Yann Droneaud Jul 27 '13 at 07:31
  • It is assumed the parent directory of the configuration directory is secure. Additionally to improve security, functions `openat()`, `fstatat()`, `fstat()` are used to ensure that file from the checked directory are used and not files from another directory, moved behind the back of the application. Using O_NOFOLLOW when opening files/directory could prevent some symlink attack. – Yann Droneaud Jul 27 '13 at 07:34

2 Answers2

8

Your rules and your code look correct to me, although you should be aware of the following security risks that could still affect your implementation.

  1. An attacker with physical access to the machine or NFS/SMB access could mount the file system with a box that has root privileges, and then modify your file.
  2. A vulnerability in another program being run as either the trusted user or root could allow that program to be exploited to modify your file.
  3. All it would take to break your security check would be a careless user or sys-admin that messes up the privilege settings of the file. I've seen this happen during backups and copies to thumb drives, etc.
  4. Also make sure the file is not executable. I can't think of an instance where this could be exploited on a config file, but the general rule with security is don't give any privileges that aren't required for the job.

As you can see these are not issues under control of your code. Therefore, you should make sure you client is aware of these risks before assuring them of the non-tamperability of the config file.

Freedom_Ben
  • 11,247
  • 10
  • 69
  • 89
  • 1
    Thanks for your answer. Your comments are very interesting. It's refreshing a lot about whole system security. My threat model/security needs, is/are very narrow. I just want to be confident the configuration is not under control of non-trustable user (another user). Ensuring the file content is correct is outside of the scope of the question, it's another, whole system, problem. – Yann Droneaud May 24 '13 at 08:00
7

I believe you also want to check permissions on the directory.

A user would be able to mv another file belonging to the correct user to replace this one, if they are allowed to write to the directory.

Something like:

sudo touch foo.conf
sudo touch foo.conf-insecure-sample
mv -f foo.conf-insecure-sample foo.conf
Freedom_Ben
  • 11,247
  • 10
  • 69
  • 89
CMoi
  • 846
  • 4
  • 9
  • Good catch. It wasn't shown in the code presented here, but the directory is also checked before the individual files in the real code. BTW it doesn't ensure that the parent directory or any previous levels is correct: other level are not in the scope of the application, its system related. – Yann Droneaud May 24 '13 at 13:10