Skip to content

Commit dedca2f

Browse files
authored
Fix credential exposure in kubeconfig file permissions, azcmagent argv, and log file handling (#145)
1 parent 94a1422 commit dedca2f

File tree

3 files changed

+13
-64
lines changed

3 files changed

+13
-64
lines changed

components/arc/v20260301/arc_registration.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,13 @@ func (a *installArcAction) addAuthenticationArgs(ctx context.Context, args *[]st
164164
return fmt.Errorf("failed to get access token: %w", err)
165165
}
166166

167-
// Add access token to azcmagent arguments
167+
// TODO(security): The access token is passed on the command line and is therefore visible
168+
// to all local users via /proc/<pid>/cmdline for the lifetime of the azcmagent process.
169+
// The token is short-lived (~60 minutes) which limits the exposure window, but it is
170+
// still observable during the registration process. azcmagent does not currently support
171+
// reading the token from stdin or an environment variable.
172+
// TODO: Check if we can let azcmagent discover auth settings on its own (e.g. --use-azcli
173+
// or VM MSI) instead of fetching a token ourselves and passing it on the command line.
168174
*args = append(*args, "--access-token", accessToken)
169175

170176
a.logger.Debug("Authentication arguments added to Arc agent command")

components/kubelet/v20260301/kubelet_config.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,8 @@ func (s *startKubeletServiceAction) ensureKubeletKubeconfig(
186186
return false, nil
187187
}
188188

189-
// FIXME: consider using 0640?
190-
if err := utilio.WriteFile(config.KubeletKubeconfigPath, desiredContent, 0644); err != nil {
189+
// Write kubeconfig with restricted permissions — contains credentials (SP secret / MSI config)
190+
if err := utilio.WriteFile(config.KubeletKubeconfigPath, desiredContent, 0600); err != nil {
191191
return false, fmt.Errorf("write %q: %w", config.KubeletKubeconfigPath, err)
192192
}
193193
return true, nil
@@ -216,8 +216,8 @@ func (s *startKubeletServiceAction) ensureBootstrapKubeconfig(
216216
return false, nil
217217
}
218218

219-
// FIXME: consider using 0640?
220-
if err := utilio.WriteFile(config.KubeletBootstrapKubeconfigPath, desiredContent, 0644); err != nil {
219+
// Write bootstrap kubeconfig with restricted permissions — contains bootstrap token
220+
if err := utilio.WriteFile(config.KubeletBootstrapKubeconfigPath, desiredContent, 0600); err != nil {
221221
return false, fmt.Errorf("write %q: %w", config.KubeletBootstrapKubeconfigPath, err)
222222
}
223223
return true, nil

pkg/logger/logger.go

Lines changed: 2 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import (
99
"runtime"
1010
"strings"
1111

12-
"github.com/Azure/AKSFlexNode/pkg/utils"
13-
"github.com/Azure/AKSFlexNode/pkg/utils/utilio"
1412
"github.com/sirupsen/logrus"
1513
)
1614

@@ -153,25 +151,9 @@ func setupLogFileWriter(logDir string) (io.Writer, error) {
153151

154152
logFilePath := filepath.Join(logDir, "aks-flex-node.log")
155153

156-
// Create the log file if it doesn't exist
157-
if err := createLogFileIfNotExists(logFilePath); err != nil {
158-
return nil, fmt.Errorf("failed to create log file '%s': %w", logFilePath, err)
159-
}
160-
161-
// Try to open log file for writing, handle permission issues
162-
file, err := os.OpenFile(logFilePath, os.O_WRONLY|os.O_APPEND, 0666)
154+
// Open log file for appending, creating it with 0600 if it doesn't exist
155+
file, err := os.OpenFile(logFilePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) //#nosec G304 - logFilePath is from trusted agent config
163156
if err != nil {
164-
// If it's a permission error and we're not running as root, try to fix permissions
165-
if os.IsPermission(err) {
166-
// Try to fix permissions using system command
167-
if fixErr := utils.RunSystemCommand("chmod", "666", logFilePath); fixErr == nil {
168-
// Retry opening the file after fixing permissions
169-
file, err = os.OpenFile(logFilePath, os.O_WRONLY|os.O_APPEND, 0666)
170-
if err == nil {
171-
return file, nil
172-
}
173-
}
174-
}
175157
return nil, fmt.Errorf("failed to open log file '%s': %w", logFilePath, err)
176158
}
177159

@@ -207,45 +189,6 @@ func setupLogFile(logger *logrus.Logger, logDir string) error {
207189
return nil
208190
}
209191

210-
// createLogFileIfNotExists creates a log file using appropriate method based on path privileges
211-
func createLogFileIfNotExists(logFilePath string) error {
212-
// Check if file already exists
213-
if utils.FileExists(logFilePath) {
214-
return nil
215-
}
216-
217-
// For systemd services, try direct file creation first since the service
218-
// should have the correct user/group and the log directory should already exist
219-
if isRunningUnderSystemd() {
220-
// Try direct file creation with appropriate permissions
221-
file, err := os.OpenFile(logFilePath, os.O_CREATE|os.O_WRONLY, 0644)
222-
if err == nil {
223-
_ = file.Close()
224-
return nil
225-
}
226-
// If direct creation fails, fall through to the system method
227-
fmt.Printf("Warning: Direct log file creation failed (%v), trying system method...\n", err)
228-
}
229-
230-
// Use WriteFileAtomicSystem to create an empty log file with proper permissions
231-
if err := utilio.WriteFile(logFilePath, []byte{}, 0644); err != nil {
232-
return err
233-
}
234-
235-
// Ensure proper ownership for the current user after file creation
236-
// Skip this for systemd services as they should already have correct ownership
237-
if !isRunningUnderSystemd() {
238-
currentUser := os.Getenv("USER")
239-
if currentUser != "" {
240-
if err := utils.RunSystemCommand("chown", currentUser+":"+currentUser, logFilePath); err != nil {
241-
fmt.Printf("Warning: Failed to change ownership of %s to %s: %v\n", logFilePath, currentUser, err)
242-
}
243-
}
244-
}
245-
246-
return nil
247-
}
248-
249192
// GetLoggerFromContext retrieves the logger from context
250193
func GetLoggerFromContext(ctx context.Context) *logrus.Logger {
251194
if logger, ok := ctx.Value(loggerContextKey).(*logrus.Logger); ok {

0 commit comments

Comments
 (0)