fix(05): WR-02 fill Encrypted columns from plaintext, never Scan request input
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user