Skip to content

Introduce sm-cipher workflow tests to wolfssl#9193

Closed
gojimmypi wants to merge 1 commit intowolfSSL:masterfrom
gojimmypi:pr-sm-cipher-test
Closed

Introduce sm-cipher workflow tests to wolfssl#9193
gojimmypi wants to merge 1 commit intowolfSSL:masterfrom
gojimmypi:pr-sm-cipher-test

Conversation

@gojimmypi
Copy link
Copy Markdown
Contributor

@gojimmypi gojimmypi commented Sep 12, 2025

Description

This PR introduces SM ShangMi cipher tests in GitHub workflow for the wolfssl repo.

The SM TLS 1.3 is currently not working properly; Thus this PR can be merged as-is to show that, or wait until #9175 is merged. I could also temporarily disable the TLS 1.3 tests in this PR.

See sample action history: https://github.com/gojimmypi/wolfssl/actions/workflows/sm-cipher.yml

(failing as expected on current pr-sm-cipher-test but working on espressif-latest-debug with the needed #9175 patch)

See also:

I also have a WIP wolfsm companion SM Cipher Test (2 of 2) test. See sample for that. Update: merged in wolfsm #28

SM tests are currently failing there in the wolfsm repo as I do not manually apply the needed patch from #9175 . (merged)

Update: Example GitHub Actions Output:

Fixes zd# n/a

Testing

Tested on my branches:

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@JacobBarthelmeh
Copy link
Copy Markdown
Contributor

@gojimmypi does this need rebased after the SM fix was merged? I manually clicked to re-run the SM GitHub action test and it failed at the same location.

@gojimmypi
Copy link
Copy Markdown
Contributor Author

Hi @JacobBarthelmeh - yes, the new sm-cipher.yml workflow needs #9175 in order to run successfully.

Now that it is merged, I've refreshed from upstream and confirmed the new test runs successfully in this PR.

@gojimmypi
Copy link
Copy Markdown
Contributor Author

Jenkins retest this please.

For org.jenkinsci.plugins.workflow.support.steps.AgentOfflineException: Unable to create live FilePath for wolf-linux-cloud-node-[n]; wolf-linux-cloud-node-[n] was marked offline: Connection was broken

@gojimmypi gojimmypi marked this pull request as draft September 22, 2025 23:21
@gojimmypi
Copy link
Copy Markdown
Contributor Author

The SM Cipher test over in wolfsm repo should be nearly identical to this one.

After code review there, I've updated the sm-cipher.yml in this PR with those changes.

@gojimmypi
Copy link
Copy Markdown
Contributor Author

Jenkins retest this please

For StreamCorruptedException: invalid stream header: 636F7272
at java.base/java.io.ObjectInputStream.readStreamHeader(ObjectInputStream.java:958)

@gojimmypi gojimmypi marked this pull request as ready for review September 23, 2025 02:09
@@ -0,0 +1,194 @@
name: SM Cipher Test (1 of 2)
#
# Test fetches wolfssl-examples/Arduino and uses local, latest github master branch wolfssl
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fix comment... Arduino is not part of this test.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@@ -0,0 +1,194 @@
name: SM Cipher Test (1 of 2)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we need a sm-cipher test in wolfSSL and also in wolfSM? Why duplicated? I recommend only having the one in wolfsm and adding rules to run automatically on schedule. https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#schedule

@wolfSSL-Bot
Copy link
Copy Markdown

Can one of the admins verify this patch?

@JacobBarthelmeh
Copy link
Copy Markdown
Contributor

Closed since tests are added here #9795

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.

5 participants