Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/libsnap: add simplified feature flag checker #6116

Merged
merged 2 commits into from Nov 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion cmd/Makefile.am
Expand Up @@ -96,6 +96,8 @@ libsnap_confine_private_a_SOURCES = \
libsnap-confine-private/error.h \
libsnap-confine-private/fault-injection.c \
libsnap-confine-private/fault-injection.h \
libsnap-confine-private/feature.c \
libsnap-confine-private/feature.h \
libsnap-confine-private/locking.c \
libsnap-confine-private/locking.h \
libsnap-confine-private/mount-opt.c \
Expand Down Expand Up @@ -125,15 +127,16 @@ libsnap_confine_private_unit_tests_SOURCES = \
libsnap-confine-private/cleanup-funcs-test.c \
libsnap-confine-private/error-test.c \
libsnap-confine-private/fault-injection-test.c \
libsnap-confine-private/feature-test.c \
libsnap-confine-private/locking-test.c \
libsnap-confine-private/mount-opt-test.c \
libsnap-confine-private/mountinfo-test.c \
libsnap-confine-private/privs-test.c \
libsnap-confine-private/secure-getenv-test.c \
libsnap-confine-private/snap-test.c \
libsnap-confine-private/string-utils-test.c \
libsnap-confine-private/test-utils.c \
libsnap-confine-private/test-utils-test.c \
libsnap-confine-private/test-utils.c \
libsnap-confine-private/unit-tests-main.c \
libsnap-confine-private/unit-tests.c \
libsnap-confine-private/unit-tests.h \
Expand Down
86 changes: 86 additions & 0 deletions cmd/libsnap-confine-private/feature-test.c
@@ -0,0 +1,86 @@
/*
* Copyright (C) 2018 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

#include "feature.h"
#include "feature.c"

#include <limits.h>

#include <glib.h>

#include "string-utils.h"
#include "test-utils.h"

static char *sc_testdir(void)
{
char *d = g_dir_make_tmp(NULL, NULL);
g_assert_nonnull(d);
g_test_queue_free(d);
g_test_queue_destroy((GDestroyNotify) rm_rf_tmp, d);
return d;
}

// Set the feature flag directory to given value, useful for cleanup handlers.
static void set_feature_flag_dir(const char *dir)
{
feature_flag_dir = dir;
}

// Mock the location of the feature flag directory.
static void sc_mock_feature_flag_dir(const char *d)
{
g_test_queue_destroy((GDestroyNotify) set_feature_flag_dir,
(void *)feature_flag_dir);
set_feature_flag_dir(d);
}

static void test_feature_enabled__missing_dir(void)
{
const char *d = sc_testdir();
char subd[PATH_MAX];
sc_must_snprintf(subd, sizeof subd, "%s/absent", d);
sc_mock_feature_flag_dir(subd);
g_assert(!sc_feature_enabled(SC_PER_USER_MOUNT_NAMESPACE));
}

static void test_feature_enabled__missing_file(void)
{
const char *d = sc_testdir();
sc_mock_feature_flag_dir(d);
g_assert(!sc_feature_enabled(SC_PER_USER_MOUNT_NAMESPACE));
}

static void test_feature_enabled__present_file(void)
{
const char *d = sc_testdir();
sc_mock_feature_flag_dir(d);
char pname[PATH_MAX];
sc_must_snprintf(pname, sizeof pname, "%s/per-user-mount-namespace", d);
g_file_set_contents(pname, "", -1, NULL);

g_assert(sc_feature_enabled(SC_PER_USER_MOUNT_NAMESPACE));
}

static void __attribute__ ((constructor)) init(void)
{
g_test_add_func("/feature/missing_dir",
test_feature_enabled__missing_dir);
g_test_add_func("/feature/missing_file",
test_feature_enabled__missing_file);
g_test_add_func("/feature/present_file",
test_feature_enabled__present_file);
}
62 changes: 62 additions & 0 deletions cmd/libsnap-confine-private/feature.c
@@ -0,0 +1,62 @@
/*
* Copyright (C) 2018 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

#define _GNU_SOURCE

#include "feature.h"

#include <errno.h>
#include <fcntl.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>

#include "cleanup-funcs.h"
#include "utils.h"

static const char *feature_flag_dir = "/var/lib/snapd/features";

bool sc_feature_enabled(sc_feature_flag flag)
{
const char *file_name;
switch (flag) {
case SC_PER_USER_MOUNT_NAMESPACE:
file_name = "per-user-mount-namespace";
break;
default:
die("unknown feature flag code %d", flag);
}

int dirfd SC_CLEANUP(sc_cleanup_close) = -1;
dirfd = open(feature_flag_dir, O_CLOEXEC | O_DIRECTORY | O_NOFOLLOW | O_PATH);
if (dirfd < 0 && errno == ENOENT) {
return false;
}
if (dirfd < 0) {
die("cannot open path %s", feature_flag_dir);
}

struct stat file_info;
if (fstatat(dirfd, file_name, &file_info, AT_SYMLINK_NOFOLLOW) < 0) {
if (errno == ENOENT) {
return false;
}
die("cannot inspect file %s/%s", feature_flag_dir, file_name);
}

return S_ISREG(file_info.st_mode);
}
35 changes: 35 additions & 0 deletions cmd/libsnap-confine-private/feature.h
@@ -0,0 +1,35 @@
/*
* Copyright (C) 2018 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

#ifndef SNAP_CONFINE_FEATURE_H
#define SNAP_CONFINE_FEATURE_H

#include <stdbool.h>

typedef enum sc_feature_flag {
SC_PER_USER_MOUNT_NAMESPACE,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you change the PR maybe SC_FEATURE_PER_USER_MOUNT_NAMESPACE (i.e. prefix with SC_FEATURE_) but not really important.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the SC_FEATURE idea. I'll change this in the next one.

} sc_feature_flag;

/**
* sc_feature_enabled returns true if a given feature flag has been activated
* by the user via "snap set core experimental.xxx=true". This is determined by
* testing the presence of a file in /var/lib/snapd/features/ that is named
* after the flag name.
**/
bool sc_feature_enabled(sc_feature_flag flag);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would make sense to simply pass feature names here that directly map to filenames under features directory (and we would have const char* constants for them). That way adding a new feature flag would reduce the scope of changes (no need to touch sc_feature_enabled). Not a biggie, just a question for consideration.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this but I decided to use an enum instead. There are only so many feature flags and it is easier to cause a typo this way (because you can always pass a string manually). This has the advantage that the code knows the set of flags that exist so it cannot be fooled.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for typos


#endif