Skip to content
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

feat(ext/crypto) - exportKey(pkcs8/spki/jwk) for ECDSA and ECDH #13104

Merged
merged 21 commits into from
Jan 19, 2022
Merged

feat(ext/crypto) - exportKey(pkcs8/spki/jwk) for ECDSA and ECDH #13104

merged 21 commits into from
Jan 19, 2022

Conversation

cryptographix
Copy link
Contributor

@cryptographix cryptographix commented Dec 16, 2021

Towards #11690 - exportKey implemented for ECDSA and ECDH, extracted from #13013 with modifications

pkcs8 : curve P-256 and P-384
spki : curves P-256 and P-384
jwk :

  • public key - curves P-256 and P-384
  • private key - curve P-256 and P-384

Shares ec_key.rs with #13154.

@cryptographix
Copy link
Contributor Author

cryptographix commented Dec 16, 2021

@lucacasonato - strange behaviour for WPT import_export ECDH SPKI P-256. The W3C spec says to export with OID 1.3.132.1.12 but the test appears to require the standard ecPublicKey OID.
BTW: Maybe need to patch importKey SPKI /PKCS8 to accept both OID, including algorithm as a param to import_key_ec() and using something like :

if alg != elliptic_curve::ALGORITHM_OID {
        let ok = match algorithm {
          ImportKeyOptions::Ecdh{ .. } => {
            // id-ecDh
            alg != ObjectIdentifier::new("1.3.132.1.12")
          },
          _ => false,
        };

        if !ok {
          return Err(data_error("unsupported algorithm"));
        }
      }

@panva
Copy link
Contributor

panva commented Dec 16, 2021

@seanwykes export ecPublicKey, see w3c/webcrypto#305. The spec editors are in the process of aligning the editor's draft of the specification with what is commonly actually implemented. None of the existing implementations use support importing or exporting id-ecDH.

@cryptographix
Copy link
Contributor Author

@seanwykes export ecPublicKey, see w3c/webcrypto#305. The spec editors are in the process of aligning the editor's draft of the specification with what is commonly actually implemented. None of the existing implementations use support importing or exporting id-ecDH.

@panva thanks .. backed out previous commit.

ext/crypto/export_key.rs Outdated Show resolved Hide resolved
ext/crypto/00_crypto.js Show resolved Hide resolved
ext/crypto/00_crypto.js Outdated Show resolved Hide resolved
ext/crypto/export_key.rs Outdated Show resolved Hide resolved
ext/crypto/export_key.rs Outdated Show resolved Hide resolved
@cryptographix
Copy link
Contributor Author

@littledivy and @lucacasonato - I'd like your feedback on the lack of support for PKCS#8 SecretKey in p384 crate that currently affects both importKey and exportKey.

I'm testing a strategy to solve this:

  1. ExportKey(JWK private) -> parse PKCS#8 ECPrivateKey using der::asn1 to extract priv/pub parts.
  2. ImportKey(PKCS#8) -> same code to extract, then use ring::pkcs8 to build the key, thus checking valid key before import.
  3. ImportKey(JWK private) -> use der::asn1 to build ECPrivateKey from priv/pub, wrap in PKCS#8 PrivateKeyInfo. Then validate using ring::pkcs8.

Any ideas on this?

@littledivy
Copy link
Member

parse PKCS#8 ECPrivateKey using der::asn1 to extract priv/pub parts.

Sounds good. We do similar stuff with RSA key parameters.

@cryptographix
Copy link
Contributor Author

cryptographix commented Dec 20, 2021

parse PKCS#8 ECPrivateKey using der::asn1 to extract priv/pub parts.

Sounds good. We do similar stuff with RSA key parameters.

@littledivy export implemented here and import in #13154. The ec_key.rs code is the same, with only an allow(dead_code) so that lint doesn't complain.

Copy link
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

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

LGTM, thank you! ...regarding the addition WPT changes in unwrap/wrapKey, looks like they were false positive earlier? I guess we'll get a better idea in #13154

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.

3 participants