[cryptor] support AES and SM4 encryption strategies by im47cn · Pull Request #6531 · apache/shenyu

im47cn · 2026-08-01T11:50:01Z

Motivation

The cryptor plugin ships only RSA out of the box. Users needing symmetric ciphers (AES, SM4) have no built-in strategy today; SM4 is also required for GM (国密) compliance.

Modifications

New aes and sm4 strategies (SPI via @Join, names aes / sm4), sharing AbstractAeadCryptorStrategy:
- Authenticated GCM (AES/GCM/NoPadding, SM4/GCM/NoPadding): fresh 96-bit SecureRandom nonce per message, 128-bit tag, output base64(nonce || ciphertext || tag).
- Key format is base64(secret) (16/24/32 bytes for AES, 16 bytes for SM4). SM4 registers/uses BouncyCastle (bcprov-jdk18on).
rsa keeps the legacy PKCS#1 v1.5 padding of previous releases — no breaking change, existing rules stay wire-compatible.
New rsa-oaep strategy (RSAES-OAEP, SHA-256/MGF1 pinned explicitly) for new deployments preferring semantically secure padding; not wire-compatible with rsa.

Tests

Parameterized AES/SM4 round trips (ASCII / CJK / JSON), tamper and invalid-key negative cases, SPI wiring regression tests.
RSA: PKCS#1 v1.5 round trip with the original upstream fixture (proves old ciphertexts still decrypt), OAEP round trip + tamper detection under rsa-oaep.

Aias00

Reviewed #6531. No blockers. Two should-fix items and a few nits.

Should fix

Static IV in CBC (security). AbstractCbcCryptorStrategy reuses the configured IV for every call (buildCipher, lines 62-71), and the documented key format is base64(secret):base64(iv) — i.e. one fixed (key, IV) pair per rule. Reusing a (key, IV) in CBC leaks first-block plaintext equality (identical JSON prefixes → identical first ciphertext blocks across requests) and opens chosen-plaintext paths. I know this matches the existing shenyu-common/AesUtils posture, but for a security plugin it's worth not silently propagating. At minimum, add a Javadoc warning that the IV must be unique per deployment/rule; ideally add a random-IV-per-message variant that prepends the IV to the ciphertext.
No SPI-wiring regression test. Both new tests instantiate the strategy directly (new AesStrategy() / new Sm4Strategy()), so the META-INF SPI registration that the plugin actually depends on (CryptorStrategyFactory.newInstance("aes") → ExtensionLoader.getJoin) is never exercised. If the aes=/sm4= line or the @Join annotation is dropped, these tests still pass and the plugin fails at runtime (silently — the factory catches and returns null). Please add a test that loads via CryptorStrategyFactory.newInstance("aes")/"sm4" and round-trips.

Nits

Key-format divergence from AesUtils (raw UTF-8 string key/iv vs base64(secret):base64(iv)) is unmentioned and will confuse operators using the same secret across shenyu.aes.secret.* and the cryptor aes strategy. A Javadoc cross-reference would help.
Negative tests cover separator/format errors only — no non-base64 content or wrong byte-length (15-byte AES secret, 17-byte SM4 key) cases.
AesStrategyTest/Sm4StrategyTest vs the existing RSAStrategyTest naming — trivial style divergence.

The crypto wiring itself (explicit BC provider via Cipher.getInstance(transformation, "BC"), PKCS7Padding for AES/SM4, per-call Cipher, standard Base64 output decoded by the factory's MIME decoder) is correct, and the @Join/SPI file additions are right. The static-block provider registration with a getProvider null-guard is actually cleaner than AesUtils's unconditional addProvider per call.

…ative test cases Should-fix apache#1: Add Javadoc security warning on IV reuse in CBC mode, including cross-reference to AesUtils key format divergence (nit apache#3). Should-fix apache#2: Add CryptorStrategyFactorySpiTest that loads strategies via CryptorStrategyFactory.newInstance() (exercising META-INF SPI + ExtensionLoader.getJoin), not just direct instantiation. Nit apache#4: Add negative tests for wrong byte-length keys (15-byte AES, 18-byte SM4) and non-base64 content. apache#6531

im47cn · 2026-08-04T17:17:46Z

Thanks @Aias00 for the thorough review. All items addressed in the latest push:

Should-fix #1 (IV reuse Javadoc): Added security warning to AbstractCbcCryptorStrategy class-level Javadoc documenting that the IV is fixed per rule, the CBC reuse risk, and operator guidance to regenerate IVs per deployment/rule.

Should-fix #2 (SPI-wiring regression test): Added CryptorStrategyFactorySpiTest that loads strategies via CryptorStrategyFactory.newInstance("aes") / newInstance("sm4") — exercising the real ExtensionLoader.getJoin + META-INF SPI path, not direct instantiation. If the SPI file or @Join annotation is dropped, these tests fail.

Nit #3 (key format divergence): Added cross-reference in the same Javadoc pointing to AesUtils and warning against interchanging secrets.

Nit #4 (negative test coverage): Added 3 new negative tests: AES 15-byte key (wrong length), SM4 18-byte key (wrong length), and non-base64 key content.

Aias00 · 2026-08-05T14:52:22Z

Strong test coverage (parameterized round-trip incl. CJK/JSON, wrong key-length, non-base64, SPI loadability). The SPI registration and the BouncyCastle provider guard are correct. One security design issue I think should be addressed before merge:

CBC with a fixed IV — IV reused across all messages. AbstractCbcCryptorStrategy.buildCipher parses the IV from the rule's key string (base64(secret):base64(iv)) and uses the same (key, IV) pair for every message encrypted under that rule. A gateway encrypts many requests/responses under one configured rule, so the same IV is reused per-message. CBC IV reuse leaks first-block plaintext equality and opens chosen-plaintext attack paths (CWE-329/CWE-1204) — and gateway request bodies are often partially attacker-controlled, which makes the CPA path reachable. The Javadoc acknowledges this ("Security note on IV reuse") and mentions a random-IV-per-message variant as a future enhancement, but I think it should be the default design rather than a TODO. Suggested fix: generate a fresh IV via SecureRandom per encrypt, prepend it to the ciphertext (base64(iv || ciphertext)), and parse it on decrypt — or switch to AES/GCM/NoPadding, which also gives authenticated encryption and resolves the malleability concern below.

CBC without authentication (malleable). AES/CBC/PKCS7Padding with no MAC means an attacker who can modify ciphertext in transit can flip bits to affect the decrypted plaintext (CBC bit-flipping). AES-GCM (above) would solve this; alternatively CBC+HMAC. The existing RSA strategy is also encryption-only, so this isn't a regression, but for new strategies it's worth flagging.

Minor: key material (base64(secret):base64(iv)) lives in the rule config and flows through admin/data-sync — consistent with RSA, so not a regression, but there's no KMS/vault integration for the new symmetric secrets. Also a small nit: encrypt uses Base64.getEncoder() (standard) while key/ciphertext parsing uses getMimeDecoder() (lenient, ignores line separators) — getDecoder() would be stricter and symmetric.

The cryptor plugin shipped only RSA out of the box. Add AesStrategy and Sm4Strategy so the CryptorStrategy SPI covers symmetric ciphers too. A shared AbstractCbcCryptorStrategy encapsulates the CBC wiring -- key parsing, SecretKeySpec/IvParameterSpec init and BouncyCastle provider registration -- while subclasses merely declare the transformation and algorithm name. Key convention: base64(secret):base64(iv), with a fixed AES|SM4/CBC/PKCS7Padding transformation. The CryptorStrategy interface, CryptorRuleHandler and admin stay untouched, so RSA and existing rules keep working. Tests: parameterized round-trip (ASCII/CJK/JSON payloads) plus invalid key-format cases. New strategy classes at 100% instruction coverage.

…ative test cases Should-fix apache#1: Add Javadoc security warning on IV reuse in CBC mode, including cross-reference to AesUtils key format divergence (nit apache#3). Should-fix apache#2: Add CryptorStrategyFactorySpiTest that loads strategies via CryptorStrategyFactory.newInstance() (exercising META-INF SPI + ExtensionLoader.getJoin), not just direct instantiation. Nit apache#4: Add negative tests for wrong byte-length keys (15-byte AES, 18-byte SM4) and non-base64 content. apache#6531

Replace CBC with a fixed IV by authenticated encryption throughout the cryptor plugin to close the IV-reuse (CWE-329) and malleability (CWE-1204) issues raised in review. AES/SM4: - Switch AES/CBC/PKCS7Padding and SM4/CBC/PKCS7Padding to GCM/NoPadding with a fresh 96-bit SecureRandom nonce per message and a 128-bit tag, emitted as base64(nonce || ciphertext || tag). Key format simplified to base64(secret) — GCM must never reuse a fixed IV, so the configured IV is removed and the legacy base64(secret):base64(iv) form is rejected. RSA: - Default 'rsa' strategy upgraded from PKCS#1 v1.5 to RSA/ECB/OAEPWithSHA-256AndMGF1Padding, with an explicit OAEPParameterSpec (MGF1 SHA-256) so the transformation is identical across JDKs/providers. - New 'rsa-pkcs1' strategy keeps PKCS#1 v1.5 for legacy/external peers that cannot speak OAEP. Shared logic factored into AbstractRsaStrategy. Misc: - Unify Base64 encoder/decoder; decrypt now decodes UTF-8 explicitly. - RSA test fixtures moved from 512-bit to 2048-bit (OAEP/SHA-256 cannot encrypt any plaintext under a 512-bit key). - Refactor CryptorRequestPluginTest to shared key constants (DRY). https://github.com/apache/shenyu/pr/6531

im47cn · 2026-08-06T07:35:35Z

Thanks @Aias00 — your points on IV reuse and missing authentication were spot on. Rather than documenting the risk, this push closes it. All three items addressed:

1. CBC IV reuse (CWE-329) → resolved by switching to GCM.
AES and SM4 now use AES/GCM/NoPadding / SM4/GCM/NoPadding with a fresh 96-bit SecureRandom nonce per message and a 128-bit tag. Output is base64(nonce ‖ ciphertext ‖ tag), so the nonce is unique per message — no fixed IV to reuse. The base64(secret):base64(iv) key form is gone (GCM must never reuse a fixed IV); the key is now just base64(secret), and the legacy secret:iv form is explicitly rejected with a clear error.

2. CBC malleability (CWE-1204) → resolved by GCM's authentication tag.
Any in-transit modification now fails decryption (AEADBadTagException) instead of silently producing corrupted plaintext. New tests assert both properties: shouldEmitDifferentCiphertextForRepeatedEncrypts (nonce freshness) and shouldFailAuthenticationWhenCiphertextIsTampered (integrity).

3. Base64 encoder/decoder asymmetry → fixed. Unified to getEncoder()/getDecoder() throughout; decrypt now decodes UTF-8 explicitly.

RSA — PKCS#1 v1.5 → OAEP, with a PKCS#1 fallback for your non-regression concern.
To address the padding-oracle weakness without breaking external peers that only speak PKCS#1 v1.5:

The default rsa strategy is now RSA/ECB/OAEPWithSHA-256AndMGF1Padding, with an explicit OAEPParameterSpec (MGF1 also SHA-256) so the transformation is identical across JDKs/providers.
A new rsa-pkcs1 strategy keeps PKCS#1 v1.5 — any rule interoperating with a legacy/external PKCS#1 system can opt in by strategy name, with no code change and no surprise breakage.

Shared RSA logic is factored into AbstractRsaStrategy; AbstractCbcCryptorStrategy is removed.

Note: RSA test fixtures moved from 512-bit to 2048-bit — OAEP/SHA-256 physically cannot encrypt any plaintext under a 512-bit key (keyBytes − 2·32 − 2 < 0), so the old fixture is no longer usable. AES/SM4 round-trip tests cover CJK + JSON payloads.

The full cryptor module suite (40 tests) is green locally (main project install + integrated-test test-compile both pass), including the SPI-wiring path you flagged earlier. Would appreciate another look when you have time.

Aias00

Review: add AES/SM4 symmetric ciphers + RSA OAEP hardening

Verdict: ✅ Approved (with two important notes the maintainer should consciously sign off on).

This is a high-quality, security-focused PR. The crypto design is sound and the tests are meaningful. Two points need addressing — both about communication/documentation, not code correctness — because the PR's own "no breaking changes" claim is inaccurate.

What's done well

Secure AEAD design. AbstractAeadCryptorStrategy draws a fresh SecureRandom 96-bit nonce per message, prepends it to ciphertext||tag, Base64-wraps the lot (base64(nonce || ct || tag)). This is the correct pattern and defeats GCM IV-reuse (CWE-329).
parseSecret rejects the legacy base64(secret):base64(iv) form for GCM (lines 209-216). Good defensive choice — it stops an operator from accidentally configuring a fixed IV, which would catastrophically leak the secret under GCM.
Tamper detection is real and tested. GCM's auth tag makes the ciphertext non-malleable; the tests shouldFailAuthenticationWhenCiphertextIsTampered / shouldFailWhenCiphertextIsTampered flip a bit and expect failure. Same for OAEP (RSAStrategyTest.shouldFailWhenCiphertextIsTampered).
RSA hardening. RsaStrategy now pins OAEPParameterSpec("SHA-256","MGF1",SHA256,PSpecified.DEFAULT), avoiding the JDK's default that pairs a SHA-256 label with a SHA-1 MGF1. OAEP is semantically secure vs the old PKCS#1 v1.5.
Clean SPI refactor. AbstractRsaStrategy + RsaPkcs1Strategy share wiring; rsa-pkcs1 is provided as a compatibility shim. SPI file + @Join wiring is correct, and CryptorStrategyFactorySpiTest verifies resolution via the factory (not just direct instantiation).
BouncyCastle dependency is version-managed (bcprov-jdk18on.version=1.84 in root pom dependencyManagement), so the unversioned <dependency> resolves and compiles. No build risk.

Note 1 — the default `rsa` strategy is now wire-incompatible (contradicts "no breaking changes")

The original RsaStrategy called Cipher.getInstance("RSA") → RSA/ECB/PKCS1Padding (PKCS#1 v1.5). This PR changes the rsa SPI name to OAEP (RSA/ECB/OAEPWithSHA-256AndMGF1Padding). That is a wire-format breaking change: any existing peer/client that encrypts with the old PKCS#1 v1.5 scheme can no longer interoperate with the upgraded gateway (and vice-versa).

The class javadoc correctly admits this ("Not wire-compatible with PKCS#1 v1.5") and the rsa-pkcs1 shim mitigates it — but the PR description still claims "No breaking changes: … existing RSA rules are untouched." The rule config is untouched, yet the on-the-wire format of the rsa strategy changed. This must be called out in the release notes, and the description's claim should be corrected. OAEP is the right default (strict security improvement) and a compatibility path exists, so this isn't a blocker — but it needs explicit maintainer acknowledgement, not a silent upgrade.

Note 2 — PR description is stale vs the implementation (CBC → GCM)

The "Modifications" section describes a different (earlier) design:

Shared AbstractCbcCryptorStrategy encapsulates CBC wiring … Key convention base64(secret):base64(iv), fixed AES|SM4/CBC/PKCS7Padding.

The shipped code is AbstractAeadCryptorStrategy using GCM (AES/GCM/NoPadding, SM4/GCM/NoPadding) with key base64(secret) (no IV; random per-message nonce). The GCM design in the code is actually better and safer than the CBC design the text describes — but a reviewer reading the description would be misled (and a fixed IV under CBC would itself be a security smell). Please update the description to match the actual GCM implementation before merge.

Minor / non-blocking

Security.addProvider(new BouncyCastleProvider()) (static block, line 153-157) registers BC globally at the lowest preference. It's idempotent-guarded and Shenyu already uses BC elsewhere, so this is acceptable — just noting it's a global side effect.
AbstractRsaStrategy relies on built-in SunJCE (Cipher.getInstance(transformation()) with no provider arg) for RSA, which is fine.

Bottom line

Code is correct, secure, and well-tested. Please (a) correct the "no breaking changes" claim and add a release note about the rsa PKCS#1 v1.5 → OAEP wire change (use rsa-pkcs1 to keep old peers working), and (b) update the description's CBC/base64(secret):base64(iv) text to the real GCM/base64(secret) design. After those, this is good to merge.

Aias00 · 2026-08-14T14:12:57Z

hi, pls add my wechat: aias00

+    private Cipher buildCipher(final int mode, final byte[] secret, final byte[] nonce) throws Exception {
+        Cipher cipher = Cipher.getInstance(getTransformation(), BouncyCastleProvider.PROVIDER_NAME);
+        cipher.init(mode, new SecretKeySpec(secret, getAlgorithm()),
+                new GCMParameterSpec(TAG_LENGTH_BITS, nonce));


Aias00 · 2026-08-15T06:53:47Z

@im47cn hi some security issue should be handled

Aias00 · 2026-09-18T23:32:45Z

Heads-up (PMC Aias00): your PR's CI failures are in shared infrastructure checks (k8s-examples-http, e2e-case-logging-rocketmq, build-docker-images) that are currently failing across many unrelated PRs. This looks like a CI-environment issue, not a problem with your change. Please re-run the failed checks; the code change itself appears fine. No code changes needed for these specific failures.

Aias00

The existing rsa SPI name is changed from PKCS#1 v1.5 to OAEP while the default admin configuration still uses strategyName=rsa. This breaks wire compatibility for existing peers. Please preserve rsa as the legacy strategy and add a new rsa-oaep name, or provide an explicit compatible migration path.

Aias00 · 2026-10-11T15:09:58Z

Hi @im47cn, this PR has been in Changes Requested since my earlier review and there has been no follow-up push addressing the requested fixes (last commit 2026-08-06).

To keep the review queue healthy, please either:

(a) push the requested changes and request a re-review, or
(b) rebase onto the latest master if it has diverged.

If there are no plans to continue, we may close it for staleness — happy to reopen if you pick it up later. Thanks!

Address review: the default `rsa` strategy now uses PKCS#1 v1.5 again, identical to previous releases, so existing rules stay wire-compatible. OAEP (SHA-256/MGF1) moves to a new `rsa-oaep` SPI name; RSA unit and plugin tests are restored to their upstream fixtures, which doubles as a compatibility check (old ciphertexts still decrypt).

Aias00

Approving. b5484430 resolves the blocking issue exactly as asked, and it does it by inverting the default rather than by adding a compatibility shim on top:

rsa is back to RSA/ECB/PKCS1Padding with params() == null, so every existing rule configured with strategyName=rsa keeps the wire format it had before this PR. No migration, no silent interop break.
OAEP moves to the new rsa-oaep SPI name (RsaOaepStrategy), still pinning OAEPParameterSpec("SHA-256","MGF1",MGF1ParameterSpec.SHA256,PSpecified.DEFAULT).
rsa-pkcs1 and RsaPkcs1Strategy are gone, and no reference to either survives anywhere in the tree — I grepped the full diff. Since rsa-pkcs1 never shipped in a release, dropping the name costs nothing.
The Javadoc now states the direction of the compatibility guarantee on all three classes (AbstractRsaStrategy, RsaStrategy, RsaOaepStrategy), which was the actual defect: the previous text claimed compatibility while changing the default.
Tests follow the split correctly: RSAStrategyTest is back on the legacy 512-bit fixture (PKCS#1 v1.5 can use it; OAEP/SHA-256 cannot), RsaOaepStrategyTest carries the 2048-bit fixture and the tamper-detection assertion, and CryptorStrategyFactorySpiTest resolves rsa-oaep through the SPI so a dropped @Join or SPI line still fails.

Non-blocking observations, all fine to leave for a follow-up:

RSAStrategyTest switched to Base64.getMimeDecoder(). It is required for the hardcoded ciphertext constant if it is line-wrapped, but it is also applied to cryptorStrategy.encrypt(...) output, and the MIME decoder silently skips non-alphabet characters — a corrupted ciphertext would decode to shorter input instead of failing. Plain getDecoder() on the encrypt output would keep that check meaningful.
RsaStrategy (PKCS#1 v1.5) is still the default while the secure option is opt-in. That is the right call for compatibility, but it means the security improvement only lands for operators who read the Javadoc. Worth one line in the plugin docs / a note in the PR description so the rsa → rsa-oaep migration is discoverable.
PKCS#1 v1.5 has no integrity check, so rsa decrypts tampered ciphertext to garbage rather than failing. The test split documents this correctly; just make sure nobody later "restores" the tamper test on RSAStrategyTest and expects it to pass.

One CI note, not a code problem: b5484430 has no check runs at all — gh pr checks reports "no checks reported on the 'feature/cryptor-aes-sm4-support' branch", and actions/runs?head_sha=b5484430 is empty. This is a fork PR, so the push did not start a workflow. Please push an empty commit or ask a maintainer to approve the run so pr_build and the rat check execute before merge.

+public class RsaStrategy extends AbstractRsaStrategy {

-    private static final String RSA = "rsa";
+    private static final String TRANSFORMATION = "RSA/ECB/PKCS1Padding";


-        cipher.init(Cipher.DECRYPT_MODE, priKey);
-        return new String(cipher.doFinal(encryptData));
+    protected String transformation() {
+        return TRANSFORMATION;


Aias00 · 2026-10-24T07:49:40Z

@im47cn This PR is approved, but the only red check is CodeQL. That is a separate scan from the build, and today it is also red on master, so it is not reading your diff.

Please rebase onto master and push to re-trigger it; if it stays red, tell me and I'll pull the analysis result.

Aias00 · 2026-10-25T01:04:03Z

@im47cn — a correction to the note I left here yesterday. The failures on this branch all predate the fix, so nothing in your change is implicated.

master went green at a8080ae: the SubscribeTest fixture isolation (#8092), the MQTT session scoping (#8093) and the pr_build dependency fix (#8075) all landed, and the latest ci.yml, integrated-test.yml and CodeQL runs on master are all success.

That means the red checks recorded here are stale. A push — an empty commit or a rebase, whichever is easier — re-runs them against the current baseline. I would not spend time reading the old logs.

Aias00 · 2026-10-26T09:45:03Z

Hi @im47cn, this PR is already approved, but it currently cannot be merged. Here is the blocker and what is needed:

Blocker
The CodeQL analysis check is failing on this PR. No compile/test job is failing, so this looks like a CodeQL workflow/scan issue rather than a defect in your diff.

What we need
Please open the CodeQL run log and confirm whether it is a scanner config/timeout or a real finding, then re-run the check once resolved.

Once the branch is up to date and CI is green, we will squash-merge it. Thanks!

Aias00 · 2026-10-26T12:16:30Z

Root cause of the CodeQL failure, with a per-alert breakdown. Short version: one of these is a false positive on your new code, and the rest are the legacy strategies you deliberately kept — so I do not think you need to change any logic, but the PR should explain them.

CodeQL fails with "3 new alerts including 3 high severity security vulnerabilities". The alerts reported against this PR:

Location	Rule	Assessment
`AbstractAeadCryptorStrategy.java:137`	`java/static-initialization-vector`	False positive
`RsaStrategy.java:37, 41`	`java/rsa-without-oaep`	Pre-existing legacy path
`RsaPkcs1Strategy.java:37, 41`	`java/rsa-without-oaep`	Pre-existing legacy path
`AbstractCbcCryptorStrategy.java:91`	`java/weak-cryptographic-algorithm`	Pre-existing legacy path

Why the AEAD alert is a false positive. Line 137 is new GCMParameterSpec(TAG_LENGTH_BITS, nonce) inside buildCipher. TAG_LENGTH_BITS is the static 128 constant, and the rule appears to key on a static value reaching the GCMParameterSpec constructor. The IV itself is not static:

encrypt draws a fresh 96-bit nonce per call: byte[] nonce = new byte[NONCE_LENGTH_BYTES]; SECURE_RANDOM.nextBytes(nonce);
the wire format is base64(nonce || ciphertext || tag), so the nonce travels with each message
decrypt reads the nonce back out of that prefix
parseSecret explicitly rejects the legacy base64(secret):base64(iv) compound form so an operator cannot pin a fixed IV that GCM cannot honour

That is the correct construction, and the class javadoc already states the CWE-329 reasoning. Nothing to fix.

Why the other three are not new. RsaStrategy (PKCS#1 v1.5) and the CBC strategy are the compatibility paths this PR intentionally preserves — the same point I raised in my earlier review, where I asked you to keep rsa as the legacy name rather than repointing it at OAEP. CodeQL reports them because the file is touched by the PR, not because the PR introduced them.

What to do: dismiss the AbstractAeadCryptorStrategy alert as a false positive with the nonce argument above, and either dismiss or add a comment on the legacy-path alerts noting they are the deliberately retained compatibility strategies. Then re-run CodeQL. No logic change should be needed.

Separately, my earlier blocking item is still open and unrelated to CodeQL: the existing rsa SPI name was repointed from PKCS#1 v1.5 to OAEP while the default admin configuration still uses strategyName=rsa, which breaks wire compatibility for existing peers. Keep rsa on the legacy implementation and expose OAEP under a new name, or document an explicit migration path.

Aias00 reviewed Aug 2, 2026

View reviewed changes

github-advanced-security Bot found potential problems Aug 3, 2026

View reviewed changes

Comment thread ...ptor/src/main/java/org/apache/shenyu/plugin/cryptor/strategy/AbstractCbcCryptorStrategy.java Fixed

im47cn added 3 commits August 6, 2026 07:28

im47cn force-pushed the feature/cryptor-aes-sm4-support branch from a2da12d to 88b310d Compare August 6, 2026 07:28

Merge branch 'master' into feature/cryptor-aes-sm4-support

2468f91

Aias00 approved these changes Aug 14, 2026

View reviewed changes

github-advanced-security Bot found potential problems Aug 15, 2026

View reviewed changes

Aias00 added 2 commits August 18, 2026 22:33

Merge branch 'master' into feature/cryptor-aes-sm4-support

92a0918

Merge branch 'master' into feature/cryptor-aes-sm4-support

6342b0c

Aias00 added 2 commits September 20, 2026 04:56

Merge branch 'master' into feature/cryptor-aes-sm4-support

bda0a27

Merge branch 'master' into feature/cryptor-aes-sm4-support

67c750f

Aias00 requested changes Oct 4, 2026

View reviewed changes

Aias00 approved these changes Oct 21, 2026

View reviewed changes

github-advanced-security Bot found potential problems Oct 24, 2026

View reviewed changes

Conversation

im47cn commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Tests

Uh oh!

Aias00 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

im47cn commented Aug 4, 2026

Uh oh!

Aias00 commented Aug 5, 2026

Uh oh!

im47cn commented Aug 6, 2026

Uh oh!

Aias00 left a comment

Choose a reason for hiding this comment

Review: add AES/SM4 symmetric ciphers + RSA OAEP hardening

What's done well

Note 1 — the default rsa strategy is now wire-incompatible (contradicts "no breaking changes")

Note 2 — PR description is stale vs the implementation (CBC → GCM)

Minor / non-blocking

Bottom line

Uh oh!

Aias00 commented Aug 14, 2026

Uh oh!

Uh oh!

Uh oh!

Aias00 commented Aug 15, 2026

Uh oh!

Aias00 commented Sep 18, 2026

Uh oh!

Aias00 left a comment

Choose a reason for hiding this comment

Uh oh!

Aias00 commented Oct 11, 2026

Uh oh!

Aias00 left a comment

Choose a reason for hiding this comment

Uh oh!

Aias00 commented Oct 24, 2026

Uh oh!

Aias00 commented Oct 25, 2026

Uh oh!

Aias00 commented Oct 26, 2026

Uh oh!

Aias00 commented Oct 26, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

im47cn commented Aug 1, 2026 •

edited

Loading

Note 1 — the default `rsa` strategy is now wire-incompatible (contradicts "no breaking changes")