Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
many: snapctl outside hooks v2 #3328
Conversation
stolowski
added some commits
Jan 9, 2017
All checks have failed
All checks have failed
All checks have failed
| @@ -111,6 +111,14 @@ func (s *setCommand) setConfigSetting(context *hookstate.Context) error { | ||
| tr.Set(s.context().SnapName(), key, value) | ||
| } | ||
| + if context.IsEphemeral() { |
pedronis
May 16, 2017
Contributor
this still look like the wrong place for this, it seems something to do in api
pedronis
changed the title from
Snapctl outside hooks v2
to
many: snapctl outside hooks v2
May 16, 2017
All checks have failed
zyga
requested changes
May 16, 2017
Initial feedback
Marking as blocked due to parts in snap-confine but will gladly help on iterating on those.
| @@ -214,6 +214,8 @@ snap_confine_snap_confine_SOURCES = \ | ||
| snap-confine/snap-confine-args.c \ | ||
| snap-confine/snap-confine-args.h \ | ||
| snap-confine/snap-confine.c \ | ||
| + snap-confine/context-support.c \ |
| @@ -48,6 +48,34 @@ bool verify_security_tag(const char *security_tag) | ||
| return (status == 0); | ||
| } | ||
| +bool verify_hook_security_tag_name(const char *security_tag) |
zyga
May 16, 2017
Contributor
We have a NACK on using regular expressions. This needs to be re-written using other logic.
| + return (status == 0); | ||
| +} | ||
| + | ||
| +bool verify_snap_name(const char *name) |
zyga
May 16, 2017
Contributor
We already have that but it was NACKed. I think I have a better version without the regular expression anymore. Let me find that.
| @@ -45,4 +45,7 @@ void sc_snap_name_validate(const char *snap_name, struct sc_error **errorp); | ||
| bool verify_security_tag(const char *security_tag); | ||
| +bool verify_hook_security_tag_name(const char *security_tag); |
| + * You should have received a copy of the GNU General Public License | ||
| + * along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
| + * | ||
| + */ |
zyga
May 16, 2017
Contributor
You must include your own header file first. Otherwise you risk having a header that doesn't compile.
| +#include "context-support.h" | ||
| + | ||
| +#include <string.h> | ||
| +#include <unistd.h> |
| + | ||
| +#define CONTEXT_DIR "/var/lib/snapd/context" | ||
| + | ||
| +char *sc_nonfatal_context_get_from_snapd(const char *snap_name, |
zyga
May 16, 2017
Contributor
Ever since we have sc_error the nonfatal naming feels wrong. Please drop this part.
| @@ -116,6 +131,9 @@ int main(int argc, char **argv) | ||
| debug | ||
| ("skipping sandbox setup, classic confinement in use"); | ||
| } else { | ||
| + if (snap_name == NULL) { |
zyga
May 16, 2017
Contributor
Can you not check this here. We should have enough checks in main early on to ensure this never happens. If those are missing add them early there.
| @@ -201,7 +219,9 @@ int main(int argc, char **argv) | ||
| #if 0 | ||
| setup_user_xdg_runtime_dir(); | ||
| #endif | ||
| - | ||
| + if (snap_context != NULL) { | ||
| + sc_maybe_set_context_environment(snap_context); |
zyga
May 16, 2017
Contributor
Can this just do the check internally? Then the maybe part makes sense.
| @@ -151,6 +152,7 @@ func SetRootDir(rootdir string) { | ||
| SnapSocket = filepath.Join(rootdir, "/run/snapd-snap.socket") | ||
| SnapAssertsDBDir = filepath.Join(rootdir, snappyDir, "assertions") | ||
| + SnapContextDir = filepath.Join(rootdir, snappyDir, "context") |
Conan-Kudo
May 16, 2017
•
Edited 1 time
-
Conan-Kudo
May 16, 2017
Contributor
-
May 16, 2017
Is this in /var/lib/snapd/? If it is, it's already covered: https://github.com/snapcore/snapd/blob/master/data/selinux/snappy.fc#L40
If it's in /var/snap/, then it's also covered: https://github.com/snapcore/snapd/blob/master/data/selinux/snappy.fc#L41
| @@ -69,13 +73,31 @@ func (s *linkSnapSuite) SetUpTest(c *C) { | ||
| snapstate.SetSnapManagerBackend(s.snapmgr, s.fakeBackend) | ||
| - s.reset = snapstate.MockReadInfo(s.fakeBackend.ReadInfo) | ||
| + reset1 := snapstate.MockReadInfo(s.fakeBackend.ReadInfo) |
| @@ -41,6 +42,9 @@ type mountSnapSuite struct { | ||
| var _ = Suite(&mountSnapSuite{}) | ||
| func (s *mountSnapSuite) SetUpTest(c *C) { | ||
| + oldDir := dirs.SnapContextDir | ||
| + dirs.SnapContextDir = c.MkDir() |
zyga
May 16, 2017
Contributor
Since this happens all over the place. What happens if you don't do it?
|
Closing to prepare this PR once again with a squashed history as this one accumulated too much noise in the last few months. |
stolowski commentedMay 16, 2017
Support running snapctl outside of hooks by the apps from the snaps. This is realized by generating a unique SNAP_CONTEXT cookie-file on per-snap basis when the snap is installed, and passing it via the env in snap-confine.
NOTE: this branch was first proposed a few months, but was reworked a lot since then and I tried to address all previous comments. For old comments see #2644.