Feat/shangmi resolves #576 by mwiede · Pull Request #1026 · mwiede/jsch

mwiede · 2026-03-31T18:21:54Z

integrates Shangmi algorithms using bouncycastle classes. Details see ShangMi.md

Implements the Chinese national cryptographic standards (GM/T) for use with OpenEuler and other SSH servers supporting the SM algorithm suite: - SM4 cipher: sm4-cbc and sm4-ctr modes (bc/SM4CBC, bc/SM4CTR) - SM3 hash and MAC: hmac-sm3 and hmac-sm3-etm@openssh.com (bc/SM3, bc/HMACSM3, bc/HMACSM3ETM) - SM2 host key and client authentication: key type "sm2" using SEC1 EC private keys with OID 1.2.156.10197.1.301 (KeyPairSM2, bc/SignatureSM2, SignatureSM2 interface) - HostKey: new SM2 = 8 type constant, GUESS logic for "sm2" blobs - KeyExchange.verify(): handles "sm2" host key algorithm - KeyPair.load(): detects SM2 public keys, overrides ECDSA type (BEGIN EC PRIVATE KEY) when public key file starts with "sm2" - JSch.java: registers all SM algorithms, adds sm2 to PubkeyAcceptedAlgorithms and server_host_key defaults - Integration test AlgorithmsShangMiIT against openEuler Docker image - Unit tests ShangMiTest for SM primitives All SM algorithms are BouncyCastle-only and opt-in via CheckCiphers/ CheckMacs/CheckSignatures runtime availability checks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Implements the sm2-sm3 KEX algorithm used by OpenEuler's OpenSSH, which uses SM2 KAP (GM/T 0003.3) instead of standard ECDH. The shared secret is computed via a manual SM2 KAP implementation that directly mirrors OpenEuler's sm2_kap_compute_key() in kexsm2.c: - Degenerate config: static key == ephemeral key for both parties - KAP identity: raw bytes {1,2,...,8,1,2,...,8} - KDF: SM3(xU || yU || ZA || ZB || counter) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Correct KEX description: SM2 KAP (GM/T 0003.3), not plain ECDH - Add dedicated section explaining the degenerate SM2 KAP configuration used by OpenEuler, including the full shared-secret derivation formula - Note the KAP identity value vs. the signature identity value - Add caveat about BC's SM2KeyExchange divergence from OpenEuler - Add sm2-sm3 and ecdh-sm2p256v1 to the config key reference table Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

BC's SM2KeyExchange.calculateKey(kLen) expects kLen in bits. The original failure was caused by passing 32 (bits = 4 bytes) instead of 256 (bits = 32 bytes). Add ECDHSM2BC (BC-based, kLen=256) and a unit test that asserts manual ECDHSM2 and ECDHSM2BC produce identical shared secrets. Correct the note in ShangMi.md accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Use BC's SM2KeyExchange (kLen=256 bits) as the production implementation. The manual ECDHSM2 remains in the codebase as reference. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

norrisjeremy · 2026-03-31T18:27:01Z

src/main/java/com/jcraft/jsch/JSch.java

-    // by certificate authorities (CAs). Default matches OpenSSH 8.2+ (excludes ssh-rsa/SHA-1).
-    config.put("ca_signature_algorithms", Util.getSystemProperty("jsch.ca_signature_algorithms",
-        "ssh-ed25519,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,rsa-sha2-512,rsa-sha2-256"));
+        "ssh-ed25519,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521,rsa-sha2-512,rsa-sha2-256,sm2"));


I don't think we should add sm2 to the default server_host_key config:

It causes us to diverge from OpenSSH.

This algorithm isn't officially standardized by IETF and violates IETF standards, by which non vendor specific algorithm names are supposed to be registered to IANA (see https://www.iana.org/assignments/ssh-parameters/ssh-parameters.xhtml) before use.

To date, we've kept the default algorithms that are enabled to be aligned with those that have native JCE implementations and avoided added any that are only supported via Bouncy Castle.

sure this makes sense. Will remove it

norrisjeremy · 2026-03-31T18:30:56Z

src/main/java/com/jcraft/jsch/JSch.java

@@ -256,26 +256,17 @@ public class JSch {
    config.put("use_sftp_write_flush_workaround",
        Util.getSystemProperty("jsch.use_sftp_write_flush_workaround", "yes"));

-    config.put("CheckCiphers",
-        Util.getSystemProperty("jsch.check_ciphers", "chacha20-poly1305@openssh.com"));
-    config.put("CheckMacs", Util.getSystemProperty("jsch.check_macs", ""));
+    config.put("CheckCiphers", Util.getSystemProperty("jsch.check_ciphers",
+        "chacha20-poly1305@openssh.com,sm4-cbc,sm4-ctr"));
+    config.put("CheckMacs", Util.getSystemProperty("jsch.check_macs", "hmac-sm3"));
    config.put("CheckKexes", Util.getSystemProperty("jsch.check_kexes",
-        "mlkem768x25519-sha256,mlkem768nistp256-sha256,mlkem1024nistp384-sha384,sntrup761x25519-sha512,sntrup761x25519-sha512@openssh.com,curve25519-sha256,curve25519-sha256@libssh.org,curve448-sha512"));
+        "mlkem768x25519-sha256,mlkem768nistp256-sha256,mlkem1024nistp384-sha384,sntrup761x25519-sha512,sntrup761x25519-sha512@openssh.com,curve25519-sha256,curve25519-sha256@libssh.org,curve448-sha512,sm2-sm3"));
    config.put("CheckSignatures",
-        Util.getSystemProperty("jsch.check_signatures", "ssh-ed25519,ssh-ed448"));
+        Util.getSystemProperty("jsch.check_signatures", "ssh-ed25519,ssh-ed448,sm2"));


I would vote against adding these algorithms by default into the various CheckXYZ configs, as I don't imagine they will be commonly used or encountered by many (most) JSch users, and we've had feedback about how this algorithm checking can be expensive to perform for many algorithms.

mwiede · 2026-03-31T18:42:45Z

ci should not work as of now because building the server docker images takes very long (18min on my local only for updating repo package information). not sure how long it will take on gh infrastructure though

norrisjeremy · 2026-03-31T18:46:07Z

ci should not work as of now because building the server docker images takes very long (18min on my local only for updating repo package information). not sure how long it will take on gh infrastructure though

Not sure I fully understand: does it take a long time because of the introduction of these new algorithms in this PR?
Or do you mean in general, the build times take too long even w/o these changes?

norrisjeremy

Do you know what version of Bouncy Castle first introduced each of these algorithms?
We want to make sure that the OSGi metadata configured in bnd-maven-plugin that specifies a minimum compatible Bouncy Castle of 1.79 is still accurate, or if we need to bump it to a newer Bouncy Castle version.

norrisjeremy · 2026-03-31T18:47:01Z

src/main/java/com/jcraft/jsch/bc/ECDHSM2.java

+    ECPoint U = Q_peer.multiply(Xp_bar).add(Q_peer).multiply(t).normalize();
+
+    if (U.isInfinity()) {
+      throw new Exception("SM2 KAP: shared point is at infinity");


Do we want to throw any sort of more specific Exception type besides a generic Exception?

this is dead code. It's only left to be able to test the calculation correctness in src/test/java/com/jcraft/jsch/bc/ECDHSM2ComparisonTest.java The active class is the ECDHSM2BC one.

So is it intended for users to never use ECDHSM2 and to instead only use ECDHSM2BC?
If so, would it be easer to just move ECDHSM2 as a static inner class in the test class, so that it isn't part of the published releases?

Move testECDHSM2ManualMatchesBC to com.jcraft.jsch.bc package so it can call package-private initForTest() on ECDHSM2/ECDHSM2BC directly, eliminating the setAccessible() calls that fail the forbiddenapis check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

norrisjeremy · 2026-04-01T10:40:38Z

src/test/java/com/jcraft/jsch/AlgorithmsShangMiIT.java

+          .withFileFromClasspath("ssh_host_sm2_key.pub", "docker/ssh_host_sm2_key.pub")
+          .withFileFromClasspath("sshd_config.shangmi", "docker/sshd_config.shangmi")
+          .withFileFromClasspath("authorized_keys", "docker/authorized_keys")
+          .withFileFromClasspath("Dockerfile", "docker/Dockerfile.openeuler.shangmi"))


This Dockerfile seems to rely upon a locally generated image tag: doesn't this mean anyone that tries to run the ITs locally will encounter failures unless they manually build the base image from Dockerfile.openeuler.shangmi.base?

Perhaps we could mark these ITs with a JUnit tag that is such that they aren't run by default so that users attempting to run ITs locally won't automatically fail?

We could still setup the GHA workflow to go ahead and run ITs with that tag so that the CI pipeline would continue to test these algorithms.

For example, I haven't tested this to confirm, but I think we might be able to do something like this:

Add a @Tag("openeuler") on the AlgorithmsShangMiIT class.

In the maven-failsafe-plugin configuration in the pom.xml, add <excludedGroups>openeuler</excludedGroups>.

In the profiles in the pom.xml, add a manually activated profile named openeuler for maven-failsafe-plugin that overrides the excludedGroups as <excludedGroups></excludedGroups>.

In the GHA workflow, make sure the Test with Maven step selects the openeuler profile added above (-Pcoverage,openeuler).

yes this Dockerfile.openeuler.shangmi.base handling is still open.
Now I have setup the GHA cache and it is working. Building the base image took 6 minutes here and now it's in the cache.

I understand your idea using the exclusion of the test.

Another approach would be to build the Dockerfile.openeuler.shangmi.base by some other maven-plugin like jib or fabric8io-docker before running IT-Tests. This would mean, the first run somebody does on his local machine would take time, but once it is build, it would not waste time.

Here is my suggestion:

diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index b01550d..681805e 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -97,7 +97,7 @@ jobs: run: | LOWER_JDK="${{ matrix.java }}" UPPER_JDK=$((LOWER_JDK+1)) - ./mvnw -B -V -e -Pcoverage verify -Dtoolchain.jdk.version="[$LOWER_JDK,$UPPER_JDK)" -Dmaven.resources.skip=true -Dflatten.skip=true -Dmaven.main.skip=true -Dbnd.skip=true -Dassembly.skipAssembly=true -Dmaven.javadoc.skip=true -Dcyclonedx.skip=true -Dspdx.skip=true -Dformatter.skip=true -Dforbiddenapis.skip=true -DskipTests=false -DskipITs=false + ./mvnw -B -V -e -P'coverage,!disabled_tests' verify -Dtoolchain.jdk.version="[$LOWER_JDK,$UPPER_JDK)" -Dmaven.resources.skip=true -Dflatten.skip=true -Dmaven.main.skip=true -Dbnd.skip=true -Dassembly.skipAssembly=true -Dmaven.javadoc.skip=true -Dcyclonedx.skip=true -Dspdx.skip=true -Dformatter.skip=true -Dforbiddenapis.skip=true -DskipTests=false -DskipITs=false - name: Upload test results uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 with: diff --git a/pom.xml b/pom.xml index fef295b..0be80d5 100644 --- a/pom.xml +++ b/pom.xml @@ -805,6 +805,24 @@ </plugins> </build> </profile> + <profile> + <id>disabled_tests</id> + <activation> +  + <jdk>[8,)</jdk> + </activation> + <build> + <plugins> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-failsafe-plugin</artifactId> + <configuration> + <excludedGroups>openeuler</excludedGroups> + </configuration> + </plugin> + </plugins> + </build> + </profile> <profile> <id>errorprone</id> <build> diff --git a/src/test/java/com/jcraft/jsch/AlgorithmsShangMiIT.java b/src/test/java/com/jcraft/jsch/AlgorithmsShangMiIT.java index 816e4c4..fbef5f1 100644 --- a/src/test/java/com/jcraft/jsch/AlgorithmsShangMiIT.java +++ b/src/test/java/com/jcraft/jsch/AlgorithmsShangMiIT.java @@ -7,7 +7,6 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import com.github.valfirst.slf4jtest.LoggingEvent; import com.github.valfirst.slf4jtest.TestLogger; import com.github.valfirst.slf4jtest.TestLoggerFactory; - import java.io.IOException; import java.io.OutputStream; import java.nio.file.Files; @@ -17,11 +16,11 @@ import java.util.Base64; import java.util.List; import java.util.Locale; import java.util.Random; - import org.apache.commons.codec.digest.DigestUtils; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; @@ -37,6 +36,7 @@ import org.testcontainers.junit.jupiter.Testcontainers; * Integration tests for ShangMi (SM4/SM3) cipher and MAC support. Uses an openEuler SSH server that * supports SM4 ciphers and HMAC-SM3 MACs natively. */ +@Tag("openeuler") @Testcontainers class AlgorithmsShangMiIT {

yes this Dockerfile.openeuler.shangmi.base handling is still open. Now I have setup the GHA cache and it is working. Building the base image took 6 minutes here and now it's in the cache.

I understand your idea using the exclusion of the test.

Another approach would be to build the Dockerfile.openeuler.shangmi.base by some other maven-plugin like jib or fabric8io-docker before running IT-Tests. This would mean, the first run somebody does on his local machine would take time, but once it is build, it would not waste time.

I think doing something like that sounds fragile; I think it would be far simpler to just tag the tests so they aren't run by default, then change GHA workflow to toggle the test on for GHA runs.

sonarqubecloud · 2026-04-01T11:44:00Z

Quality Gate passed

Issues
61 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
14.0% Duplication on New Code

See analysis details on SonarQube Cloud

norrisjeremy · 2026-04-01T12:57:50Z

ShangMi.md

+- **SM3**: GB/T 32905 — 256-bit hash function (structural similarity to SHA-256)
+- **SM4**: GB/T 32907 — 128-bit block cipher (successor of SMS4)
+
+Their use in SSH is defined by the Chinese standard **GM/T 0054-2018** ("SSH protocol based on


Is there a public link to the GM/T 0054-2018 standard?

norrisjeremy · 2026-04-01T13:02:23Z

ShangMi.md

+## Status in OpenSSH
+
+Mainline OpenSSH does **not** support SM algorithms. Support exists in a fork maintained by
+[openEuler](https://gitee.com/src-openeuler/openssh), a Linux distribution developed by Huawei.


Is this URL for openEuler still accurate?
I see a note in the Gitee project indicating it has moved to AtomGit?

mwiede and others added 5 commits March 31, 2026 12:32

config: switch ecdh-sm2p256v1 from ECDHSM2 to ECDHSM2BC

396b7de

Use BC's SM2KeyExchange (kLen=256 bits) as the production implementation. The manual ECDHSM2 remains in the codebase as reference. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

norrisjeremy reviewed Mar 31, 2026

View reviewed changes

mwiede and others added 8 commits April 1, 2026 11:55

Merge branch 'master' into feat/shangmi

99fd13b

fix sonar issues

2149bd9

remove sm algorithms from default config

9d93c39

setup docker build cache and cache jsch-openeuler-sshd-base image here

9736103

add sm2 back to PubkeyAcceptedAlgorithms

450b140

run formatter

e4fd01e

move test prep to the right place

ff36480

norrisjeremy reviewed Apr 1, 2026

View reviewed changes

move image preparation to build stage

b880746

norrisjeremy reviewed Apr 1, 2026

View reviewed changes

Conversation

mwiede commented Mar 31, 2026

Uh oh!

norrisjeremy Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mwiede commented Mar 31, 2026

Uh oh!

norrisjeremy commented Mar 31, 2026

Uh oh!

norrisjeremy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Apr 1, 2026

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

norrisjeremy Mar 31, 2026 •

edited

Loading

norrisjeremy Apr 1, 2026 •

edited

Loading

mwiede Apr 1, 2026 •

edited

Loading