diff --git a/lagoon/fill.go b/lagoon/fill.go index 6c15ba3..5cdfa83 100644 --- a/lagoon/fill.go +++ b/lagoon/fill.go @@ -122,6 +122,9 @@ func setField(field reflect.Value, val any) error { field.Set(converted) return nil } + if field.Type() == encryptedType { + return err + } if field.CanAddr() { if scanner, ok := field.Addr().Interface().(sql.Scanner); ok { scanSrc, scanErr := fillScanSource(val) @@ -147,7 +150,12 @@ func fillScanSource(val any) (any, error) { } } +var encryptedType = reflect.TypeOf(Encrypted{}) + func convertValue(src reflect.Value, destType reflect.Type) (reflect.Value, error) { + if destType == encryptedType { + return encryptedFromRequest(src) + } if src.Type().AssignableTo(destType) { return src, nil } @@ -156,3 +164,17 @@ func convertValue(src reflect.Value, destType reflect.Type) (reflect.Value, erro } return reflect.Value{}, fmt.Errorf("cannot assign %s to %s", src.Type(), destType) } + +// encryptedFromRequest treats request input for an Encrypted column as +// plaintext. It never falls through to Encrypted.Scan: Scan decrypts, so a +// write path that scanned request input would reject real secrets and accept +// another row's ciphertext, copying that row's secret. +func encryptedFromRequest(src reflect.Value) (reflect.Value, error) { + if src.Type() == encryptedType { + return src, nil + } + if src.Kind() != reflect.String { + return reflect.Value{}, fmt.Errorf("encrypted value must be a string") + } + return reflect.ValueOf(NewEncrypted(src.String())), nil +} diff --git a/lagoon/fill_test.go b/lagoon/fill_test.go index 2381049..649c281 100644 --- a/lagoon/fill_test.go +++ b/lagoon/fill_test.go @@ -103,3 +103,60 @@ func TestFillDroppedKeyNeverErrors(t *testing.T) { t.Fatalf("collection_id = %d", row.CollectionID) } } + +type fillSecretFixture struct { + Name string `gorm:"column:name"` + APIKey Encrypted `gorm:"column:api_key" json:"-"` + Token *Encrypted `gorm:"column:token" json:"-"` +} + +func TestFillEncryptedTakesPlaintext(t *testing.T) { + if err := PublishEncryptionKeys(nil, bytes.Repeat([]byte("F"), 32), nil); err != nil { + t.Fatal(err) + } + allowed := []string{"name", "api_key", "token"} + var row fillSecretFixture + if err := Fill(&row, allowed, map[string]any{"api_key": "sk-plain", "token": "tok-plain"}, true); err != nil { + t.Fatal(err) + } + if row.APIKey.Reveal() != "sk-plain" { + t.Fatalf("api_key Reveal = %q", row.APIKey.Reveal()) + } + if row.Token == nil || row.Token.Reveal() != "tok-plain" { + t.Fatalf("token = %v", row.Token) + } + + // Another row's ciphertext is stored as literal text, never decrypted. + stolen, err := NewEncrypted("victim-secret").Value() + if err != nil { + t.Fatal(err) + } + ciphertext, ok := stolen.(string) + if !ok { + t.Fatalf("ciphertext driver value is %T", stolen) + } + var thief fillSecretFixture + if err := Fill(&thief, allowed, map[string]any{"api_key": ciphertext}, true); err != nil { + t.Fatal(err) + } + if thief.APIKey.Reveal() != ciphertext { + t.Fatal("ciphertext in a request must not be decrypted into the victim's plaintext") + } + + for _, bad := range []any{42, true, map[string]any{"x": 1}, []byte("raw")} { + var r fillSecretFixture + if err := Fill(&r, allowed, map[string]any{"api_key": bad}, true); err == nil { + t.Fatalf("api_key=%T must be rejected", bad) + } + if r.APIKey.Reveal() != "" { + t.Fatalf("api_key=%T left a value behind", bad) + } + } + + if err := Fill(&row, allowed, map[string]any{"api_key": nil, "token": nil}, true); err != nil { + t.Fatal(err) + } + if row.APIKey.Reveal() != "" || row.Token != nil { + t.Fatalf("nil must clear: api_key=%q token=%v", row.APIKey.Reveal(), row.Token) + } +}