Modernize Tool Config credential encryption to AES-256-GCM#15058
Open
Maffooch wants to merge 3 commits into
Open
Modernize Tool Config credential encryption to AES-256-GCM#15058Maffooch wants to merge 3 commits into
Maffooch wants to merge 3 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>
Member
|
No migration? At some point we'll have to I think? |
Contributor
Author
|
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
Member
|
As far as I can see it only reencrypts whenever the user changes or saves the value. Which may never happen for many entries. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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