------------------------------------------------------------ revno: 1426 committer: James Hunt <[email protected]> branch nick: upstart-setenv+getenv timestamp: Thu 2013-01-31 17:23:55 +0000 message: * init/control.c: - Use control_check_permission() rather than control_get_origin_uid() directly. * init/control.h: Prototypes. * init/job_class.c: Change calls to job_class_environment_init() to asserts as the former only needs to be called once. * init/main.c: main(): Make job_class_environment_init() call as early as possible. * init/tests/test_event.c: main(): Call job_class_environment_init(). * util/tests/test_initctl.c: - test_default_job_env(): - Set TERM and PATH if not set. - Check line counts before checking expected output. - test_clear_job_env(): - Make use of TEST_INITCTL_DEFAULT_PATH. modified: ChangeLog init/control.c init/control.h init/job_class.c init/main.c init/tests/test_event.c util/tests/test_initctl.c
-- lp:upstart https://code.launchpad.net/~upstart-devel/upstart/trunk Your team Upstart Reviewers is subscribed to branch lp:upstart. To unsubscribe from this branch go to https://code.launchpad.net/~upstart-devel/upstart/trunk/+edit-subscription
=== modified file 'ChangeLog' --- ChangeLog 2013-01-31 16:51:49 +0000 +++ ChangeLog 2013-01-31 17:23:55 +0000 @@ -1,3 +1,22 @@ +2013-01-31 James Hunt <[email protected]> + + * init/control.c: + - Use control_check_permission() rather than + control_get_origin_uid() directly. + * init/control.h: Prototypes. + * init/job_class.c: Change calls to job_class_environment_init() + to asserts as the former only needs to be called once. + * init/main.c: main(): Make job_class_environment_init() call as + early as possible. + * init/tests/test_event.c: main(): Call + job_class_environment_init(). + * util/tests/test_initctl.c: + - test_default_job_env(): + - Set TERM and PATH if not set. + - Check line counts before checking expected output. + - test_clear_job_env(): + - Make use of TEST_INITCTL_DEFAULT_PATH. + 2013-01-30 James Hunt <[email protected]> * TESTING.sessions: Removed as basic sessions have now gone. === modified file 'init/control.c' --- init/control.c 2013-01-31 16:51:49 +0000 +++ init/control.c 2013-01-31 17:23:55 +0000 @@ -1184,7 +1184,6 @@ const char *var, int replace) { - uid_t uid, origin_uid; Session *session; Job *job = NULL; char *job_name = NULL; @@ -1203,7 +1202,14 @@ } else if (getpid () == 1) { nih_dbus_error_raise_printf ( DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", - _("Not permissable to modify PID 1 job environment")); + _("Not permissible to modify PID 1 job environment")); + return -1; + } + + if (! control_check_permission (message)) { + nih_dbus_error_raise_printf ( + DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", + _("You do not have permission to modify job environment")); return -1; } @@ -1214,8 +1220,6 @@ return -1; } - uid = getuid (); - /* Get the relevant session */ session = session_from_dbus (NULL, message); @@ -1227,17 +1231,6 @@ return 0; } - /* Disallow users from changing Upstarts environment, unless they happen to - * own this process (which they may do in the test scenario and - * when running Upstart as a non-privileged user). - */ - if (control_get_origin_uid (message, &origin_uid) && origin_uid != uid) { - nih_dbus_error_raise_printf ( - DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", - _("You do not have permission to modify job environment")); - return -1; - } - /* Lookup the job */ control_get_job (session, job, job_name, instance); @@ -1290,7 +1283,6 @@ char * const *job_details, const char *name) { - uid_t uid, origin_uid; Session *session; Job *job = NULL; char *job_name = NULL; @@ -1300,6 +1292,13 @@ nih_assert (job_details); nih_assert (name); + if (! control_check_permission (message)) { + nih_dbus_error_raise_printf ( + DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", + _("You do not have permission to modify job environment")); + return -1; + } + if (job_details[0]) { job_name = job_details[0]; @@ -1308,7 +1307,7 @@ } else if (getpid () == 1) { nih_dbus_error_raise_printf ( DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", - _("Not permissable to modify PID 1 job environment")); + _("Not permissible to modify PID 1 job environment")); return -1; } @@ -1319,8 +1318,6 @@ return -1; } - uid = getuid (); - /* Get the relevant session */ session = session_from_dbus (NULL, message); @@ -1332,17 +1329,6 @@ return 0; } - /* Disallow users from changing Upstarts environment, unless they happen to - * own this process (which they may do in the test scenario and - * when running Upstart as a non-privileged user). - */ - if (control_get_origin_uid (message, &origin_uid) && origin_uid != uid) { - nih_dbus_error_raise_printf ( - DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", - _("You do not have permission to modify job environment")); - return -1; - } - /* Lookup the job */ control_get_job (session, job, job_name, instance); @@ -1392,7 +1378,6 @@ const char *name, char **value) { - uid_t uid, origin_uid; Session *session; const char *tmp; Job *job = NULL; @@ -1402,6 +1387,13 @@ nih_assert (message != NULL); nih_assert (job_details); + if (! control_check_permission (message)) { + nih_dbus_error_raise_printf ( + DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", + _("You do not have permission to query job environment")); + return -1; + } + if (job_details[0]) { job_name = job_details[0]; @@ -1416,8 +1408,6 @@ return -1; } - uid = getuid (); - /* Get the relevant session */ session = session_from_dbus (NULL, message); @@ -1429,17 +1419,6 @@ return 0; } - /* Disallow users from querying Upstarts environment, unless they happen to - * own this process (which they may do in the test scenario and - * when running Upstart as a non-privileged user). - */ - if (control_get_origin_uid (message, &origin_uid) && origin_uid != uid) { - nih_dbus_error_raise_printf ( - DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", - _("You do not have permission to modify job environment")); - return -1; - } - /* Lookup the job */ control_get_job (session, job, job_name, instance); @@ -1495,7 +1474,6 @@ char * const *job_details, char ***env) { - uid_t uid, origin_uid; Session *session; Job *job = NULL; char *job_name = NULL; @@ -1505,6 +1483,13 @@ nih_assert (job_details); nih_assert (env); + if (! control_check_permission (message)) { + nih_dbus_error_raise_printf ( + DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", + _("You do not have permission to query job environment")); + return -1; + } + if (job_details[0]) { job_name = job_details[0]; @@ -1519,22 +1504,9 @@ return -1; } - uid = getuid (); - /* Get the relevant session */ session = session_from_dbus (NULL, message); - /* Disallow users from changing Upstarts environment, unless they happen to - * own this process (which they may do in the test scenario and - * when running Upstart as a non-privileged user). - */ - if (control_get_origin_uid (message, &origin_uid) && origin_uid != uid) { - nih_dbus_error_raise_printf ( - DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", - _("You do not have permission to query job environment")); - return -1; - } - /* Lookup the job */ control_get_job (session, job, job_name, instance); @@ -1573,7 +1545,6 @@ NihDBusMessage *message, char * const *job_details) { - uid_t uid, origin_uid; Session *session; Job *job = NULL; char *job_name = NULL; @@ -1590,11 +1561,16 @@ } else if (getpid () == 1) { nih_dbus_error_raise_printf ( DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", - _("Not permissable to modify PID 1 job environment")); + _("Not permissible to modify PID 1 job environment")); return -1; } - uid = getuid (); + if (! control_check_permission (message)) { + nih_dbus_error_raise_printf ( + DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", + _("You do not have permission to modify job environment")); + return -1; + } /* Verify that job name is valid */ if (job_name && ! strlen (job_name)) { @@ -1614,17 +1590,6 @@ return 0; } - /* Disallow users from modifying Upstarts environment, unless they happen to - * own this process (which they may do in the test scenario and - * when running Upstart as a non-privileged user). - */ - if (control_get_origin_uid (message, &origin_uid) && origin_uid != uid) { - nih_dbus_error_raise_printf ( - DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", - _("You do not have permission to modify job environment")); - return -1; - } - /* Lookup the job */ control_get_job (session, job, job_name, instance); === modified file 'init/control.h' --- init/control.h 2013-01-31 16:51:49 +0000 +++ init/control.h 2013-01-31 17:23:55 +0000 @@ -179,6 +179,19 @@ char ***env) __attribute__ ((warn_unused_result)); +int +control_reset_env (void *data, + NihDBusMessage *message, + char * const *job_details) + __attribute__ ((warn_unused_result)); + +int +control_unset_env (void *data, + NihDBusMessage *message, + char * const *job_details, + const char *name) + __attribute__ ((warn_unused_result)); + NIH_END_EXTERN #endif /* INIT_CONTROL_H */ === modified file 'init/job_class.c' --- init/job_class.c 2013-01-31 16:51:49 +0000 +++ init/job_class.c 2013-01-31 17:23:55 +0000 @@ -152,8 +152,7 @@ job_class_environment_set (const char *var, int replace) { nih_assert (var); - - job_class_environment_init (); + nih_assert (job_environ); if (! environ_add (&job_environ, NULL, NULL, replace, var)) return -1; @@ -174,8 +173,7 @@ job_class_environment_unset (const char *name) { nih_assert (name); - - job_class_environment_init (); + nih_assert (job_environ); if (! environ_remove (&job_environ, NULL, NULL, name)) return -1; @@ -196,7 +194,7 @@ char ** job_class_environment_get_all (const void *parent) { - job_class_environment_init (); + nih_assert (job_environ); return nih_str_array_copy (parent, NULL, job_environ); } @@ -217,8 +215,7 @@ job_class_environment_get (const char *name) { nih_assert (name); - - job_class_environment_init (); + nih_assert (job_environ); return environ_get (job_environ, name); } @@ -615,8 +612,7 @@ char **env; nih_assert (class != NULL); - - job_class_environment_init (); + nih_assert (job_environ); env = nih_str_array_new (parent); if (! env) === modified file 'init/main.c' --- init/main.c 2013-01-31 16:51:49 +0000 +++ init/main.c 2013-01-31 17:23:55 +0000 @@ -538,6 +538,8 @@ nih_free (dirs); } + job_class_environment_init (); + conf_reload (); /* Create a listening server for private connections. */ @@ -631,8 +633,6 @@ if (disable_sessions) nih_debug ("Sessions disabled"); - job_class_environment_init (); - /* Set us as the child subreaper. * This ensures that even when init doesn't run as PID 1, it'll always be * the ultimate parent of everything it spawns. */ === modified file 'init/tests/test_event.c' --- init/tests/test_event.c 2012-11-27 23:49:12 +0000 +++ init/tests/test_event.c 2013-01-31 17:23:55 +0000 @@ -2047,6 +2047,8 @@ /* run tests in legacy (pre-session support) mode */ setenv ("UPSTART_NO_SESSIONS", "1", 1); + job_class_environment_init (); + test_new (); test_block (); test_unblock (); === modified file 'util/tests/test_initctl.c' --- util/tests/test_initctl.c 2013-01-31 16:51:49 +0000 +++ util/tests/test_initctl.c 2013-01-31 17:23:55 +0000 @@ -60,6 +60,14 @@ #define BUFFER_SIZE 1024 +/* A 'reasonable' path, but which also contains a marker at the end so + * we know when we're looking at a PATH these tests have set. + */ +#define TEST_INITCTL_DEFAULT_PATH "/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/bin:/sbin:/wibble" + +/* Default value for TERM if not already set */ +#define TEST_INITCTL_DEFAULT_TERM "linux" + /** * WAIT_FOR_UPSTART: * @@ -15604,15 +15612,35 @@ assert (dbus_pid); /*******************************************************************/ + /* Ensure basic variables are set in the current environment */ + + if (! getenv ("TERM")) { + fprintf (stderr, "WARNING: setting TERM to '%s' as not set\n", + TEST_INITCTL_DEFAULT_TERM); + assert0 (setenv ("TERM", TEST_INITCTL_DEFAULT_TERM, 1)); + } + + if (! getenv ("PATH")) { + fprintf (stderr, "WARNING: setting PATH to '%s' as not set\n", + TEST_INITCTL_DEFAULT_PATH); + assert0 (setenv ("PATH", TEST_INITCTL_DEFAULT_PATH, 1)); + } + + cmd = nih_sprintf (NULL, "%s reset-env 2>&1", get_initctl ()); + TEST_NE_P (cmd, NULL); + RUN_COMMAND (NULL, cmd, &output, &line_count); + assert0 (line_count); + + /*******************************************************************/ TEST_FEATURE ("ensure list-env returns default environment"); cmd = nih_sprintf (NULL, "%s list-env 2>&1", get_initctl ()); TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); + TEST_EQ (line_count, 2); TEST_STR_MATCH (output[0], "PATH=*"); TEST_STR_MATCH (output[1], "TERM=*"); - TEST_EQ (line_count, 2); nih_free (output); /*******************************************************************/ @@ -15622,9 +15650,9 @@ TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); + TEST_EQ (line_count, 2); TEST_STR_MATCH (output[0], "PATH=*"); TEST_STR_MATCH (output[1], "TERM=*"); - TEST_EQ (line_count, 2); nih_free (output); /*******************************************************************/ @@ -15634,10 +15662,6 @@ TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - /* don't check the actual value (in case user has changed it from - * default value when compiling), just see if it matches a - * reasonable pattern. - */ TEST_EQ_STR (output[0], getenv ("TERM")); TEST_EQ (line_count, 1); nih_free (output); @@ -15649,10 +15673,6 @@ TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - /* don't check the actual value (in case user has changed it from - * default value when compiling), just see if it matches a - * reasonable pattern. - */ TEST_EQ_STR (output[0], getenv ("TERM")); TEST_EQ (line_count, 1); nih_free (output); @@ -15664,12 +15684,8 @@ TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - /* don't check the actual value (in case user has changed it from - * default value when compiling), just see if it matches a - * reasonable pattern. - */ - TEST_STR_MATCH (output[0], "[a-zA-Z/:][a-zA-Z0-9/:]*"); TEST_EQ (line_count, 1); + TEST_EQ_STR (output[0], getenv ("PATH")); nih_free (output); /*******************************************************************/ @@ -15679,12 +15695,8 @@ TEST_NE_P (cmd, NULL); RUN_COMMAND (NULL, cmd, &output, &line_count); - /* don't check the actual value (in case user has changed it from - * default value when compiling), just see if it matches a - * reasonable pattern. - */ - TEST_STR_MATCH (output[0], "[a-zA-Z/:][a-zA-Z0-9/:]*"); TEST_EQ (line_count, 1); + TEST_EQ_STR (output[0], getenv ("PATH")); nih_free (output); /*******************************************************************/ @@ -15790,6 +15802,7 @@ nih_local char *cmd = NULL; char **output; nih_local char *logfile = NULL; + nih_local char *contents = NULL; size_t line_count; FILE *fi; @@ -15822,9 +15835,12 @@ * Add a silly entry at the end so we can check our version has * been set. */ - CREATE_FILE (confdir, "empty-env.conf", - "env PATH=/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/sbin:/sbin:/bin:/wibble\n" - "exec env"); + contents = nih_sprintf (NULL, + "env PATH=%s\n" + "exec env", TEST_INITCTL_DEFAULT_PATH); + TEST_NE_P (contents, NULL); + + CREATE_FILE (confdir, "empty-env.conf", contents); cmd = nih_sprintf (NULL, "%s start empty-env 2>&1", get_initctl ()); TEST_NE_P (cmd, NULL);
-- upstart-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/upstart-devel
