Skip to content

Fix RSA decryption of binary data - #2678

Open
zhangli091011 wants to merge 1 commit into
gchq:masterfrom
zhangli091011:fix/issue-2436-rsa-binary-io
Open

zhangli091011 wants to merge 1 commit into
gchq:masterfrom
zhangli091011:fix/issue-2436-rsa-binary-io

Conversation

@zhangli091011

Copy link
Copy Markdown

Description
Treat RSA ciphertext and plaintext as raw bytes instead of UTF-8 strings. This prevents binary ciphertext from being corrupted before decryption and allows non-UTF-8 plaintext, such as decrypted symmetric keys, to flow into operations like To Hex.

Existing Issue
Closes #2436.

Screenshots
Not applicable; this is an operation data handling fix.

AI disclosure
OpenCode (gpt-5.6-sol) was used to investigate the issue, implement the fix, and add the regression test. I reviewed the diff and ran the full non-UI test suite.

Test Coverage
Added an RSA-OAEP/SHA-256 regression vector covering From Hex -> RSA Decrypt -> To Hex.

  • npm test: 262 Node API tests passed; 2242 operation tests passed
  • npx eslint src/core/operations/RSADecrypt.mjs tests/operations/tests/RSA.mjs

@zhangli091011

Copy link
Copy Markdown
Author

The CLA is complete, but the required main check has not been produced because the fork Pull Requests workflow is awaiting maintainer approval: https://github.com/gchq/CyberChef/actions/runs/29688294131. Could a maintainer please approve the run when convenient?

@sylvesterkaczmarek sylvesterkaczmarek 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.

This looks correct and appropriately scoped. Moving RSA Decrypt to byteArray input/output removes the implicit UTF-8 decode that corrupted arbitrary binary plaintext, while the forge raw encode/decode preserves the exact encrypted/decrypted bytes. The added OAEP-SHA256 regression case exercises genuinely non-text binary output, and the existing round-trip coverage should protect the normal text path. Looks good to me.

@zhangli091011

Copy link
Copy Markdown
Author

Friendly bump: the branch is mergeable and the only missing requirement is the fork workflow approval for the required \main\ check. Could a maintainer approve the run and review when convenient?

This branch has not been deployed

No deployments
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.

RSA Decrypt: "URIError: malformed URI sequence" when piping From Hex → RSA Decrypt (OAEP)

2 participants