Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Add support for running hooks. #64
Conversation
zyga
reviewed
Jun 30, 2016
| @@ -54,19 +54,19 @@ int sc_main(int argc, char **argv) | ||
| if (argc < NR_ARGS + 1) | ||
| die("Usage: %s <security-tag> <binary>", argv[0]); | ||
| - const char *appname = argv[1]; | ||
| - debug("appname is %s", appname); | ||
| + const char *executable_name = argv[1]; |
zyga
Jun 30, 2016
Collaborator
If you are changing this, can you just call it as such, and remove aa_profile entirely as it was always the same thing
zyga
reviewed
Jun 30, 2016
| + // - <hookname must start with a lowercase letter, then may | ||
| + // contain lowercase letters and '-' | ||
| + const char *whitelist_re = | ||
| + "^snap\\.[a-z](-?[a-z0-9])*\\.([a-zA-Z0-9](-?[a-zA-Z0-9])*|hook\\.[a-z](-?[a-z])*)$"; |
zyga
reviewed
Jun 30, 2016
| + | ||
| +static void test_verify_executable_name() | ||
| +{ | ||
| + // First, test the names we know are good |
zyga
reviewed
Jun 30, 2016
| + g_assert_true(verify_executable_name("snap.f00.bar-baz1")); | ||
| + g_assert_true(verify_executable_name("snap.foo.hook.bar")); | ||
| + g_assert_true(verify_executable_name("snap.foo.hook.bar-baz")); | ||
| + |
zyga
Jun 30, 2016
Collaborator
Can you add a canary test for "snap.name". At some point we may decide to abbreviate the security tag and this would be a nice test to catch that change.
zyga
reviewed
Jun 30, 2016
| - const char *whitelist_re = "^snap\\.[a-z][a-z0-9-]*\\.[a-zA-Z0-9-]+$"; | ||
| + // - <appname> may contain alphanumerics and '-' | ||
| + // - <hookname must start with a lowercase letter, then may | ||
| + // contain lowercase letters and '-' |
zyga
reviewed
Jun 30, 2016
| regex_t re; | ||
| if (regcomp(&re, whitelist_re, REG_EXTENDED | REG_NOSUB) != 0) | ||
| die("can not compile regex %s", whitelist_re); | ||
| - int status = regexec(&re, appname, 0, NULL, 0); |
zyga
Jun 30, 2016
Collaborator
Just thinking about something nasty being done. Can we require that total length of security_tag be smaller than some sane default. I don't want to let people explode the system by installing a snap with a long app name that is expensive to check.
If there is something in snapd, let's use that limit, if not then let's make up one now.
@jdstrand ^^ FYI
kyrofa
Jun 30, 2016
•
Edited 1 time
-
kyrofa
Jun 30, 2016
Member
-
Jun 30, 2016
I don't know of a limit imposed by snapd, so throw out a number! Though that may be something for another PR.
jdstrand
Jun 30, 2016
Contributor
There are practical limits to how long this can be and the review tools enforce this. The max length enforced by the store is '100' for the full security label ('snap.$snap.$app').
zyga
Jun 30, 2016
Collaborator
This can go in as there wasn't any check around there before. I can take care of that.
|
+1, thanks for working on this |
kyrofa commentedJun 30, 2016
•
Edited 1 time
-
kyrofa
Jun 30, 2016
This involves a move away from "appname" to "security tag".