Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent writing outside allowed directories list from a config payload with actions #766

Merged
merged 4 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 in the Agent Config |



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 in the Agent Config
oliveromahony marked this conversation as resolved.
Show resolved Hide resolved
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
Loading