fix: harden target recovery metadata safety
This commit is contained in:
@@ -20,7 +20,10 @@ import (
|
||||
"proxyctl/internal/target"
|
||||
)
|
||||
|
||||
const service = "docker.service"
|
||||
const (
|
||||
service = "docker.service"
|
||||
recoveryVersion = 1
|
||||
)
|
||||
|
||||
type Configuration struct {
|
||||
DaemonConfigPath string
|
||||
@@ -188,9 +191,10 @@ func parseProperties(output string) map[string]string {
|
||||
}
|
||||
|
||||
type recovery struct {
|
||||
Version int `json:"version"`
|
||||
Original []byte `json:"original,omitempty"`
|
||||
OriginalExists bool `json:"originalExists"`
|
||||
OriginalMode uint32 `json:"originalMode,omitempty"`
|
||||
OriginalMode uint32 `json:"originalMode"`
|
||||
ConfigFingerprint string `json:"configFingerprint"`
|
||||
RestartedFingerprint string `json:"restartedFingerprint,omitempty"`
|
||||
}
|
||||
@@ -277,6 +281,7 @@ func (a *Adapter) enable(ctx context.Context, p profile.Profile) error {
|
||||
return fmt.Errorf("read Docker daemon configuration: %w", readErr)
|
||||
}
|
||||
if !managed {
|
||||
metadata.Version = recoveryVersion
|
||||
metadata.OriginalExists = readErr == nil
|
||||
if metadata.OriginalExists {
|
||||
metadata.Original = append([]byte(nil), current...)
|
||||
@@ -372,12 +377,59 @@ func (a *Adapter) readRecovery() (recovery, bool, error) {
|
||||
if err != nil {
|
||||
return recovery{}, false, err
|
||||
}
|
||||
var fields map[string]json.RawMessage
|
||||
if err := json.Unmarshal(data, &fields); err != nil {
|
||||
return recovery{}, true, err
|
||||
}
|
||||
if fields == nil {
|
||||
return recovery{}, true, errors.New("Docker daemon recovery metadata must be a JSON object")
|
||||
}
|
||||
var value recovery
|
||||
if err := json.Unmarshal(data, &value); err != nil {
|
||||
return recovery{}, true, err
|
||||
}
|
||||
if err := validateRecovery(value, fields); err != nil {
|
||||
return recovery{}, true, err
|
||||
}
|
||||
return value, true, nil
|
||||
}
|
||||
|
||||
func validateRecovery(value recovery, fields map[string]json.RawMessage) error {
|
||||
for _, name := range []string{"version", "originalExists", "originalMode", "configFingerprint"} {
|
||||
if _, present := fields[name]; !present {
|
||||
return fmt.Errorf("Docker daemon recovery metadata is missing required field %q", name)
|
||||
}
|
||||
}
|
||||
if value.Version != recoveryVersion {
|
||||
return fmt.Errorf("unsupported Docker daemon recovery metadata version %d", value.Version)
|
||||
}
|
||||
if !validFingerprint(value.ConfigFingerprint) {
|
||||
return errors.New("Docker daemon recovery metadata has an invalid configuration fingerprint")
|
||||
}
|
||||
if value.RestartedFingerprint != "" && (value.RestartedFingerprint != value.ConfigFingerprint || !validFingerprint(value.RestartedFingerprint)) {
|
||||
return errors.New("Docker daemon recovery metadata has an inconsistent restarted fingerprint")
|
||||
}
|
||||
_, hasOriginal := fields["original"]
|
||||
if value.OriginalExists {
|
||||
if !hasOriginal {
|
||||
return errors.New("Docker daemon recovery metadata is missing the recorded original configuration or mode")
|
||||
}
|
||||
if value.OriginalMode > 0o777 {
|
||||
return errors.New("Docker daemon recovery metadata has an invalid original file mode")
|
||||
}
|
||||
if err := validateConfigSchema(value.Original); err != nil {
|
||||
return fmt.Errorf("Docker daemon recovery metadata has an invalid original configuration: %w", err)
|
||||
}
|
||||
} else if hasOriginal || len(value.Original) != 0 || value.OriginalMode != 0 {
|
||||
return errors.New("Docker daemon recovery metadata records original data although the original did not exist")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func validFingerprint(value string) bool {
|
||||
decoded, err := hex.DecodeString(value)
|
||||
return err == nil && len(decoded) == sha256.Size
|
||||
}
|
||||
func (a *Adapter) writeRecovery(value recovery) error {
|
||||
data, err := json.MarshalIndent(value, "", " ")
|
||||
if err != nil {
|
||||
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"proxyctl/internal/app"
|
||||
"proxyctl/internal/dockerdaemon"
|
||||
"proxyctl/internal/profile"
|
||||
"proxyctl/internal/target"
|
||||
@@ -106,6 +107,61 @@ type fixedAuthority int
|
||||
|
||||
func (a fixedAuthority) EffectiveUID() int { return int(a) }
|
||||
|
||||
type immediateLock struct{}
|
||||
|
||||
func (immediateLock) Lock(context.Context) (func(), error) { return func() {}, nil }
|
||||
|
||||
func TestUnsafeRecoveryMetadataIsDriftedAndResetNeverMutatesDaemon(t *testing.T) {
|
||||
fingerprint := strings.Repeat("a", 64)
|
||||
cases := map[string]string{
|
||||
"empty object": `{}`,
|
||||
"truncated": `{"version":1`,
|
||||
"unsupported version": `{"version":2,"originalExists":false,"configFingerprint":"` + fingerprint + `"}`,
|
||||
"missing original": `{"version":1,"originalExists":true,"originalMode":384,"configFingerprint":"` + fingerprint + `"}`,
|
||||
"unexpected original": `{"version":1,"originalExists":false,"original":"e30=","configFingerprint":"` + fingerprint + `"}`,
|
||||
"mode without original": `{"version":1,"originalExists":false,"originalMode":384,"configFingerprint":"` + fingerprint + `"}`,
|
||||
"invalid mode": `{"version":1,"originalExists":true,"original":"e30=","originalMode":4096,"configFingerprint":"` + fingerprint + `"}`,
|
||||
"invalid fingerprint": `{"version":1,"originalExists":false,"configFingerprint":"short"}`,
|
||||
"contradictory restart record": `{"version":1,"originalExists":false,"configFingerprint":"` + fingerprint + `","restartedFingerprint":"` + strings.Repeat("b", 64) + `"}`,
|
||||
}
|
||||
for name, metadata := range cases {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
a, record, daemonPath, recoveryPath, _ := fixture(t)
|
||||
originalConfig := []byte(`{"log-level":"debug"}`)
|
||||
if err := os.MkdirAll(filepath.Dir(daemonPath), 0o700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(daemonPath, originalConfig, 0o640); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.MkdirAll(filepath.Dir(recoveryPath), 0o700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(recoveryPath, []byte(metadata), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
record.events = nil
|
||||
|
||||
if inspection := a.Inspect(context.Background(), nil); inspection.State != target.Drifted {
|
||||
t.Fatalf("inspection = %#v", inspection)
|
||||
}
|
||||
reset := app.NewMaintenance(profile.State{}, []target.Target{a}, immediateLock{}, nil).Reset(context.Background(), true)
|
||||
if reset.Err == nil || len(reset.Targets) != 1 || reset.Targets[0].Err == nil {
|
||||
t.Fatalf("unsafe reset = %#v", reset)
|
||||
}
|
||||
if events := mutationEvents(record.events); len(events) != 0 {
|
||||
t.Fatalf("unsafe metadata caused mutation: %#v", events)
|
||||
}
|
||||
if got, err := os.ReadFile(daemonPath); err != nil || !reflect.DeepEqual(got, originalConfig) {
|
||||
t.Fatalf("daemon changed to %q: %v", got, err)
|
||||
}
|
||||
if got, err := os.ReadFile(recoveryPath); err != nil || string(got) != metadata {
|
||||
t.Fatalf("recovery changed to %q: %v", got, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestDiagnosticChecksCoverSystemdSchemaAndAuthorityWithoutMutation(t *testing.T) {
|
||||
a, record, daemonPath, recoveryPath, runner := fixture(t)
|
||||
a = dockerdaemon.New(dockerdaemon.Configuration{DaemonConfigPath: daemonPath, RecoveryPath: recoveryPath, TrustedRoot: filepath.Dir(filepath.Dir(daemonPath))}, recordingFS{recorder: record}, runner, fixedAuthority(0))
|
||||
|
||||
Reference in New Issue
Block a user