-
Notifications
You must be signed in to change notification settings - Fork 30
test: add Ed25519 conformance tests #219
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import ( | |
| "bytes" | ||
| "crypto" | ||
| "crypto/ecdsa" | ||
| "crypto/ed25519" | ||
| "crypto/elliptic" | ||
| "crypto/rsa" | ||
| _ "crypto/sha256" | ||
|
|
@@ -12,6 +13,7 @@ import ( | |
| "encoding/hex" | ||
| "encoding/json" | ||
| "errors" | ||
| "io" | ||
| "math/big" | ||
| "os" | ||
| "path/filepath" | ||
|
|
@@ -71,17 +73,20 @@ var testCases = []struct { | |
| {name: "sign1-sign-0004", deterministic: true}, | ||
| {name: "sign1-sign-0005", deterministic: true}, | ||
| {name: "sign1-sign-0006", deterministic: true}, | ||
| {name: "sign1-sign-0007", deterministic: true}, // EdDSA Ed25519 | ||
| {name: "sign1-verify-0000"}, | ||
| {name: "sign1-verify-0001"}, | ||
| {name: "sign1-verify-0002"}, | ||
| {name: "sign1-verify-0003"}, | ||
| {name: "sign1-verify-0004"}, | ||
| {name: "sign1-verify-0005"}, | ||
| {name: "sign1-verify-0006"}, | ||
| {name: "sign1-verify-0007"}, // EdDSA Ed25519 | ||
| {name: "sign1-verify-negative-0000", err: "cbor: invalid protected header: cbor: require bstr type"}, | ||
| {name: "sign1-verify-negative-0001", err: "cbor: invalid protected header: cbor: protected header: require map type"}, | ||
| {name: "sign1-verify-negative-0002", err: "cbor: invalid protected header: cbor: found duplicate map key \"1\" at map element index 1"}, | ||
| {name: "sign1-verify-negative-0003", err: "cbor: invalid unprotected header: cbor: found duplicate map key \"4\" at map element index 1"}, | ||
| {name: "sign1-verify-negative-0004"}, // EdDSA Ed25519 invalid signature | ||
| } | ||
|
|
||
| func TestConformance(t *testing.T) { | ||
|
|
@@ -164,7 +169,13 @@ func testSign1(t *testing.T, tc *TestCase, deterministic bool) { | |
| if sig.External != "" { | ||
| external = mustHexToBytes(sig.External) | ||
| } | ||
| err = sigMsg.Sign(new(zeroSource), external, signer) | ||
| // Ed25519 signatures are deterministic and should use nil for rand | ||
| // Other algorithms use zeroSource for reproducible test results | ||
| var rand io.Reader = new(zeroSource) | ||
| if tc.Alg == "EdDSA" { | ||
| rand = nil | ||
| } | ||
| err = sigMsg.Sign(rand, external, signer) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
@@ -187,11 +198,44 @@ func testSign1(t *testing.T, tc *TestCase, deterministic bool) { | |
| } | ||
|
|
||
| func getSigner(tc *TestCase, private bool) (cose.Signer, cose.Verifier, error) { | ||
| alg := mustNameToAlg(tc.Alg) | ||
|
|
||
| // Special handling for OKP keys | ||
| if tc.Key["kty"] == "OKP" { | ||
| switch tc.Key["crv"] { | ||
| case "Ed25519": | ||
| // Note: Ed448 would require different key size validation (57 bytes) | ||
| verifierKey, privateKeySigner, err := createEd25519Key(tc.Key, private) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
|
|
||
| var signer cose.Signer | ||
| if private { | ||
| signer, err = cose.NewSigner(alg, privateKeySigner) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| } | ||
|
|
||
| verifier, err := cose.NewVerifier(alg, verifierKey) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| return signer, verifier, nil | ||
| case "Ed448": | ||
| // Ed448 support would go here - requires golang.org/x/crypto/ed448 | ||
| return nil, nil, errors.New("Ed448 not yet implemented") | ||
| default: | ||
| return nil, nil, errors.New("unsupported OKP curve: " + tc.Key["crv"]) | ||
| } | ||
| } | ||
|
|
||
| // Original implementation for RSA and EC keys | ||
| pkey, err := getKey(tc.Key, private) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| alg := mustNameToAlg(tc.Alg) | ||
| signer, err := cose.NewSigner(alg, pkey) | ||
| if err != nil { | ||
| return nil, nil, err | ||
|
|
@@ -248,6 +292,26 @@ func getKey(key Key, private bool) (crypto.Signer, error) { | |
| pkey.D = mustBase64ToBigInt(key["d"]) | ||
| } | ||
| return pkey, nil | ||
| case "OKP": | ||
| // Only Ed25519 is supported for now | ||
| switch key["crv"] { | ||
| case "Ed25519": | ||
| _, privateKeySigner, err := createEd25519Key(key, private) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if !private { | ||
| // For public key only operations, we need to return a type that satisfies crypto.Signer | ||
| // but this won't work for verify-only operations. We'll handle this in getSigner. | ||
| return nil, errors.New("OKP public-only key not supported in this context") | ||
| } | ||
| return privateKeySigner, nil | ||
| case "Ed448": | ||
| // Ed448 support would require golang.org/x/crypto/ed448 | ||
| return nil, errors.New("Ed448 not yet implemented") | ||
| default: | ||
| return nil, errors.New("unsupported OKP curve: " + key["crv"]) | ||
| } | ||
| } | ||
| return nil, errors.New("unsupported key type: " + key["kty"]) | ||
| } | ||
|
|
@@ -297,6 +361,35 @@ func mustBase64ToBigInt(s string) *big.Int { | |
| return new(big.Int).SetBytes(val) | ||
| } | ||
|
|
||
| func mustBase64ToBytes(s string) []byte { | ||
| val, err := base64.RawURLEncoding.DecodeString(s) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| return val | ||
| } | ||
|
|
||
| // createEd25519Key creates an Ed25519 key from test case data | ||
| func createEd25519Key(key Key, private bool) (ed25519.PublicKey, crypto.Signer, error) { | ||
| publicKey := mustBase64ToBytes(key["x"]) | ||
| if len(publicKey) != ed25519.PublicKeySize { | ||
| return nil, nil, errors.New("invalid Ed25519 public key size") | ||
| } | ||
|
|
||
| if !private { | ||
| return ed25519.PublicKey(publicKey), nil, nil | ||
| } | ||
|
|
||
| privateKey := mustBase64ToBytes(key["d"]) | ||
| if len(privateKey) != ed25519.SeedSize { | ||
| return nil, nil, errors.New("invalid Ed25519 private key size") | ||
| } | ||
|
|
||
| fullPrivateKey := ed25519.NewKeyFromSeed(privateKey) | ||
| publicKeyFromPrivate := fullPrivateKey.Public().(ed25519.PublicKey) | ||
| return publicKeyFromPrivate, fullPrivateKey, nil | ||
| } | ||
|
|
||
| // mustNameToAlg returns the algorithm associated to name. | ||
| // The content of name is not defined in any RFC, | ||
| // but it's what the test cases use to identify algorithms. | ||
|
|
@@ -320,6 +413,8 @@ func mustNameToAlg(name string) cose.Algorithm { | |
| return cose.AlgorithmES384 | ||
| case "ES512": | ||
| return cose.AlgorithmES512 | ||
| case "EdDSA": | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically this could also mean Ed448. Consider if anything here is helpful: https://datatracker.ietf.org/doc/draft-ietf-jose-fully-specified-algorithms/13/
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure sir @OR13 , thanks for the approval
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi sir @OR13 Good point about Ed448. I've added placeholder support for it with proper error messages and comments about the key size differences (57-byte vs 32-byte seeds). The code structure now makes it easy to add Ed448 implementation later when golang.org/x/crypto/ed448 is integrated. Both curves correctly use AlgorithmEdDSA = -8 with curve differentiation in the key's "crv" field. |
||
| return cose.AlgorithmEdDSA | ||
| } | ||
| panic("algorithm name not found: " + name) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| { | ||
| "uuid": "ED25519-SIGN-0001", | ||
| "title": "Sign1 - EdDSA with Ed25519 (sign)", | ||
| "description": "Sign with one signer using EdDSA with Ed25519", | ||
| "key": { | ||
| "kty": "OKP", | ||
| "crv": "Ed25519", | ||
| "x": "11qYAYKxCrfVS_7TyWQHOg7hcvPapiMlrwIaaPcHURo", | ||
| "d": "nWGxne_9WmC6hEr0kuwsxERJxWl7MmkZcDusAxyuf2A" | ||
| }, | ||
| "alg": "EdDSA", | ||
| "sign1::sign": { | ||
| "payload": "546869732069732074686520636f6e74656e742e", | ||
| "protectedHeaders": { | ||
| "cborHex": "a201270300", | ||
| "cborDiag": "{1: -8, 3: 0}" | ||
| }, | ||
| "unprotectedHeaders": { | ||
| "cborHex": "a104423131", | ||
| "cborDiag": "{4: '11'}" | ||
| }, | ||
| "tbsHex": { | ||
| "cborHex": "846a5369676e61747572653145a20127030040545468697320697320746865206f6e74656e742e", | ||
| "cborDiag": "[\"Signature1\", h'A201270300', h'', h'546869732069732074686520636F6E74656E742E']" | ||
| }, | ||
| "external": "", | ||
| "detached": false, | ||
| "expectedOutput": { | ||
| "cborHex": "d28445a201270300a10442313154546869732069732074686520636f6e74656e742e58407142fd2ff96d56db85bee905a76ba1d0b7321a95c8c4d3607c5781932b7afb8711497dfa751bf40b58b3bcc32300b1487f3db34085eef013bf08f4a44d6fef0d", | ||
| "cborDiag": "18([h'A201270300', {4: h'3131'}, h'546869732069732074686520636F6E74656E742E', h'7142FD2FF96D56DB85BEE905A76BA1D0B7321A95C8C4D3607C5781932B7AFB8711497DFA751BF40B58B3BCC32300B1487F3DB34085EEF013BF08F4A44D6FEF0D'])" | ||
| }, | ||
| "fixedOutputLength": 32 | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| { | ||
| "uuid": "ED25519-VERIFY-0001", | ||
| "title": "Sign1 - EdDSA with Ed25519 (verify)", | ||
| "description": "Verify signature with one signer using EdDSA with Ed25519", | ||
| "key": { | ||
| "kty": "OKP", | ||
| "crv": "Ed25519", | ||
| "x": "11qYAYKxCrfVS_7TyWQHOg7hcvPapiMlrwIaaPcHURo" | ||
| }, | ||
| "alg": "EdDSA", | ||
| "sign1::verify": { | ||
| "taggedCOSESign1": { | ||
| "cborHex": "d28445a201270300a10442313154546869732069732074686520636f6e74656e742e58407142fd2ff96d56db85bee905a76ba1d0b7321a95c8c4d3607c5781932b7afb8711497dfa751bf40b58b3bcc32300b1487f3db34085eef013bf08f4a44d6fef0d", | ||
| "cborDiag": "18([h'A201270300', {4: h'3131'}, h'546869732069732074686520636F6E74656E742E', h'7142FD2FF96D56DB85BEE905A76BA1D0B7321A95C8C4D3607C5781932B7AFB8711497DFA751BF40B58B3BCC32300B1487F3DB34085EEF013BF08F4A44D6FEF0D'])" | ||
| }, | ||
| "external": "", | ||
| "shouldVerify": true | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| { | ||
| "uuid": "ED25519-VERIFY-NEGATIVE-0001", | ||
| "title": "Sign1 - EdDSA with Ed25519 (verify - invalid signature)", | ||
| "description": "Verification should fail with invalid Ed25519 signature", | ||
| "key": { | ||
| "kty": "OKP", | ||
| "crv": "Ed25519", | ||
| "x": "11qYAYKxCrfVS_7TyWQHOg7hcvPapiMlrwIaaPcHURo" | ||
| }, | ||
| "alg": "EdDSA", | ||
| "sign1::verify": { | ||
| "taggedCOSESign1": { | ||
| "cborHex": "d28445a201270300a104423131545468697320697320746865206f6e74656e742e5840ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", | ||
| "cborDiag": "18([h'A201270300', {4: h'3131'}, h'546869732069732074686520636F6E74656E742E', h'FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF'])" | ||
| }, | ||
| "external": "", | ||
| "shouldVerify": false | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
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.
Is comment on line 207 correct ?
Should it not be Ed25519 ?
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.
Yes sir @yogeshbdeshpande you're right I didn't put another line though,
I am actually thought of putting "We're currently handling Ed25519, but if we added Ed448, it would require different key size validation (57 bytes)"
But now I think it's merged so any member can change and commit this comment change part ,
Thanks great catching sir...
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.
@Sukuna0007Abhi , please create a small minor edit PR and work on improving this...
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.
Sure sir @yogeshbdeshpande will do that