matebenyovszky opened a new issue, #45787:
URL: https://github.com/apache/superset/issues/45787
### Bug description
Extension Storage values written with `encrypt: true` can never be read
back. Writes succeed, but every read of that key fails. The DAO logs `Failed to
decrypt extension storage value ... possible key rotation issue` and returns
`None`.
`_enc_type()` in `superset/extensions/storage/persistent_dao.py` wraps the
payload in `EncryptedType(LargeBinary, ...)`. The sqlalchemy_utils engines only
encrypt text:
- **On write,** the engine receives `bytes` and encrypts `repr(value)`, i.e.
the string `"b'...'"`.
- **On read,** `process_result_value()` rebuilds the value with
`LargeBinary.python_type(decrypted)`, i.e. `bytes(str)`. That raises
`TypeError: string argument without an encoding`.
This happens with both `SQLALCHEMY_ENCRYPTED_FIELD_ENGINE = "aes"` and
`"aes-gcm"`, and for both user-scoped and shared rows. The existing unit tests
mock `_enc_type`, so no test round-trips a real ciphertext.
Minimal reproduction with only sqlalchemy-utils 0.43.0 (the version Superset
pins):
```python
from sqlalchemy import LargeBinary
from sqlalchemy_utils import EncryptedType
from sqlalchemy_utils.types.encrypted.encrypted_type import AesGcmEngine
t = EncryptedType(LargeBinary, key="secret", engine=AesGcmEngine)
stored = t.process_bind_param(b'{"a": 1}', None)
t.engine.decrypt(stored.decode()) # 'b\'{"a": 1}\''
t.process_result_value(stored, None) # TypeError: string argument without
an encoding
```
Found while moving an extension (conversation history in the persistent
tier) to 7.0.0rc2. We now store the value unencrypted as a workaround.
### Screenshots/recordings
_No response_
### Superset version
master / latest-dev (also 7.0.0rc1 and 7.0.0rc2)
### Python version
3.11
### Node version
I don't know
### Browser
Not applicable
### Additional context
A related problem, not covered by the fix I'm proposing: `superset
re-encrypt-secrets` includes `extension_storage` through
`CONDITIONAL_ENCRYPTED_TABLES`. It decrypts those rows with the same
`EncryptedType(LargeBinary)` and with the plain `SECRET_KEY`. User-scoped rows,
however, are encrypted with a key derived from `SECRET_KEY` and the user ID. So
once a deployment holds an encrypted user-scoped row, the migrator cannot
decrypt it, counts a failure and rolls back the whole run. That probably
deserves its own change, since the migrator would need to know about the
per-user key derivation.
### Checklist
- [x] I have searched Superset docs and Slack and didn't find a solution to
my problem.
- [x] I have searched the GitHub issue tracker and didn't find a similar bug
report.
- [x] I have checked Superset's logs for errors and if I found a relevant
Python stacktrace, I included it here as text in the "additional context"
section.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
