Modernize Tool Config credential encryption to AES-256-GCM#15058
Modernize Tool Config credential encryption to AES-256-GCM#15058Maffooch wants to merge 5 commits into
Conversation
The legacy credential encryption used AES-256-OFB ("AES.1" stored format).
OFB has been moved to cryptography.hazmat.decrepit.ciphers.modes and is
scheduled for removal from primitives.ciphers.modes, which raises a
deprecation warning today and will break the decrypt path in a future
cryptography release.
Introduce a modern "AES.2" scheme using AES-256-GCM (authenticated
encryption, no padding) written via dojo_crypto_encrypt(). prepare_for_view()
now reads both formats, so every secret already stored in the database keeps
decrypting; any unrecognized prefix falls through to the legacy path. The new
scheme reuses the existing get_db_key() value, so no key rotation or settings
change is required. Existing "AES.1" values upgrade lazily the next time a
Tool Config is saved.
The retained legacy decrypt path now imports OFB from its decrepit home (with
an ImportError fallback for older cryptography), silencing the deprecation
warning and surviving the eventual removal. Error handling in
prepare_for_view() is hardened to catch InvalidTag/ValueError/IndexError so a
tampered or malformed value degrades to "" instead of raising.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Maffooch
force-pushed
the
tool-config-aes-gcm-encryption
branch
from
8069794 to
c25ddc7
Compare
Now safe to land alongside the AES-256-GCM migration: the legacy decrypt path no longer triggers the OFB deprecation removed/relocated in cryptography 49, and DefectDojo does not import pyopenssl directly (it is a transitive/pinned dependency). pyopenssl 26.3.0 requires cryptography >= 49. Supersedes the standalone Dependabot bumps #15029 and #15032. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No migration? At some point we'll have to I think? |
|
Sure I can add a migration. The |
Migration 0270 eagerly upgrades every stored Tool_Configuration credential (password, ssh, api_key) from the legacy "AES.1" (AES-256-OFB) scheme to the modern "AES.2" (AES-256-GCM) scheme, rather than waiting for the lazy per-save upgrade. It re-encrypts only "AES.1"-prefixed values via prepare_for_view()/dojo_crypto_encrypt(), reuses the existing get_db_key() (no key rotation), pages in bounded chunks, skips values that fail to decrypt instead of clobbering them, and is idempotent and reverse-safe. Also adds REMOVAL TRACKING notes in dojo/utils.py spelling out the conditions under which the legacy OFB path can be deleted once no "AES.1" values remain, with bidirectional pointers between the migration and utils. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
the
New Migration
|
As far as I can see it only reencrypts whenever the user changes or saves the value. Which may never happen for many entries. |
valentijnscholten
left a comment
valentijnscholten
left a comment
There was a problem hiding this comment.
I suggest to add something to the upgrade notes to make users aware of this in case they run into problems or are crypto fanatics.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
valentijnscholten
left a comment
valentijnscholten
left a comment
There was a problem hiding this comment.
Thanks. I think in the future when the old algorithms are deleted, we'll have a break of backwards compatibility where users first need to upgrade to the latest version that still can re-encrypt old values.
| updates[field] = dojo_crypto_encrypt(decrypted) | ||
|
|
||
| if updates: | ||
| Tool_Configuration.objects.filter(id=row["id"]).update(**updates) |
There was a problem hiding this comment.
There's a risk here that we'd overflow some of these fields, which will cause the migration to crash. The new encryption scheme includes additional data (auth tag, 16 bytes) that could prevent some encrypted fields that are at the max length from being upgraded.
To replicate, under the dev branch create a tool config with the following password:
password:92WF889R18WRQP3XVPPHSPTCWRCXFVY7VCWPA4QIY7NUJN9GYSZ4DJZ13VLW35TGYRYJTJ1XQFLUERAUQ28PUAT3XGMRNRV4U4L81HZXCPIK7BCAEIU4TU8J17GV2HZBDE67KUR1CY52WDIPTRNQ1P9PG4GRJK2MIJZDUPR60ONIZA6RPOLYPKY8UU3YJFRBJPCVUK7ZFJTT6W87ON9WJHBFD1LJI9G7DF4F1UFUJXEF235NCJ1QYH022L1ND0BYKE1TFP
Save it. (Well -- you'll have to go back and edit it and save it. It looks like encryption doesn't happen on create? Is that a side issue?). Confirm the password in the DB is prefixed with "AES.1."
Then switch to the new branch and run the migration. It will fail with an exception like django.db.utils.DataError: value too long for type character varying(600).
Probably the simplest solution would be to just add an extra ~50 chars to the appropriate model fields.
There was a problem hiding this comment.
Good catch! Will update the migration to expand the field size as well
GCM appends a 12-byte nonce and 16-byte auth tag, so values stored at the old column max length under AES.1 would overflow when the migration re-encrypts them. Grow password/ssh/api_key by 50% (600->900, 6000->9000, 600->900) via AlterField in the existing migration, run before the re-encryption so the larger values fit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4 participants
Summary
Modernizes the encryption used for Tool Config credentials (and any other secret routed through
dojo_crypto_encrypt/prepare_for_view) from AES-256-OFB to AES-256-GCM, in a fully backward-compatible way, and bumps the cryptography stack now that the code is safe for it:cryptography46.0.7 → 49.0.0 (supersedes chore(deps): bump cryptography from 46.0.7 to 49.0.0 #15029)pyopenssl26.2.0 → 26.3.0, which requires cryptography ≥ 49 (supersedes chore(deps): bump pyopenssl from 26.2.0 to 26.3.0 #15032)The legacy scheme used
cryptography.hazmat.primitives.ciphers.modes.OFB. OFB has been moved to thedecrepitmodule and is scheduled for removal fromprimitives, so the current usage emits a deprecation warning and would eventually break under the bump above — which is exactly why the code change and the bump land together here.What changed (
dojo/utils.py)dojo_crypto_encrypt()asAES.2:<nonce_hex>:<ct_hex>(12-byte random nonce, authenticated, no padding).prepare_for_view()reads both formats — branches on the version prefix;AES.2→ GCM, anything else → the legacy OFB path. Existing DB values keep decrypting unchanged.get_db_key()value (valid 32-byte AES-256 key for both schemes), so no key rotation or settings change.AES.1values transparently re-encrypt toAES.2the next time a Tool Config is saved (decrypt-on-GET + encrypt-on-save). No data migration.cryptography.hazmat.decrepit.ciphers.modes(with anImportErrorfallback toprimitives), silencing the deprecation warning and surviving the eventual removal.try; catchesInvalidTag/ValueError/IndexErrorso a tampered or malformed secret returns""instead of raising.The four callers (
tool_config/views.py,api_sonarqube/api_client.py,display_tags.pyget_pwd) go through these helpers and need no changes. DefectDojo does not importpyopenssldirectly — it is a transitive/pinned dependency, so its bump carries no code impact.Testing
New unit tests in
unittests/test_utils.pycover: AES.2 round-trips (ascii/unicode/long/empty), backward-compatible decryption of a legacy AES.1 value, the newAES.2:format prefix, and tampered/garbage input returning"".Verified the edited helpers against cryptography 49.0.0 with
DeprecationWarningpromoted to errors: AES.2 round-trips, legacy AES.1 decrypts with no deprecation warnings, and tamper/garbage/None all degrade to"".🤖 Generated with Claude Code