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

Improper encoding of JWK for RSA keys (The JWK "n" member contained a leading zero.) #140

Closed
wellcaffeinated opened this issue Apr 1, 2022 · 6 comments · Fixed by #141
Closed
Labels
bug Something isn't working

Comments

@wellcaffeinated
Copy link

Affected version picky 6.3.0

Problem: When attempting to use the JWK that picky created in chrome, i get this error:

DOMException: The JWK "n" member contained a leading zero.

This is related to the following:

For what it's worth, i was doing this:

let pk = picky::key::PublicKey::from_rsa_der(&der)?;
let mut jwk = picky::jose::jwk::Jwk::from_public_key(&pk)?
jwk.alg = Some(picky::jose::jwk::Jwa::Sig(picky::jose::jws::JwsAlg::RS256));

And it gave me the output:

{
  "alg": "RS256",
  "e": "AQAB",
  "kty": "RSA",
  "n": "ANA4_xFppBBf56Vcbt2CRQTe35qaGzFvo30mIpuwJjAP9H1lhoGwWPAISL9f0unC4zPIZWsOf8xSXGSgjLYJkguPUph17qznKZZ1hH2Wbti-2ES-1M_MJS4x6oC2v-9UmE6xV1mVI6mepBwqrKMY2Fe2HZWS1zk7FKnWmPz_dIbzZyP8ctrbHFyZqgJCJjSLHwO6Lzh0K4cGdMn6WGnzgj6rYj65qDTSvTXoToSxHDUeOYoVn0ERqzm-DinJlXGYyE-Ucqkx7u7nqWSQysE1G8zlUVQ5ilv38FfHf2Gi7rI7k3PxBNWNme95UADzlYQGUtW-YpTkR9Mfsd4-wliA5Yc"
}

Note the leading zero: ANA4_xFpp...

@CBenoit CBenoit added the bug Something isn't working label Apr 4, 2022
@CBenoit
Copy link
Member

CBenoit commented Apr 4, 2022

Thank you for reporting this!

CBenoit added a commit that referenced this issue Apr 7, 2022
JWK encoding of a value is the unsigned big-endian representation as an octet sequence.
The octet sequence MUST utilize the minimum number of octets needed to represent the value.
That is: **no leading zero** must be present.

See issue #140:
#140
CBenoit added a commit that referenced this issue Apr 7, 2022
JWK encoding of a value is the unsigned big-endian representation as an octet sequence.
The octet sequence MUST utilize the minimum number of octets needed to represent the value.
That is: **no leading zero** must be present.

See issue #140:
#140
@CBenoit
Copy link
Member

CBenoit commented Apr 7, 2022

@wellcaffeinated could you confirm the issue is fixed on your side as well when using the patch from #141?
In your Cargo.toml:

picky = { git = "https://github.com/Devolutions/picky-rs", rev = "5f1862f7a8f3f5f21e28a801377b3e926e133f97", … }

Thank you!

@wellcaffeinated
Copy link
Author

Hmm.... I'm having some trouble with conflicting versions now

error: failed to select a version for `zeroize`.
    ... required by package `num-bigint-dig v0.8.1`
    ... which satisfies dependency `num-bigint-dig = "^0.8.1"` of package `picky v7.0.0-rc.2 (https://github.com/Devolutions/picky-rs?rev=5f1862f7a8f3f5f21e28a801377b3e926e133f97#5f1862f7)`
    ... which satisfies git dependency `picky` of package `hsm-bridge v0.1.0 (/Users/wellcaffeinated/Documents/github/buffbeacon/twine-ipfs/beacon/hsm-bridge)`
versions that meet the requirements `^1.5` are: 1.5.4, 1.5.3

all possible versions conflict with previously selected packages.

  previously selected package `zeroize v1.3.0`
    ... which satisfies dependency `zeroize = "^1"` (locked to 1.3.0) of package `crypto-bigint v0.3.2`
    ... which satisfies dependency `crypto-bigint = "^0.3"` (locked to 0.3.2) of package `der v0.5.1`
    ... which satisfies dependency `der = "^0.5"` (locked to 0.5.1) of package `ecdsa v0.13.4`
    ... which satisfies dependency `ecdsa-core = "^0.13"` (locked to 0.13.4) of package `p256 v0.10.1`
    ... which satisfies dependency `p256 = "^0.10"` (locked to 0.10.1) of package `yubihsm v0.40.0`
    ... which satisfies dependency `yubihsm = "^0.40"` (locked to 0.40.0) of package `hsm-bridge v0.1.0 (/Users/wellcaffeinated/Documents/github/buffbeacon/twine-ipfs/beacon/hsm-bridge)`

failed to select a version for `zeroize` which could resolve this conflict

@CBenoit
Copy link
Member

CBenoit commented Apr 8, 2022

Maybe you could try update your Cargo.lock with

$ cargo update -p zeroize

@wellcaffeinated
Copy link
Author

no dice :(

error: failed to select a version for `zeroize`.
    ... required by package `curve25519-dalek v3.2.1`
    ... which satisfies dependency `curve25519-dalek = "^3"` (locked to 3.2.1) of package `ed25519-dalek v1.0.1`
    ... which satisfies dependency `ed25519-dalek = "^1"` (locked to 1.0.1) of package `yubihsm v0.40.0`
    ... which satisfies dependency `yubihsm = "^0.40"` (locked to 0.40.0) of package `hsm-bridge v0.1.0 (/Users/wellcaffeinated/Documents/github/buffbeacon/twine-ipfs/beacon/hsm-bridge)`
versions that meet the requirements `>=1, <1.4` are: 1.3.0, 1.2.0, 1.1.1, 1.1.0, 1.0.0

all possible versions conflict with previously selected packages.

  previously selected package `zeroize v1.5.3`
    ... which satisfies dependency `zeroize = "^1.5"` of package `num-bigint-dig v0.8.1`
    ... which satisfies dependency `num-bigint-dig = "^0.8.1"` of package `picky v7.0.0-rc.2 (https://github.com/Devolutions/picky-rs?rev=5f1862f7a8f3f5f21e28a801377b3e926e133f97#5f1862f7)`
    ... which satisfies git dependency `picky` of package `hsm-bridge v0.1.0 (/Users/wellcaffeinated/Documents/github/buffbeacon/twine-ipfs/beacon/hsm-bridge)`

failed to select a version for `zeroize` which could resolve this conflict

@CBenoit
Copy link
Member

CBenoit commented Apr 9, 2022

Oh, the error changed and is actually showing something useful!
Looks like curve25519-dalek is depending on zeroize >= 1, <1.4, but since num-bigint-dig is depending on zeroize ^1.5, there is no version that can be used by both :/

Here are some relevant links:

I understand curve25519-dalek did that because they have a different policy about MSRV.

Now, there is no good solution in such case, ideally curve25519-dalek could bump their MSRV. There is a patch for just that you could use: dalek-cryptography/curve25519-dalek#386 (make sure to pin a specific commit using rev so you don’t pull arbitrary new commits implicitly)

CBenoit added a commit that referenced this issue Apr 20, 2022
JWK encoding of a value is the unsigned big-endian representation as an octet sequence.
The octet sequence MUST utilize the minimum number of octets needed to represent the value.
That is: **no leading zero** must be present.

See issue #140:
#140
CBenoit added a commit that referenced this issue Apr 20, 2022
JWK encoding of a value is the unsigned big-endian representation as an octet sequence.
The octet sequence MUST utilize the minimum number of octets needed to represent the value.
That is: **no leading zero** must be present.

See issue #140:
#140
CBenoit added a commit that referenced this issue Apr 20, 2022
JWK encoding of a value is the unsigned big-endian representation as an octet sequence.
The octet sequence MUST utilize the minimum number of octets needed to represent the value.
That is: **no leading zero** must be present.

See issue #140:
#140
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

2 participants