-
-
Notifications
You must be signed in to change notification settings - Fork 83
Add ShangMi (SM2/SM3/SM4/SM9) algorithm families #812
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add ShangMi (SM2/SM3/SM4/SM9) algorithm families #812
Conversation
Signed-off-by: Mehrn0ush <[email protected]>
|
Hi @bhess — if you have time, would you mind taking a look at this one as well? This adds ShangMi (SM2/SM3/SM4/SM9) families and updates the schema enum. Thanks a lot — always appreciate your feedback. |
|
cc: @bhess |
bhess
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Mehrn0ush for the additions! See the comments inline.
| "variant": [ | ||
| { | ||
| "pattern": "SM4[-(ECB|CBC|CFB|OFB|CTR|XTS)][-{padding}][-{ivlen}]", | ||
| "primitive": "block-cipher" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the "ae" modes seem to be missing (e.g., CCM, GCM..)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the note — you’re absolutely right. I’ve added an explicit ae variant for SM4 covering GCM and CCM (pattern: SM4-(GCM|CCM)[-{tagLength}][-{ivLength}]), and kept the existing block-mode variant (ECB/CBC/CFB/OFB/CTR/XTS) under block-cipher. This mirrors the registry’s existing separation of block modes vs authenticated-encryption modes.
schema/cryptography-defs.json
Outdated
| "family": "SM9", | ||
| "standard": [ | ||
| { | ||
| "name": "RFC8998", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SM9 seems to be not specified in RFC8998. See https://en.wikipedia.org/wiki/SM9_(cryptography_standard) for the references.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing that out. You’re correct — SM9 is not specified in RFC8998. I updated the SM9 standard reference accordingly to GM/T0044.1-2016 and switched the URL to the corresponding GM/T document.
schema/cryptography-defs.json
Outdated
| ], | ||
| "variant": [ | ||
| { | ||
| "pattern": "SM9", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rather than a single "other" primitive, it might be worth splitting this up to different variants.
I didn't look at the standard in the detail, but see https://en.wikipedia.org/wiki/SM9_(cryptography_standard), there are different primitives:
- signature
- keyagree/wrap
- kem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appreciate the suggestion — I replaced that entry using primitive other with multiple variants split by primitive to reflect the distinct SM9 functionalities:
signature via SM9-(SIG|SIGNATURE)
key-agree via SM9-(KEX|KEYEXCHANGE|KEY-EXCHANGE|KEYAGREE|KEYAGREEMENT|KEY-AGREE|KEY-AGREEMENT)
kem via SM9-(KEM|KEYENCAPSULATION|KEY-ENCAPSULATION)
pke via SM9-(ENC|ENCRYPTION|PKE|PUBLICKEY-ENCRYPTION|PUBLIC-KEY-ENCRYPTION)
If you’d prefer limiting SM9 strictly to signature / key agreement / KEM (as commonly summarized), I’m happy to drop the pke variant — I included it for completeness in case implementations label SM9 encryption explicitly.
Signed-off-by: Mehrn0ush <[email protected]>
|
One question on SM2 naming conventions for the registry: I currently model SM2 as SM2-256 (and ...-256 for ENC/KEX) to be explicit about the common curve size. Would you prefer making the suffix optional (SM2[-256]) to align with the registry’s broader matching style, or keeping it strict as SM2-256? I can adjust either way — just want to follow the convention you think is best for downstream matching. Thanks again @bhess |
Signed-off-by: Mehrn0ush <[email protected]>
|
Made SM2’s -256 suffix optional to accept both SM2 and SM2-256, following the registry’s [] convention. Updated patterns accordingly. |
Fixes #811
Adds SM2/SM3/SM4/SM9 algorithm families to the CycloneDX cryptography registry and updates algorithmFamiliesEnum accordingly.
Note: some official ShangMi specification mirrors appear region-blocked or unreliable from multiple locations.
To avoid fragile links in the registry, this PR relies on globally accessible references (RFC Editor + ISO)