Add DER & PEM support for SigningKeySeed and VerificationKeyBytes (RFC 8410) #46

droark · 2021-03-31T23:20:31Z

Add encoding to and decoding from DER bytes and PEM strings for SigningKeySeed and VerificationKeyBytes.
Add some functions so that the Java code mirrors, to a certain degree, the JDK 15 interface for Ed25519 keys and signatures.
Add encoding and decoding for signatures (technically identity functions).
Miscellaneous cleanup.

droark · 2021-03-31T23:26:27Z

FYI, the code should work as is. It passes all tests locally. I'm leaving it in draft form for a little while as I clean it up and make sure it's ready for review. That said, feel free to play with the code and make suggestions.

Thanks.

src/signing_key.rs

tarcieri · 2021-05-21T20:14:59Z

I just discovered something interesting about PKCS#8-serialized Ed25519 private keys. Per this CURDLE draft:

https://datatracker.ietf.org/doc/html/draft-ietf-curdle-pkix-07#section-10.3

The private_key field actually contains an encapsulated 34-byte ASN.1 document containing an OCTET STRING with tag/length header:

   12 04   34:   OCTET STRING
             :     04 20 D4 EE 72 DB F9 13 58 4A D5 B6 D8 F1 F7 69
             :     F8 AD 3A FE 7C 28 CB F1 D4 FB E0 97 A8 8F 44 75
             :     58 42
             :   }

   Note that the value of the private key is:

   D4 EE 72 DB F9 13 58 4A D5 B6 D8 F1 F7 69 F8 AD
   3A FE 7C 28 CB F1 D4 FB E0 97 A8 8F 44 75 58 42

See also the second example which uses PKCS#8v2 and includes a public key:

    12  34:   OCTET STRING, encapsulates {
    14  32:     OCTET STRING D4 EE 72 DB F9 13 58 4A D5 B6 D8 F1 F7
                    69 F8 AD 3A FE 7C 28 CB F1 D4 FB E0 97 A8 8F 44
                    75 58 42
          :     }

(that's right, it's an OCTET STRING inside of another OCTET STRING)

This makes it a bit trickier, as decoding you'll need to check the private key begins with the bytes 04 20 and strip them, and then encoding you'll need to add them, creating a 34-byte ASN.1 DER document with those leading bytes to encapsulate into the private_key field.

droark · 2021-05-22T16:59:41Z

@tarcieri - Thanks for the catch! I see that this is indeed a part of the final RFC 8410 spec. I'll add a fix for that.

droark · 2021-05-24T03:18:46Z

@tarcieri - Pushed updated code.

droark · 2021-06-02T23:10:42Z

@dconnolly - Anything I can do to get some 👀 on this one? Thanks!

tarcieri · 2021-06-02T23:21:57Z

@droark FYI, I plan on cutting another release of the pkcs8 crate (prospectively v0.7.0) in the next few days.

I'm also happy to do another pass on reviewing this soon (we'd actually very much like to use it)

tarcieri · 2021-06-08T03:43:13Z

pkcs8 v0.7.0 is out: https://docs.rs/pkcs8/0.7.0/

FYI: Minimum Supported Rust Version is 1.51

src/signing_key.rs

droark · 2021-06-09T01:35:21Z

pkcs8 v0.7.0 is out: https://docs.rs/pkcs8/0.7.0/

FYI: Minimum Supported Rust Version is 1.51

Thanks. Pushed a new commit that uses it.

src/signing_key.rs

src/verification_key.rs

src/signing_key.rs

…C 8410) - Add encoding to and decoding from DER bytes and PEM strings for SigningKeySeed and VerificationKeyBytes. - Add some functions so that the Java code mirrors, to a certain degree, the JDK 15 interface for Ed25519 keys and signatures. - Add encoding and decoding for signatures (technically identity functions). - Miscellaneous cleanup.

- In RFC 8410, DER-encoded private keys are in an octet string that's encapsulated by another octet string. Add the extra octet string, and adjust tests as necessary. - In the tests, use the private key from RFC 8410, Sect. 10.3.

- Enhance PEM capabilities for SigningKey and VerificationKeyBytes. This also allowed for some tests to be simplified. - From -> TryFrom for some VerificationKeyBytes impls.

- Make necessary changes to support the newer crate. - Fix an unrelated compiler warning.

- Get code to compile after updating to the latest Rust. - Fix a couple of failing tests (add LF to expected encoding output).

- Update pkcs8 crate to 0.10.0, and update code as required to support the crate. This includes supporting the Decode(Public/Private)Key and Encode(Public/Private)Key traits so as to take advantage of Ed25519 DER and PEM code in the crate. - Add the latest ed25519 crate (2.2.0) to support KeypairBytes and other features. - Remove the signature code and implement Signature (Signer and Verifier traits) from the "signatures" crate included with the pkcs8 crate. - Update the JNI code. This includes mandating Scala 3 usage. - Minor cleanup (including warning fixes) and changes to make the code a bit clearer. A follow-up commit will clean up the tests and probably add support for v2 private DER keys.

- Update pkcs8 crate to 0.10.1. - Fix PEM feature code. - Update Ed25519 JNI code as needed. - Remove dead code. - Re-enable a couple of unit tests. Note that a couple of Ed25519 JNI unit tests are still failing. A follow-up PR will have the fix.

When executing `sbt publishLocal` and generating a JAR file, there are warnings regarding some functions not having public comments. Add public comments as needed.

- Finish adding PEM/PKCS8 tags and cfg items as needed to separate the features from default compilation. - Revert some minor name changes. - Make the JNI README more precise with regards to requirements. - Add ARM64 macOS support to JNI. Untested but it should work, and it doesn't break Intel Macs. - Miscellaneous cleanup, including fixing cargo and sbt warnings.

droark · 2023-03-18T00:53:21Z

@conradoplg - I think I caught everything you requested. All the PEM/PKCS8 stuff is optional and properly firewalled (AFAIK), Signature is left alone, and all tests pass, with and without the features. I'm thinking it might not hurt to add JNI testing to CI. We can cross that bridge another time. As is, the only quirk I don't like is that sbt test will fail by default when testing JNI. This is discussed in the README. I don't think there's a good way to work around this, so JNI users will just have to RTFM when compiling.

The 0.21.X crates feature a major refactor that breaks the code. Don't upgrade to them until some issues are resolved. (See jni-rs/jni-rs#432 for more info.)

- A path forward to upgrading to 0.21.X was suggested by the jni-rs library developer (jni-rs/jni-rs#439 (comment)). Upgrade the code, improving the safety of the JNI code. - Cargo.toml fixups.

conradoplg

This looks good, just needs addressing some small comments.

Can you also please cargo fmt the code and fix the 3 cargo clippy warnings?

Cargo.toml

ed25519jni/rust/Cargo.toml

Also do minor JNI README cleanup.

droark · 2023-04-18T02:58:10Z

@conradoplg - Pushed the changes. Everything should be ready now.

conradoplg

Sorry, one last detail I missed 😬

src/lib.rs

tests/unit_tests.rs

tests/util/mod.rs

droark · 2023-04-21T04:40:55Z

@conradoplg - Fixed.

conradoplg

Thank you so much for the contribution and with the patience 😅

droark force-pushed the pkcs8_support branch from 442f076 to f45b768 Compare March 31, 2021 23:31

tarcieri reviewed Apr 1, 2021

View reviewed changes

src/signing_key.rs Outdated Show resolved Hide resolved

tarcieri reviewed Apr 1, 2021

View reviewed changes

src/signing_key.rs Outdated Show resolved Hide resolved

droark force-pushed the pkcs8_support branch from f45b768 to 602e757 Compare April 3, 2021 00:46

tarcieri reviewed Apr 3, 2021

View reviewed changes

src/signing_key.rs Outdated Show resolved Hide resolved

tarcieri reviewed Apr 3, 2021

View reviewed changes

src/signing_key.rs Outdated Show resolved Hide resolved

tarcieri reviewed Apr 3, 2021

View reviewed changes

src/signing_key.rs Outdated Show resolved Hide resolved

tarcieri reviewed Apr 3, 2021

View reviewed changes

src/signing_key.rs Outdated Show resolved Hide resolved

droark force-pushed the pkcs8_support branch from 602e757 to 5e41b11 Compare April 3, 2021 06:26

tarcieri reviewed Apr 4, 2021

View reviewed changes

src/signing_key.rs Outdated Show resolved Hide resolved

droark force-pushed the pkcs8_support branch 2 times, most recently from e862319 to 257c206 Compare April 5, 2021 02:55

droark marked this pull request as ready for review April 21, 2021 21:10

tarcieri mentioned this pull request May 10, 2021

Support deserialising and serialising to/from PKCS8 keypair bytes dalek-cryptography/ed25519-dalek#169

Closed

droark force-pushed the pkcs8_support branch from f229b4a to 2e961fe Compare May 24, 2021 03:39

tarcieri reviewed Jun 8, 2021

View reviewed changes

src/signing_key.rs Outdated Show resolved Hide resolved

droark commented Jun 9, 2021

View reviewed changes

src/signing_key.rs Outdated Show resolved Hide resolved

tarcieri reviewed Jun 9, 2021

View reviewed changes

src/signing_key.rs Outdated Show resolved Hide resolved

droark force-pushed the pkcs8_support branch from b950450 to 02abef5 Compare June 9, 2021 17:04

tarcieri reviewed Jun 9, 2021

View reviewed changes

src/verification_key.rs Outdated Show resolved Hide resolved

tarcieri reviewed Jun 9, 2021

View reviewed changes

src/verification_key.rs Outdated Show resolved Hide resolved

tarcieri reviewed Jun 9, 2021

View reviewed changes

src/signing_key.rs Outdated Show resolved Hide resolved

Douglas Roark and others added 12 commits March 16, 2023 20:58

Accommodate extra octet string in private key DER bytes

54e7cda

- In RFC 8410, DER-encoded private keys are in an octet string that's encapsulated by another octet string. Add the extra octet string, and adjust tests as necessary. - In the tests, use the private key from RFC 8410, Sect. 10.3.

Update pkcs8 to 0.7.0

388c817

Cleanup

822d721

- Enhance PEM capabilities for SigningKey and VerificationKeyBytes. This also allowed for some tests to be simplified. - From -> TryFrom for some VerificationKeyBytes impls.

Upgrade JNI Rust bindings to PKCS8 0.7.5

34d78ae

- Make necessary changes to support the newer crate. - Fix an unrelated compiler warning.

More fixups

8baef99

- Get code to compile after updating to the latest Rust. - Fix a couple of failing tests (add LF to expected encoding output).

Further code cleanup

918dcbf

- Update pkcs8 crate to 0.10.1. - Fix PEM feature code. - Update Ed25519 JNI code as needed. - Remove dead code. - Re-enable a couple of unit tests. Note that a couple of Ed25519 JNI unit tests are still failing. A follow-up PR will have the fix.

Add missing DER/PEM files for unit tests

0b3880a

Add JNI comments to resolve publisher warnings

b4d8d97

When executing `sbt publishLocal` and generating a JAR file, there are warnings regarding some functions not having public comments. Add public comments as needed.

JNI README update

93163d7

Comment touchup

e0e6ca5

droark force-pushed the pkcs8_support branch from 56e8378 to e0e6ca5 Compare March 17, 2023 02:20

droark force-pushed the pkcs8_support branch from f7cfc2b to 70bc7d8 Compare March 18, 2023 00:47

droark requested a review from conradoplg March 18, 2023 00:49

droark added 2 commits March 28, 2023 19:58

Upgrade jni crate to 0.20.0

1a2738e

The 0.21.X crates feature a major refactor that breaks the code. Don't upgrade to them until some issues are resolved. (See jni-rs/jni-rs#432 for more info.)

Upgrade jni crate to 0.21.1

98d3a6d

- A path forward to upgrading to 0.21.X was suggested by the jni-rs library developer (jni-rs/jni-rs#439 (comment)). Upgrade the code, improving the safety of the JNI code. - Cargo.toml fixups.

conradoplg requested changes Apr 14, 2023

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

Cargo.toml Outdated Show resolved Hide resolved

ed25519jni/rust/Cargo.toml Show resolved Hide resolved

cargo clippy / cargo fmt cleanup

f8d5299

Also do minor JNI README cleanup.

conradoplg requested changes Apr 19, 2023

View reviewed changes

src/lib.rs Show resolved Hide resolved

tests/unit_tests.rs Outdated Show resolved Hide resolved

tests/util/mod.rs Outdated Show resolved Hide resolved

Use an export to clean up some tests a bit

3882282

conradoplg approved these changes Apr 21, 2023

View reviewed changes

conradoplg merged commit 346f4cd into ZcashFoundation:main Apr 21, 2023

This was referenced Jun 7, 2023

Implement the signature traits. #10

Closed

Add AsRef<[u8]> for Signature #9

Closed

droark deleted the pkcs8_support branch July 18, 2023 13:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add DER & PEM support for SigningKeySeed and VerificationKeyBytes (RFC 8410) #46

Add DER & PEM support for SigningKeySeed and VerificationKeyBytes (RFC 8410) #46

droark commented Mar 31, 2021

droark commented Mar 31, 2021

tarcieri commented May 21, 2021 •

edited

Loading

droark commented May 22, 2021

droark commented May 24, 2021

droark commented Jun 2, 2021 •

edited

Loading

tarcieri commented Jun 2, 2021 •

edited

Loading

tarcieri commented Jun 8, 2021 •

edited

Loading

droark commented Jun 9, 2021

droark commented Mar 18, 2023

conradoplg left a comment

droark commented Apr 18, 2023

conradoplg left a comment

droark commented Apr 21, 2023

conradoplg left a comment

Add DER & PEM support for SigningKeySeed and VerificationKeyBytes (RFC 8410) #46

Add DER & PEM support for SigningKeySeed and VerificationKeyBytes (RFC 8410) #46

Conversation

droark commented Mar 31, 2021

droark commented Mar 31, 2021

tarcieri commented May 21, 2021 • edited Loading

droark commented May 22, 2021

droark commented May 24, 2021

droark commented Jun 2, 2021 • edited Loading

tarcieri commented Jun 2, 2021 • edited Loading

tarcieri commented Jun 8, 2021 • edited Loading

droark commented Jun 9, 2021

droark commented Mar 18, 2023

conradoplg left a comment

Choose a reason for hiding this comment

droark commented Apr 18, 2023

conradoplg left a comment

Choose a reason for hiding this comment

droark commented Apr 21, 2023

conradoplg left a comment

Choose a reason for hiding this comment

tarcieri commented May 21, 2021 •

edited

Loading

droark commented Jun 2, 2021 •

edited

Loading

tarcieri commented Jun 2, 2021 •

edited

Loading

tarcieri commented Jun 8, 2021 •

edited

Loading