Skip to content

Commit 1f49174

Browse files
committed
chore: add file permission check
Signed-off-by: Arjun Raja Yogidas <[email protected]>
1 parent eafcaac commit 1f49174

File tree

11 files changed

+173
-203
lines changed

11 files changed

+173
-203
lines changed

.github/workflows/ci.yaml

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -105,17 +105,19 @@ jobs:
105105
run: |
106106
sudo ls /etc/cni/net.d
107107
sudo rm /etc/cni/net.d/87-podman-bridge.conflist
108+
- name: Verify Rego file presence
109+
run: ls -l ${{ github.workspace }}/docs/sample-rego-policies/example.rego
110+
- name: Set Rego file path
111+
run: echo "REGO_FILE_PATH=${{ github.workspace }}/docs/sample-rego-policies/example.rego" >> $GITHUB_ENV
112+
- name: Start finch-daemon with opa Authz
113+
run: sudo bin/finch-daemon --debug --enable-opa-middleware --rego-file ${{ github.workspace }}/docs/sample-rego-policies/example.rego --skip-rego-perm-check --socket-owner $UID --socket-addr /run/finch.sock --pidfile /run/finch.pid &
114+
- name: Run opa e2e tests
115+
run: sudo -E make test-e2e-opa
116+
- name: Clean up Daemon socket
117+
run: sudo rm /run/finch.sock && sudo rm /run/finch.pid
108118
- name: Start finch-daemon
109119
run: sudo bin/finch-daemon --debug --socket-owner $UID &
110120
- name: Run e2e test
111121
run: sudo make test-e2e
112122
- name: Clean up Daemon socket
113123
run: sudo rm /var/run/finch.sock && sudo rm /run/finch.pid
114-
- name: Verify Rego file presence
115-
run: ls -l ${{ github.workspace }}/docs/sample-rego-policies/default.rego
116-
- name: Set Rego file path
117-
run: echo "REGO_FILE_PATH=${{ github.workspace }}/docs/sample-rego-policies/default.rego" >> $GITHUB_ENV
118-
- name: Start finch-daemon with opa Authz
119-
run: sudo bin/finch-daemon --debug --enable-middleware --rego-file ${{ github.workspace }}/docs/sample-rego-policies/default.rego --socket-owner $UID &
120-
- name: Run opa e2e tests
121-
run: sudo -E make test-e2e-opa

Makefile

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,10 @@ test-e2e: linux
117117
.PHONY: test-e2e-opa
118118
test-e2e-opa: linux
119119
DOCKER_HOST="unix:///run/finch.sock" \
120-
DOCKER_API_VERSION="v1.43" \
120+
DOCKER_API_VERSION="v1.41" \
121121
MIDDLEWARE_E2E=1 \
122-
TEST_E2E=1 \
122+
TEST_E2E=0 \
123+
DAEMON_ROOT="$(BIN)/finch-daemon" \
123124
$(GINKGO) $(GFLAGS) ./e2e/...
124125

125126
.PHONY: licenses
@@ -134,4 +135,4 @@ coverage: linux
134135
.PHONY: release
135136
release: linux
136137
@echo "$@"
137-
@$(FINCH_DAEMON_PROJECT_ROOT)/scripts/create-releases.sh $(RELEASE_TAG)
138+
@$(FINCH_DAEMON_PROJECT_ROOT)/scripts/create-releases.sh $(RELEASE_TAG)

api/router/router.go

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,17 @@ type Options struct {
6262
func New(opts *Options) (http.Handler, error) {
6363
r := mux.NewRouter()
6464
r.Use(VersionMiddleware)
65+
66+
logger := flog.NewLogrus()
67+
6568
if opts.RegoFilePath != "" {
66-
regoMiddleware, err := CreateRegoMiddleware(opts.RegoFilePath)
69+
regoMiddleware, err := CreateRegoMiddleware(opts.RegoFilePath, logger)
6770
if err != nil {
6871
return nil, err
6972
}
7073
r.Use(regoMiddleware)
7174
}
7275
vr := types.VersionedRouter{Router: r}
73-
74-
logger := flog.NewLogrus()
7576
system.RegisterHandlers(vr, opts.SystemService, opts.Config, opts.NerdctlWrapper, logger)
7677
image.RegisterHandlers(vr, opts.ImageService, opts.Config, logger)
7778
container.RegisterHandlers(vr, opts.ContainerService, opts.Config, logger)
@@ -112,7 +113,7 @@ func VersionMiddleware(next http.Handler) http.Handler {
112113
// CreateRegoMiddleware dynamically parses the rego file at the path specified in options
113114
// and allows or denies the request based on the policy.
114115
// Will return a nil function and an error if the given file path is blank or invalid.
115-
func CreateRegoMiddleware(regoFilePath string) (func(next http.Handler) http.Handler, error) {
116+
func CreateRegoMiddleware(regoFilePath string, logger *flog.Logrus) (func(next http.Handler) http.Handler, error) {
116117
if regoFilePath == "" {
117118
return nil, errRego
118119
}
@@ -135,20 +136,24 @@ func CreateRegoMiddleware(regoFilePath string) (func(next http.Handler) http.Han
135136
Path: r.URL.Path,
136137
}
137138

138-
fmt.Printf("Evaluating policy rules for API request with Method = %s and Path = %s \n", input.Method, input.Path)
139+
logger.Debugf("OPA input being evaluated: Method=%s, Path=%s", input.Method, input.Path)
140+
139141
rs, err := preppedQuery.Eval(r.Context(), rego.EvalInput(input))
140142
if err != nil {
143+
logger.Errorf("OPA policy evaluation failed: %v", err)
141144
response.SendErrorResponse(w, http.StatusInternalServerError, errInput)
142145
return
143146
}
144147

148+
logger.Debugf("OPA evaluation results: %+v", rs)
149+
145150
if !rs.Allowed() {
146-
// need to log evaluation result in order to mitigate Repudiation threat
147-
fmt.Printf("Evaluation result: failed, method %s not allowed for path %s \n", r.Method, r.URL.Path)
151+
logger.Infof("OPA request denied: Method=%s, Path=%s", r.Method, r.URL.Path)
148152
response.SendErrorResponse(w, http.StatusForbidden,
149153
fmt.Errorf("method %s not allowed for path %s", r.Method, r.URL.Path))
150154
return
151155
}
156+
logger.Debugf("OPA request allowed: Method=%s, Path=%s", r.Method, r.URL.Path)
152157
newReq := r.WithContext(r.Context())
153158
next.ServeHTTP(w, newReq)
154159
})

cmd/finch-daemon/main.go

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,15 @@ const (
4343
)
4444

4545
type DaemonOptions struct {
46-
debug bool
47-
socketAddr string
48-
socketOwner int
49-
debugAddress string
50-
configPath string
51-
pidFile string
52-
regoFilePath string
53-
enableMiddleware bool
54-
regoFileLock *flock.Flock
46+
debug bool
47+
socketAddr string
48+
socketOwner int
49+
debugAddress string
50+
configPath string
51+
pidFile string
52+
regoFilePath string
53+
enableOPAMiddleware bool
54+
skipRegoPermCheck bool
5555
}
5656

5757
var options = new(DaemonOptions)
@@ -71,7 +71,9 @@ func main() {
7171
rootCmd.Flags().StringVar(&options.configPath, "config-file", defaultConfigPath, "Daemon Config Path")
7272
rootCmd.Flags().StringVar(&options.pidFile, "pidfile", defaultPidFile, "pid file location")
7373
rootCmd.Flags().StringVar(&options.regoFilePath, "rego-file", "", "Rego Policy Path")
74-
rootCmd.Flags().BoolVar(&options.enableMiddleware, "enable-middleware", false, "turn on middleware for allowlisting")
74+
rootCmd.Flags().BoolVar(&options.enableOPAMiddleware, "enable-opa-middleware", false, "turn on OPA middleware for allowlisting")
75+
rootCmd.Flags().BoolVar(&options.skipRegoPermCheck, "skip-rego-perm-check", false, "skip the rego file permission check (allows permissions more permissive than 0600)")
76+
7577
if err := rootCmd.Execute(); err != nil {
7678
log.Printf("got error: %v", err)
7779
log.Fatal(err)
@@ -149,10 +151,6 @@ func run(options *DaemonOptions) error {
149151
logger := flog.NewLogrus()
150152
r, err := newRouter(options, logger)
151153
if err != nil {
152-
// call regoFile cleanup function here to unlock previously locked file
153-
if options.regoFilePath != "" {
154-
cleanupRegoFile(options, logger)
155-
}
156154
return fmt.Errorf("failed to create a router: %w", err)
157155
}
158156

@@ -202,8 +200,6 @@ func run(options *DaemonOptions) error {
202200
}
203201
}()
204202

205-
defer cleanupRegoFile(options, logger)
206-
207203
sdNotify(daemon.SdNotifyReady, logger)
208204
serverWg.Wait()
209205
logger.Debugln("Server stopped. Exiting...")
@@ -227,8 +223,8 @@ func newRouter(options *DaemonOptions, logger *flog.Logrus) (http.Handler, error
227223
}
228224

229225
var regoFilePath string
230-
if options.enableMiddleware {
231-
regoFilePath, err = sanitizeRegoFile(options)
226+
if options.enableOPAMiddleware {
227+
regoFilePath, err = checkRegoFileValidity(options, logger)
232228
if err != nil {
233229
return nil, err
234230
}

cmd/finch-daemon/router_utils.go

Lines changed: 23 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"github.com/containerd/containerd/v2/pkg/namespaces"
1515
"github.com/containerd/nerdctl/v2/pkg/api/types"
1616
"github.com/containerd/nerdctl/v2/pkg/config"
17-
"github.com/gofrs/flock"
1817
toml "github.com/pelletier/go-toml/v2"
1918
"github.com/runfinch/finch-daemon/api/router"
2019
"github.com/runfinch/finch-daemon/internal/backend"
@@ -29,7 +28,6 @@ import (
2928
"github.com/runfinch/finch-daemon/pkg/archive"
3029
"github.com/runfinch/finch-daemon/pkg/ecc"
3130
"github.com/runfinch/finch-daemon/pkg/flog"
32-
"github.com/sirupsen/logrus"
3331
"github.com/spf13/afero"
3432
)
3533

@@ -94,45 +92,6 @@ func createContainerdClient(conf *config.Config) (*backend.ContainerdClientWrapp
9492
return backend.NewContainerdClientWrapper(client), nil
9593
}
9694

97-
// sanitizeRegoFile validates and prepares the Rego policy file for use.
98-
// It checks validates the file, acquires a file lock,
99-
// and sets rego file to be read-only.
100-
func sanitizeRegoFile(options *DaemonOptions) (string, error) {
101-
if options.regoFilePath != "" {
102-
if !options.enableMiddleware {
103-
return "", fmt.Errorf("rego file path was provided without the --enable-middleware flag, please provide the --enable-middleware flag") // todo, can we default to setting this flag ourselves is this better UX?
104-
}
105-
106-
if err := checkRegoFileValidity(options.regoFilePath); err != nil {
107-
return "", err
108-
}
109-
}
110-
111-
if options.enableMiddleware && options.regoFilePath == "" {
112-
return "", fmt.Errorf("rego file path not provided, please provide the policy file path using the --rego-file flag")
113-
}
114-
115-
fileLock := flock.New(options.regoFilePath)
116-
117-
locked, err := fileLock.TryLock()
118-
if err != nil {
119-
return "", fmt.Errorf("error acquiring lock on rego file: %v", err)
120-
}
121-
if !locked {
122-
return "", fmt.Errorf("unable to acquire lock on rego file, it may be in use by another process")
123-
}
124-
125-
// Change file permissions to read-only
126-
err = os.Chmod(options.regoFilePath, 0400)
127-
if err != nil {
128-
fileLock.Unlock()
129-
return "", fmt.Errorf("error changing rego file permissions: %v", err)
130-
}
131-
options.regoFileLock = fileLock
132-
133-
return options.regoFilePath, nil
134-
}
135-
13695
// createRouterOptions creates router options by initializing all required services.
13796
func createRouterOptions(
13897
conf *config.Config,
@@ -160,39 +119,37 @@ func createRouterOptions(
160119
}
161120
}
162121

163-
// checkRegoFileValidity verifies that the given rego file exists and has the right file extension.
164-
func checkRegoFileValidity(regoFilePath string) error {
165-
fmt.Println("filepath in checkRegoFileValidity = ", regoFilePath)
166-
if _, err := os.Stat(regoFilePath); os.IsNotExist(err) {
167-
return fmt.Errorf("provided Rego file path does not exist: %s", regoFilePath)
122+
// checkRegoFileValidity validates and prepares the Rego policy file for use.
123+
// It verifies that the file exists, has the right extension (.rego), and has appropriate permissions.
124+
func checkRegoFileValidity(options *DaemonOptions, logger *flog.Logrus) (string, error) {
125+
if options.regoFilePath == "" {
126+
return "", fmt.Errorf("rego file path not provided, please provide the policy file path using the --rego-file flag")
168127
}
169128

170-
// Check if the file has a valid extension (.rego)
171-
fileExt := strings.ToLower(filepath.Ext(regoFilePath))
172-
173-
fmt.Println("fileExt = ", fileExt)
174-
if fileExt != ".rego" {
175-
return fmt.Errorf("invalid file extension for Rego file. Only .rego files are supported")
129+
if _, err := os.Stat(options.regoFilePath); os.IsNotExist(err) {
130+
return "", fmt.Errorf("provided Rego file path does not exist: %s", options.regoFilePath)
176131
}
177132

178-
return nil
179-
}
133+
// Check if the file has a valid extension (.rego)
134+
fileExt := strings.ToLower(filepath.Ext(options.regoFilePath))
180135

181-
func cleanupRegoFile(options *DaemonOptions, logger *flog.Logrus) {
182-
if options.regoFileLock == nil {
183-
return // Already cleaned up or nothing to clean
136+
if fileExt != ".rego" {
137+
return "", fmt.Errorf("invalid file extension for Rego file. Only .rego files are supported")
184138
}
185139

186-
// unlock the rego file
187-
if err := options.regoFileLock.Unlock(); err != nil {
188-
logrus.Errorf("failed to unlock Rego file: %v", err)
189-
}
190-
logger.Infof("rego file unlocked")
140+
if !options.skipRegoPermCheck {
141+
fileInfo, err := os.Stat(options.regoFilePath)
142+
if err != nil {
143+
return "", fmt.Errorf("error checking rego file permissions: %v", err)
144+
}
191145

192-
// make rego file editable
193-
if err := os.Chmod(options.regoFilePath, 0600); err != nil {
194-
logrus.Errorf("failed to change file permissions of rego file: %v", err)
146+
if fileInfo.Mode().Perm()&0177 != 0 {
147+
return "", fmt.Errorf("rego file permissions %o are too permissive (maximum allowable permissions: 0600)", fileInfo.Mode().Perm())
148+
}
149+
logger.Debugf("rego file permissions check passed: %o", fileInfo.Mode().Perm())
150+
} else {
151+
logger.Warnf("skipping rego file permission check - file may have permissions more permissive than 0600")
195152
}
196153

197-
options.regoFileLock = nil
154+
return options.regoFilePath, nil
198155
}

0 commit comments

Comments
 (0)