Skip to content

Feat/python pycryptodome detection - #556

Open
san-zrl wants to merge 24 commits into
mainfrom
feat/python-pycryptodome-detection
Open

san-zrl wants to merge 24 commits into
mainfrom
feat/python-pycryptodome-detection

Conversation

@san-zrl

@san-zrl san-zrl commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Add PyCryptodome/PyCrypto detection support for Python

This PR adds cryptography detection rules for the pycryptodome / pycrypto Python libraries, alongside several mapper and reorganizer improvements needed to support them. This PR completely replaces #495 with an implementation based on the named parameters using the new detection rule functions '.with[Optional]NamedMethodParameter' introduced in #527.

What's changed

  • Mapper model — DigestSize.ofDefault() factory for compile-time-known sizes; RIPEMD cast constructor; SHA2 updated to use ofDefault.
  • Pyca mappers — Extended PycaCipherMapper, PycaDigestMapper, PycaMacMapper with new algorithm entries (DES, RC2, Salsa20, RIPEMD160, MD2/MD4, KangarooTwelve, CSHAKE, TupleHash, …). New shared PycaCurveMapper and PycaKeyBasedAlgorithmMapper.
  • Reorganizer rules — New ECDH→X25519/X448 replacement rules, moveNodesFromUnderFunctionalityUnderParent for Sign/Verify, WRAP_SHA512_TRUNCATED; all wired into PythonReorganizerRules.
  • Detection rules — Pyca rules moved to detection/pyca/ subpackage. New detection/pycrypto/ package with rules for cipher, hash, KDF, key agreement, MAC, public key, random, and signature.
  • Translation — New PycaKeyContextTranslator and PycaRandomContextTranslator; removed dead PycaSecretContextTranslator. Existing context translators extended with PyCryptodome-specific algorithm names, modes, and KDF parameters.
  • Tests — Pyca test fixtures and Java test classes moved to test/pyca/ subpackage. New pycrypto test fixtures and test classes added.

@san-zrl
san-zrl requested a review from a team as a code owner October 1, 2026 14:18
- DigestSize: add ofDefault() factory for compile-time-known sizes and
  propagate NodeOrigin through the copy constructor
- SHA2: use DigestSize.ofDefault() for its built-in digest sizes
- RIPEMD: add cast constructor (required by reorganizer rules)

Signed-off-by: san-zrl <san@zurich.ibm.com>
- PycaCipherMapper: add DES, RC2, Salsa20; add 3DES and CHACHA20_POLY1305
  aliases; add RC4 alias
- PycaDigestMapper: add MD2, MD4, RIPEMD160, TupleHash, Keccak, CSHAKE,
  KangarooTwelve; remove Poly1305 (belongs in MAC mapper)
- PycaMacMapper: add Poly1305 and additional algorithm entries
- PycaCurveMapper: new mapper for elliptic curve names (shared by pyca
  and pycrypto translators)
- PycaKeyBasedAlgorithmMapper: new mapper for key-based algorithm names
  (shared by pyca and pycrypto translators)

Signed-off-by: san-zrl <san@zurich.ibm.com>
…re into Python

- MessageDigestReorganizer: new WRAP_SHA512_TRUNCATED rule
- KeyAgreementReorganizer: add replaceEcdhWithXdh() factory and two
  concrete rules REPLACE_ECDH_WITH_X25519_WHEN_CURVE25519 and
  REPLACE_ECDH_WITH_X448_WHEN_CURVE448
- SignatureReorganizer: add moveNodesFromUnderFunctionalityUnderParent()
  for Sign/Verify x Signature/PSS
- PythonReorganizerRules: wire all new rules and activate
  MessageDigestReorganizer.WRAP_SHA512_TRUNCATED

Signed-off-by: san-zrl <san@zurich.ibm.com>
Move all Pyca* detection rule classes from detection/{aead,asymmetric,...}
into detection/pyca/{aead,asymmetric,...} to make room for the parallel
pycrypto/ package. No logic changes.

Also removes stale pyca test fixtures (.py) and Java test classes that
are superseded by forthcoming pycrypto-aware tests.

Signed-off-by: san-zrl <san@zurich.ibm.com>
Add detection rule classes under detection/pycrypto/:
  cipher, hash, kdf, keyagreement, mac, publickey, random, signature

Register all rule sets in PythonDetectionRules alongside the existing
pyca rules. Update imports to use new pyca/ subpackage paths.

Signed-off-by: san-zrl <san@zurich.ibm.com>
- Add PycaKeyContextTranslator (handles KeyContext) and
  PycaRandomContextTranslator (handles PRNGContext)
- Remove PycaSecretContextTranslator (dead code, superseded by
  PycaKeyContextTranslator)
- In PythonTranslator: route KeyContext after PrivateKey/PublicKey to
  avoid premature matching; add PRNGContext branch

Signed-off-by: san-zrl <san@zurich.ibm.com>
- CipherContextTranslator: add MODE_* aliases, PKCS1_OAEP, PKCS1_V1_5,
  HPKE, EAX, KW, KWP; handle ValueAction for algorithm names
- DigestContextTranslator: handle DigestSize values (SHA-512/t truncation)
- KeyAgreementContextTranslator: accept ValueAction, add X25519/X448
- KeyDerivationContextTranslator: add PBKDF1, KDFCounter, IterationCount,
  SaltSize; add pycrypto-specific KDF bundle key handling
- MacContextTranslator: delegate ValueAction to PycaMacMapper
- PrivateKeyContextTranslator: replace inline curve/algorithm switches
  with PycaCurveMapper and PycaKeyBasedAlgorithmMapper
- PublicKeyContextTranslator: same mapper refactor; add Curve handling
- SignatureContextTranslator: add RSA, DSS, ECDSA, EdDSA, RSA-PKCS1V15

Signed-off-by: san-zrl <san@zurich.ibm.com>
…bpackage

Mirrors the detection rule reorganisation: test fixtures and Java test
classes for pyca rules move from test/detection/{aead,...} into
test/detection/pyca/{aead,...}.

Also renames named_parameters/ test package (was at detection root).

Signed-off-by: san-zrl <san@zurich.ibm.com>
Signed-off-by: san-zrl <san@zurich.ibm.com>
@san-zrl
san-zrl force-pushed the feat/python-pycryptodome-detection branch from 6267ff1 to 71d1ff1 Compare October 1, 2026 14:23

@n1ckl0sk0rtge n1ckl0sk0rtge left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding PyCryptodome coverage. I checked the new rules against the official API docs and traced the mapper values through CBOM output. Several valid calls are missed, and some detected algorithms receive incorrect OIDs or size metadata; I left the concrete cases inline. mvn test -pl python -am -q passes, but some fixtures contain calls that PyCryptodome rejects at runtime.

RuleSets.rulesOf(PythonCryptoMac.class).stream(),
RuleSets.rulesOf(PythonCryptoRandom.class).stream(),
RuleSets.rulesOf(PythonCryptoCipher.class).stream(),
RuleSets.rulesOf(PythonCryptoElGamal.class).stream(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new RSA, DSA, and ECC key rule sets are missing from this production registry. Their tests instantiate each set directly, so those tests pass even though standalone RSA.generate(), DSA.generate(), and ECC.generate() calls will not run these rules in normal scans. Could we register all three and add a test through PythonDetectionRules? These are documented module-level APIs: RSA, DSA, ECC.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSA, DSA, and ECC key rule sets were deliberately omitted from registration as "first-class" objects to avoid duplicate detections. RSA, DSA, and ECC keys are always used in combination with higher-level crypto concepts. One such example are PKCS1v15 signatures.

message = b'To be signed'
key = RSA.import_key(open('private_key.der').read())
h = SHA256.new(message)                              # Noncompliant {{(MessageDigest) SHA-256}}
scheme = pkcs1_15.new(rsa_key=key)                   # Noncompliant {{(Signature) RSA-PKCS1-1.5-SHA-256}}
signature = scheme.sign(h)

The detection rule for pkcs1_15.new(rsa_key=key) adds a dependent detection rule for RSA keys. This is also done for all other crypto use-cases where DSA or ECC key are needed. El Gamal key are special since PyCryptodome explicitly removed support for sign and verify. These detection rules are registered to detect the use of El Gamal since "code that uses such methods is doomed anyway. It should be fixed ASAP to use the algorithms available in Crypto.Signature and Crypto.Cipher."

If we also register RSA, DSA, and ECC key rule sets in PythonDetectionRules we get duplicate detections from the top-level rule and the dependent rule.

@san-zrl san-zrl Oct 16, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: Here's an example what would happen if RSA, DSA, and ECC key rule sets were registered as top-level rules.

You will get two separate RSA key detections in the CBOM. They will not be deduplicated.


Step-by-Step Breakdown

1. How the Two Detections Occur

In PKCS1v15SignTestFile.py:

6 | key = RSA.import_key(open('private_key.der').read())
7 | h = SHA256.new(message)
8 | scheme = pkcs1_15.new(rsa_key=key)
9 | signature = scheme.sign(h)
  1. Top-level Detection (PythonCryptoRSA):

  2. Dependent Detection (PythonCryptoSignature:PKCS1V15):

    • When visiting pkcs1_15.new(rsa_key=key) at line 8, the PKCS1V15 detection rule runs.
    • Because PKCS1V15 registers PythonCryptoRSA as a depending detection rule on the rsa_key parameter (.addDependingDetectionRules(RuleSets.rulesOf(PythonCryptoRSA.class))), the engine traces key back to RSA.import_key(...) and attaches the RSA key finding as a child under the (Signature) RSA-PKCS1-1.5-SHA-256 detection tree.

2. Why Deduplication Does Not Occur in CBOM Generation

  1. Component Identifier in CBOMOutputFile:

    • CBOMOutputFile maps and deduplicates components using their Component.getName().
  2. Random UUID in RelatedCryptoMaterialComponentBuilder:

    • When translating a Key node into a CycloneDX component, RelatedCryptoMaterialComponentBuilder generates a random UUID and names the component:
      stringBuilder.append("@").append(this.uuid);
      this.component.setName(stringBuilder.toString()); // e.g. "key@<UUID>"
    • Because every key component instance generates a distinct UUID, their component names never collide in components.putIfAbsent(componentId, component).

Resulting CBOM Output

The generated CBOM will contain:

  1. First Key Component: A standalone related-crypto-material component ("key@<UUID_1>") from the top-level RSA.import_key(...) detection.
  2. Signature Component: An algorithm component for "RSA-PKCS1-1.5-SHA-256".
  3. Second Key Component: Another related-crypto-material component ("key@<UUID_2>") registered as a dependency child of "RSA-PKCS1-1.5-SHA-256".

(This is why PyCrypto key classes such as PythonCryptoRSA, PythonCryptoDSA, and PythonCryptoECC are kept as dependent rules only and excluded from top-level PythonDetectionRules.)

san-zrl added 11 commits October 5, 2026 11:58
Signed-off-by: san-zrl <san@zurich.ibm.com>
Signed-off-by: san-zrl <san@zurich.ibm.com>
Signed-off-by: san-zrl <san@zurich.ibm.com>
Signed-off-by: san-zrl <san@zurich.ibm.com>
Signed-off-by: san-zrl <san@zurich.ibm.com>
Signed-off-by: san-zrl <san@zurich.ibm.com>
…sh, Cast and ARC2 ciphers

Signed-off-by: san-zrl <san@zurich.ibm.com>
Signed-off-by: san-zrl <san@zurich.ibm.com>
Signed-off-by: san-zrl <san@zurich.ibm.com>
…pper

Signed-off-by: san-zrl <san@zurich.ibm.com>
@san-zrl
san-zrl requested a review from n1ckl0sk0rtge October 6, 2026 12:01
KMAC(int, ...) added a DigestSize of 2 * parameterSetIdentifier, so
KMAC128 carried 256 and KMAC256 carried 512. The CBOM output picks
DigestSize before ParameterSetIdentifier, so those values were
exported as the parameter set.

The 128/256 is the security strength of the underlying cSHAKE, not
the tag length, and the default tag length differs between libraries.
Drop the DigestSize so KMAC128 and KMAC256 export 128 and 256.

Output length is not captured, since CycloneDX has no field for it.

Signed-off-by: san-zrl <san@zurich.ibm.com>
TupleHash(int, ...) stored the 128/256 as a DigestSize, but it is the
variant's security strength; the default digest is 512 bits. The
class also had no asString(), so both variants were named TupleHash.

Store the value as a ParameterSetIdentifier and append it to the
name, as KMAC and cSHAKE do, giving TupleHash128 and TupleHash256.

The Bouncy Castle mapping uses the no-argument constructor and still
reports plain TupleHash.

Signed-off-by: san-zrl <san@zurich.ibm.com>
Assert that KMAC128/256 and TupleHash128/256 are exported under their
own names with parameterSetIdentifier 128 or 256, so the security
strength can no longer be replaced by an output length.

Signed-off-by: san-zrl <san@zurich.ibm.com>
Replace withAnyParameters() on KMAC128, KMAC256 and Poly1305 with the
parameters PyCryptodome documents (KMAC: key, data, mac_len, custom;
Poly1305: key, cipher, nonce, data), each in its own private static
final field.

Capture the Poly1305 cipher and attach it under the MAC, and attach
the CMAC ciphermod the same way HMAC does with digestmod. A Poly1305
with cipher=AES now has an AES child.

Stop binding the CMAC mac_len to a digest size: it is the tag length,
which the CBOM cannot express. The parameter is still accepted.

Signed-off-by: san-zrl <san@zurich.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants