services/mgmt/device/impl: per-instance ACLs control logs access
This change extends per-instance ACLs to provide access control to
logs under the __debug space of a device manager invoked application
and adds additional testing to demonstrate globbing and access control
to the entire __debug space.
Change-Id: I8071fa470bd9c001620c3a1e995d0c2e8e5c8963
diff --git a/services/mgmt/device/impl/app_service.go b/services/mgmt/device/impl/app_service.go
index 0087d16..8abc3e9 100644
--- a/services/mgmt/device/impl/app_service.go
+++ b/services/mgmt/device/impl/app_service.go
@@ -662,6 +662,7 @@
return installFrom(overridePackages, installationDir)
}
+// initializeSubAccessLists updates the provided acl for instance-specific ACLs
func (i *appService) initializeSubAccessLists(instanceDir string, blessings []string, acl access.Permissions) error {
for _, b := range blessings {
for _, tag := range access.AllTypicalTags() {
diff --git a/services/mgmt/device/impl/debug_acls_test.go b/services/mgmt/device/impl/debug_acls_test.go
index 56a167d..26136d6 100644
--- a/services/mgmt/device/impl/debug_acls_test.go
+++ b/services/mgmt/device/impl/debug_acls_test.go
@@ -8,7 +8,7 @@
"v.io/v23/naming"
"v.io/v23/security"
"v.io/v23/services/security/access"
- "v.io/v23/vdl"
+ "v.io/v23/verror"
mgmttest "v.io/x/ref/services/mgmt/lib/testutil"
"v.io/x/ref/test/testutil"
@@ -17,15 +17,29 @@
func updateAccessList(t *testing.T, ctx *context.T, blessing, right string, name ...string) {
acl, etag, err := appStub(name...).GetPermissions(ctx)
if err != nil {
- t.Fatalf("GetPermissions(%v) failed %v", name, err)
+ t.Fatalf(testutil.FormatLogLine(2, "GetACL(%v) failed %v", name, err))
}
acl.Add(security.BlessingPattern(blessing), right)
if err = appStub(name...).SetPermissions(ctx, acl, etag); err != nil {
- t.Fatalf("SetPermissions(%v, %v, %v) failed: %v", name, blessing, right, err)
+ t.Fatalf(testutil.FormatLogLine(2, "SetPermissions(%v, %v, %v) failed: %v", name, blessing, right, err))
}
}
-func TestDebugAccessListPropagation(t *testing.T) {
+func mn(base []string, suffix ...string) []string {
+ fn := make([]string, len(base))
+ // Must copy because append will modify the original.
+ copy(fn, base)
+ fn = append(fn, suffix...)
+ return fn
+}
+
+func testAccessFail(t *testing.T, expected verror.ID, ctx *context.T, who string, name ...string) {
+ if _, err := statsStub(mn(name, "stats/system/pid")...).Value(ctx); !verror.Is(err, expected) {
+ t.Fatalf(testutil.FormatLogLine(2, "%s got error %v but expected %v", who, err, expected))
+ }
+}
+
+func TestDebugPermissionsPropagation(t *testing.T) {
cleanup, ctx, sh, envelope, root, helperPath, idp := startupHelper(t)
defer cleanup()
@@ -63,58 +77,69 @@
// Bob permits Alice to read from his app.
updateAccessList(t, bobCtx, "root/alice/$", string(access.Read), appID, bobApp)
- // Confirm that self can access stats name (i.e. apps proxied from
- // the __debug space of the app.
- // TODO(rjkroege): validate each of the services under __debug.
- v, err := statsStub(appID, bobApp, "stats/system/pid").Value(ctx)
- if err != nil {
- t.Fatalf("Value() failed: %v\n", err)
+ // Create some globbing test vectors.
+ globtests := []globTestVector{
+ {naming.Join("dm", "apps", appID, bobApp), "*",
+ []string{"logs", "pprof", "stats"},
+ },
+ {naming.Join("dm", "apps", appID, bobApp, "stats", "system"),
+ "start-time*",
+ []string{"start-time-rfc1123", "start-time-unix"},
+ },
+ {naming.Join("dm", "apps", appID, bobApp, "logs"),
+ "*",
+ []string{
+ "STDERR-<timestamp>",
+ "STDOUT-<timestamp>",
+ "app.INFO",
+ "app.<*>.INFO.<timestamp>",
+ },
+ },
}
- var pid int
- if err := vdl.Convert(&pid, v); err != nil {
- t.Fatalf("pid returned from stats interface is not an int: %v", err)
- }
+ globtestminus := globtests[1:]
+ res := newGlobTestRegexHelper("app")
+
+ // Confirm that self can access __debug names.
+ verifyGlob(t, selfCtx, "app", globtests, res)
+ verifyStatsValues(t, selfCtx, "dm", "apps", appID, bobApp, "stats/system/start-time*")
+ verifyLog(t, selfCtx, "dm", "apps", appID, bobApp, "logs", "*")
+ verifyPProfCmdLine(t, selfCtx, "app", "dm", "apps", appID, bobApp, "pprof")
// Bob has an issue with his app and tries to use the debug output to figure it out.
- if _, err = statsStub(appID, bobApp, "stats/system/pid").Value(bobCtx); err != nil {
- t.Fatalf("Bob couldn't access debug info on his app: ", err)
- }
+ verifyGlob(t, bobCtx, "app", globtests, res)
+ verifyStatsValues(t, bobCtx, "dm", "apps", appID, bobApp, "stats/system/start-time*")
+ verifyLog(t, bobCtx, "dm", "apps", appID, bobApp, "logs", "*")
+ verifyPProfCmdLine(t, bobCtx, "app", "dm", "apps", appID, bobApp, "pprof")
// But Bob can't figure it out and hopes that hackerjoe can debug it.
updateAccessList(t, bobCtx, "root/hackerjoe/$", string(access.Debug), appID, bobApp)
// Fortunately the device manager permits hackerjoe to access the stats.
// But hackerjoe can't solve Bob's problem.
- if _, err = statsStub(appID, bobApp, "stats/system/pid").Value(hjCtx); err != nil {
- t.Fatalf("hackerjoe couldn't access debug info on the app: %v", err)
- }
-
- // Show that hackerjoe can glob the debug space.
- results, _, err := testutil.GlobName(hjCtx, naming.Join("dm/apps", appID, bobApp, "stats"), "...")
- if err != nil {
- t.Fatalf("Debug rights should let hackerjoe glob the __debug space: %v", err)
- }
- if len(results) == 0 {
- t.Fatalf("hackerjoe didn't successfully glob the __debug space: no matches returned.")
- }
+ // Because hackerjob has Debug, hackerjoe can glob the __debug resources
+ // of Bob's app but can't glob Bob's app.
+ verifyGlob(t, hjCtx, "app", globtestminus, res)
+ verifyFailGlob(t, hjCtx, "app", globtests[0:1], res)
+ verifyStatsValues(t, hjCtx, "dm", "apps", appID, bobApp, "stats", "system/start-time*")
+ verifyLog(t, hjCtx, "dm", "apps", appID, bobApp, "logs", "*")
+ verifyPProfCmdLine(t, hjCtx, "app", "dm", "apps", appID, bobApp, "pprof")
// Alice might be able to help but Bob didn't give Alice access to the debug ACLs.
- if _, err = statsStub(appID, bobApp, "stats/system/pid").Value(aliceCtx); err == nil {
- t.Fatalf("Alice could wrongly access the stats without perms.")
- }
+ testAccessFail(t, verror.ErrNoAccess.ID, aliceCtx, "Alice", appID, bobApp)
// Bob forgets that Alice can't read the stats when he can.
- if _, err = statsStub(appID, bobApp, "stats/system/pid").Value(bobCtx); err != nil {
- t.Fatalf("Bob couldn't access debug info on his app: ", err)
- }
+ verifyGlob(t, bobCtx, "app", globtests, res)
+ verifyStatsValues(t, bobCtx, "dm", "apps", appID, bobApp, "stats/system/start-time*")
// So Bob changes the permissions so that Alice can help debug too.
updateAccessList(t, bobCtx, "root/alice/$", string(access.Debug), appID, bobApp)
// Alice can access __debug content.
- if _, err = statsStub(appID, bobApp, "stats/system/pid").Value(aliceCtx); err != nil {
- t.Fatalf("Alice couldn't access debug info on the app: %v", err)
- }
+ verifyGlob(t, aliceCtx, "app", globtestminus, res)
+ verifyFailGlob(t, aliceCtx, "app", globtests[0:1], res)
+ verifyStatsValues(t, aliceCtx, "dm", "apps", appID, bobApp, "stats", "system/start-time*")
+ verifyLog(t, aliceCtx, "dm", "apps", appID, bobApp, "logs", "*")
+ verifyPProfCmdLine(t, aliceCtx, "app", "dm", "apps", appID, bobApp, "pprof")
// Bob is glum because no one can help him fix his app so he stops it.
stopApp(t, bobCtx, appID, bobApp)
diff --git a/services/mgmt/device/impl/dispatcher.go b/services/mgmt/device/impl/dispatcher.go
index b4e6e91..da1e4e3 100644
--- a/services/mgmt/device/impl/dispatcher.go
+++ b/services/mgmt/device/impl/dispatcher.go
@@ -282,7 +282,11 @@
case "logs":
logsDir := filepath.Join(appInstanceDir, "logs")
suffix := naming.Join(components[5:]...)
- return logsimpl.NewLogFileService(logsDir, suffix), auth, nil
+ appSpecificAuthorizer, err := newAppSpecificAuthorizer(auth, d.config, components[1:], d.aclstore)
+ if err != nil {
+ return nil, nil, err
+ }
+ return logsimpl.NewLogFileService(logsDir, suffix), appSpecificAuthorizer, nil
case "pprof", "stats":
info, err := loadInstanceInfo(nil, appInstanceDir)
if err != nil {
diff --git a/services/mgmt/device/impl/util_test.go b/services/mgmt/device/impl/util_test.go
index e6aba11..de3d380 100644
--- a/services/mgmt/device/impl/util_test.go
+++ b/services/mgmt/device/impl/util_test.go
@@ -505,12 +505,25 @@
}
}
+// verifyFailGlob verifies that for each globTestVector instance that the
+// pattern returns no matches.
+func verifyFailGlob(t *testing.T, ctx *context.T, appName string, testcases []globTestVector, res *globTestRegexHelper) {
+ for _, tc := range testcases {
+ results, _, _ := testutil.GlobName(ctx, tc.name, tc.pattern)
+ if len(results) != 0 {
+ t.Errorf(testutil.FormatLogLine(2, "verifyFailGlob should have failed for %q, %q", tc.name, tc.pattern))
+ }
+ }
+}
+
// verifyLog calls Size() on a selection of log file objects to
// demonstrate that the log files are accessible and have been written by
// the application.
-func verifyLog(t *testing.T, ctx *context.T, service string, nameComponents ...string) {
- path := naming.Join(nameComponents...)
- files, _, err := testutil.GlobName(ctx, service, path)
+func verifyLog(t *testing.T, ctx *context.T, nameComponents ...string) {
+ a := nameComponents
+ pattern, prefix := a[len(a)-1], a[:len(a)-1]
+ path := naming.Join(prefix...)
+ files, _, err := testutil.GlobName(ctx, path, pattern)
if err != nil {
t.Errorf(testutil.FormatLogLine(2, "unexpected glob error: %v", err))
}
@@ -518,7 +531,7 @@
t.Errorf(testutil.FormatLogLine(2, "Unexpected number of matches. Got %d, want at least %d", got, want))
}
for _, file := range files {
- name := naming.Join("dm", file)
+ name := naming.Join(path, file)
c := logreader.LogFileClient(name)
if _, err := c.Size(ctx); err != nil {
t.Errorf(testutil.FormatLogLine(2, "Size(%q) failed: %v", name, err))
@@ -528,9 +541,12 @@
// verifyStatsValues call Value() on some of the stats objects to prove
// that they are correctly being proxied to the device manager.
-func verifyStatsValues(t *testing.T, ctx *context.T, service string, nameComponents ...string) {
- path := naming.Join(nameComponents...)
- objects, _, err := testutil.GlobName(ctx, service, path)
+func verifyStatsValues(t *testing.T, ctx *context.T, nameComponents ...string) {
+ a := nameComponents
+ pattern, prefix := a[len(a)-1], a[:len(a)-1]
+ path := naming.Join(prefix...)
+ objects, _, err := testutil.GlobName(ctx, path, pattern)
+
if err != nil {
t.Errorf(testutil.FormatLogLine(2, "unexpected glob error: %v", err))
}
@@ -538,7 +554,7 @@
t.Errorf(testutil.FormatLogLine(2, "Unexpected number of matches. Got %d, want %d", got, want))
}
for _, obj := range objects {
- name := naming.Join("dm", obj)
+ name := naming.Join(path, obj)
c := stats.StatsClient(name)
if _, err := c.Value(ctx); err != nil {
t.Errorf(testutil.FormatLogLine(2, "Value(%q) failed: %v", name, err))
