Skip to content

Commit

Permalink
Prevent writing outside allowed directories list from a config payloa…
Browse files Browse the repository at this point in the history
…d with actions (#766)

* added AllowedDirectories to agent details, added tests for allowed directories
  • Loading branch information
oliveromahony authored Aug 8, 2024
1 parent ef9524a commit 3dc98f3
Show file tree
Hide file tree
Showing 19 changed files with 659 additions and 348 deletions.
9 changes: 5 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,11 @@ $(TEST_BUILD_DIR):

# Unit tests
unit-test: $(TEST_BUILD_DIR) test-core test-plugins test-sdk test-extensions ## Run unit tests
echo 'mode: atomic' > $(TEST_BUILD_DIR)/coverage.out
tail -q -n +2 $(TEST_BUILD_DIR)/*_coverage.out >> $(TEST_BUILD_DIR)/coverage.out
go tool cover -html=$(TEST_BUILD_DIR)/coverage.out -o $(TEST_BUILD_DIR)/coverage.html
@printf "\nTotal code coverage: " && go tool cover -func=$(TEST_BUILD_DIR)/coverage.out | grep 'total:' | awk '{print $$3}'
@tail -q -n +2 $(TEST_BUILD_DIR)/*_coverage.out >> $(TEST_BUILD_DIR)/tmp_coverage.out
@echo 'mode: atomic' > $(TEST_BUILD_DIR)/coverage.out
@cat $(TEST_BUILD_DIR)/tmp_coverage.out | grep -v ".pb.go" | grep -v ".gen.go" | grep -v ".pb.validate.go" | grep -v "fake_" | grep -v "_mock.go" | grep -v "_stub.go" >> $(TEST_BUILD_DIR)/coverage.out
@go tool cover -html=$(TEST_BUILD_DIR)/coverage.out -o $(TEST_BUILD_DIR)/coverage.html
@printf "\nTotal code coverage: " && $(GOTOOL) cover -func=$(TEST_BUILD_DIR)/coverage.out | grep 'total:' | awk '{print $$3}'

test-core: $(TEST_BUILD_DIR) ## Run core unit tests
GOWORK=off CGO_ENABLED=0 go test -count=1 -coverprofile=$(TEST_BUILD_DIR)/core_coverage.out -covermode count ./src/core/...
Expand Down
1 change: 1 addition & 0 deletions docs/proto/proto.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ Represents agent details. This message is sent from the management server to the
| tags | [string](#string) | repeated | List of tags |
| alias | [string](#string) | | Alias name for the agent |
| server | [Server](#f5-nginx-agent-sdk-Server) | | Server setting for the agent |
| allowed_directories | [string](#string) | repeated | List of allowed directories that the Agent can write to |



Expand Down
230 changes: 144 additions & 86 deletions sdk/proto/agent.pb.go

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions sdk/proto/agent.proto
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ message AgentDetails {
string alias = 4 [(gogoproto.jsontag) = "alias"];
// Server setting for the agent
Server server = 5 [(gogoproto.jsontag) = "server"];
// List of allowed directories that the Agent can write to
repeated string allowed_directories = 6 [(gogoproto.jsontag) = "allowed_directories"];
}

message Server {
Expand Down
10 changes: 10 additions & 0 deletions src/core/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,16 @@ type Config struct {
QueueSize int `mapstructure:"queue_size" yaml:"queue_size,omitempty"`
}

func (c *Config) AllowedDirectories() []string {
values := []string{}

// Iterate through the map and append keys to the slice
for key := range c.AllowedDirectoriesMap {
values = append(values, key)
}
return values
}

func (c *Config) IsGrpcServerConfigured() bool {
return c.Server.Host != "" && c.Server.GrpcPort != 0
}
Expand Down
10 changes: 10 additions & 0 deletions src/core/environment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,16 @@ func TestWriteFilesNotAllowed(t *testing.T) {
Contents: []byte("multi"),
Permissions: "0644",
},
{
Name: "etc/shadow/multiple1.conf",
Contents: []byte("shadowfile2"),
Permissions: "0644",
},
{
Name: "/hello.conf",
Contents: []byte("rootfile"),
Permissions: "0644",
},
}
backup, err := sdk.NewConfigApplyWithIgnoreDirectives("", nil, []string{})
assert.NoError(t, err)
Expand Down
9 changes: 9 additions & 0 deletions src/core/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,11 @@ func (n *NginxBinaryType) writeConfigWithWithFileActions(
return nil, err
}

confDir := filepath.Dir(details.ConfPath)
if err := ensureFilesAllowed(confFiles, n.config.AllowedDirectoriesMap, confDir); err != nil {
return configApply, err
}

for _, file := range confFiles {
rootDirectoryPath := filepath.Dir(details.ConfPath)
fileFullPath := file.Name
Expand All @@ -519,6 +524,10 @@ func (n *NginxBinaryType) writeConfigWithWithFileActions(
}
}

if err := ensureFilesAllowed(auxFiles, n.config.AllowedDirectoriesMap, confDir); err != nil {
return configApply, err
}

for _, file := range auxFiles {
rootDirectoryPath := config.GetZaux().GetRootDirectory()
fileFullPath := file.Name
Expand Down
1 change: 1 addition & 0 deletions src/plugins/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ func (r *OneTimeRegistration) registerAgent() {
MaxElapsedTime: r.config.Server.Backoff.MaxElapsedTime.Milliseconds(),
},
},
AllowedDirectories: r.config.AllowedDirectories(),
},
},
Details: details,
Expand Down

Large diffs are not rendered by default.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 3dc98f3

Please sign in to comment.