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

Handle extra padding for RSA "n" JWK parameter #23

Closed
MicahParks opened this issue Mar 11, 2024 · 4 comments
Closed

Handle extra padding for RSA "n" JWK parameter #23

MicahParks opened this issue Mar 11, 2024 · 4 comments
Labels
invalid This doesn't seem right

Comments

@MicahParks
Copy link
Owner

MicahParks commented Mar 11, 2024

RFC 7518 Section 6.3.1.1 notes that:

Note that implementers have found that some cryptographic libraries prefix an extra zero-valued octet to the modulus representations they return, for instance, returning 257 octets for a 2048-bit key, rather than 256. Implementations using such libraries will need to take care to omit the extra octet from the base64url-encoded representation.

This note is not implemented in this project. Only the first half of the section defining it as a Base64urlUInt-encoded was regarded, but this will be remedied.

This affects Firebase JWK Sets. Please see the user report here:
#20 (comment)

@MicahParks MicahParks added the bug Something isn't working label Mar 11, 2024
@MicahParks MicahParks removed the bug Something isn't working label Mar 11, 2024
@MicahParks
Copy link
Owner Author

Perhaps I've just read the note incorrectly...

I think this note is saying that JWK Set implementers need to respect the Base64urlUInt-encoded format, meaning they need to remove padding. That would mean this is not a bug in this project, but should be reported to Firebase.

Either way, this project is being updated to accept any padding on JWK parameters.

@MicahParks MicahParks added the invalid This doesn't seem right label Mar 11, 2024
@MicahParks
Copy link
Owner Author

MicahParks commented Mar 11, 2024

I opened a Case 10274328 with Firebase to let them know of this issue. Here's a copy of the invalid JWK Set if anyone is interested. The "n" JWK parameter on RSA keys is invalid due to the leading padding, it is not in Base64urlUInt-encoded format.

https://firebaseappcheck.googleapis.com/v1/jwks

{
  "keys": [
    {
      "kty": "RSA",
      "use": "sig",
      "alg": "RS256",
      "kid": "lYBWVg",
      "n": "AOpF5dwoCpmW2Th5kBaKDZmygOlyQSJm3JqwGvPTTViHCs4ZitlLF9za9-DPxP3zoNaryEYlFfLhYOFVS7mUjMGtLNTkLafBSIIoF28sy_z1GruxJ2aFchazBimxI1B0MXTKdIw4V268klrOECO5FIcHar7EV9W0XqToFon3oVvHWw3qkPV4o-A7Gdrh3Yh7vRUE_T5XCLYD9jO41nAqYhWYRGN-Kxu51x6VMa595TXTrpzgYGDba1MLQzB9qcHRIvRskt7Gh8M0zgcyo6c6jvktaEzh0j2kdL2JCAFHhMXUZedRUOpeqkEehpxDDR0Deiz7UPlMe6l8Ots97Wm357bgajDcxnqaGGEF5GIkr7xHw15DrTfOWPY35f0sHjNTOn9AU2bPWTy6oHZPhoFjHdSNp3UOIunnf1eXRlTa7YZ5PLmbFFyjNNSnQdcOHgKx1lJExJqXCAJ2pBkp0dX65uiqCLz4WZBcmCHGToi4mvQ5wpFqgUJ_6N8HXpP5ZLZ-hQ",
      "e": "AQAB"
    },
    {
      "kty": "RSA",
      "use": "sig",
      "alg": "RS256",
      "kid": "vc-lTA",
      "n": "AJNncxx--5oS5cEaYe7aFFzaXy5-nY3INfcIZXk3h733VB4OJBlGQ4AZcdssOgisEpTB_tHeS8HyuyaxlsWw0BQzXszHAiaTWSqVzOXzdV7P_R2TM2qjj2l26WlXRgpPpH_-wfQgZLAb4h0jwd2xBCckHklQkMtCaHCgx3nrRxj8rxLuW9Zc0N0gKFtjZYAuWkPynNL8F7O1xJThIwr8krI4ErBeG6LQSZVdqj9tW_JXWO93L4gkA-hNRY8fjodjjP0woOqX-t6E7MwEAjgt-89wHRdBTPnMgAQ5oKXz__K5ME9qSFO6eKJHSYfizVsR4_copWDeLl287EplhW7d_OHRHjbMcxSTuWdXa-VASJR7vdRJJD0px6tCRGCt5wlz2XdZLdo1QC_BCvobQiKBUPF8DGFmDtoVVyJXD4GLnhY_S-hg5y8OXTWugwGgnmx-oWC5mNIhmOVAYXd6ylF92hUjRnksIbP9UP5ex_otpxyHCxQKhHxN7fCLG2ysHEDRyw",
      "e": "AQAB"
    },
    {
      "kty": "RSA",
      "use": "sig",
      "alg": "RS256",
      "kid": "TD0rFQ",
      "n": "AKwXbMXGH3K1LLXIc6nFbaXfkwWQ2N-WOXAva_r4eZEV-U0aXNgA4VDQh6rNabTsgEQT8wxnGWTxmZH7aKdj2HUIOC-XYQ4Yfw1nwFe5qNEXDM4U8lbcF3AUe254cneseKVj-xvE35iJgnJf7Z3yRYtyIrX7mB_xXf4CX50_6hyJTk-DQ7FN08QkbK99v6qAWj9TqcY4bNSZUlR4TXQkiv6c1jFZhHbdrVLizpKB8n5lOBzP3lCU5PlWBjiQBTHs7uU5P-w7qGvR-cqHeo86IP7GP3Dnj7zZb_zMNxmWSTHWTjE0GyflwkEsRJFIXdCZONtjMB5LsD9gEpYw0ZMkgGuqOEjk13ELEkr96PeFaWnH-_Oa4ipaCqxcD15E_dwOjbX_F80US9IIfQoXL0cHheb0s9peDbAAZj1NueQTBDfF6LARgTQtFQM3J0IqKJRF9HWXuZhMwAUV3CM2AiB6Ag21ZQ_4bI1IuiHlBNeHuMo3xQidPRchaA9HvDusRQUE_Q",
      "e": "AQAB"
    },
    {
      "kty": "RSA",
      "use": "sig",
      "alg": "RS256",
      "kid": "XpHJSA",
      "n": "AKx5q-XQdJ3jSQ_2-ngY2VlE2TUdrhve2F-AT5GY6YUSNYEhw8ovLYFvjsKAbvx7zLXH-TyoRgY0gATRAJdbJIo2nafZeq-zolQ1ninJrw_FYMlUsZsXDsXk9XZgWhirbK9Ee4gSvTify2OQBZ4uPzdazUzvL1cPeo8Swi4nf2GBep5mInuCdttEWqMrRSHO2XZJwF6Ngp1Cn1OMVlfgzj0OrT2ml0NNwYrwFGnYdDpZ3auGyDkg6uJuvPVOIAzUHLRE632JS85uSR3BO_-LgQO-v3J_hy3YrtgJ53soTy5-4C0yif2sMFxWIWwET1yJgQEzedUf68MqxKzjvNwPRGKHKJvtx3vcTmwfuQZvfsC1TaOVsITG07P63vXhl-qjXmE01e7AnQuVQs4osSt9Cv9K_M3bxgLQsu2ZCuj3gFGzKPmjrAJZEktToR_jkUDjnP_n3SZ_sQo_4DwMBiEnFtwwUSvCxJRHnc29c5ROUL0kGGb8TPO_2V8qUc4E5voj6Q",
      "e": "AQAB"
    },
    {
      "kty": "RSA",
      "use": "sig",
      "alg": "RS256",
      "kid": "jA3a-Q",
      "n": "ALOx4YXowh58QJTBslhjRzJzacSYLGqsfnSfV4xZapzsJEs2swQdEkhZRC5icTq2k_qRA0sNz0vT64MbO4RI1pNey8sbOCo0tEh8rzGmS1j46EFc0i6PvWUs_31Qz4AqZPF1KNjnacwjJGZUrKjOBF69DphVxdCRAmPWZhEEIsuC--4S5YIsp7Wdh5Fc-thDq9vp7G0klytyUZgQBEy3EunjdmfrN_QeUH9feXZAjKIQwlwVrooGWkKP6UI-7q4fZtK9W0tpIg7-tp6dcREZ4iLPASfntMFZjlmMfYT0f06gzb5nmGXz7gipKsaOyrwbgdZ5_gb0_ugxaWBBZyDmwWVmASyqyPcFU26BxHM2lEtG2yA89y20aSJZ4KRU_uEV6rzxS1jle8638cGPnnJ3KWnFDWlnH_ynAhB2qy1CepvWo4Tq2PPh_TxAOaL2OpzLTv5F-svF0DI-qjIDPfVYexq7lg8qac7_d34ckMJmRoQwCMi9biz7vsu04SgwzZ8c7Q",
      "e": "AQAB"
    }
  ]
}

renovate bot referenced this issue in nobl9/nobl9-go Mar 12, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [github.com/MicahParks/jwkset](https://togithub.com/MicahParks/jwkset)
| `v0.5.14` -> `v0.5.15` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fMicahParks%2fjwkset/v0.5.15?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fMicahParks%2fjwkset/v0.5.15?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fMicahParks%2fjwkset/v0.5.14/v0.5.15?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fMicahParks%2fjwkset/v0.5.14/v0.5.15?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
|
[github.com/MicahParks/keyfunc/v3](https://togithub.com/MicahParks/keyfunc)
| `v3.2.8` -> `v3.2.9` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fMicahParks%2fkeyfunc%2fv3/v3.2.9?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fMicahParks%2fkeyfunc%2fv3/v3.2.9?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fMicahParks%2fkeyfunc%2fv3/v3.2.8/v3.2.9?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fMicahParks%2fkeyfunc%2fv3/v3.2.8/v3.2.9?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>MicahParks/jwkset (github.com/MicahParks/jwkset)</summary>

###
[`v0.5.15`](https://togithub.com/MicahParks/jwkset/releases/tag/v0.5.15):
Less strict validation

[Compare
Source](https://togithub.com/MicahParks/jwkset/compare/v0.5.14...v0.5.15)

The purpose of this release is to use less strict validation for JWK.
This will allow users to work with non-RFC compliant JWK Sets for small
padding mistakes.

Two padding related reasons for this are:

1.  Mandatory leading padding for ECDSA JWK parameters.
2.  A common mistake adding leading padding to RSA JWK parameter "n".

For padding specifically, this project is only comparing integers after
they are parsed from Base64 raw URL encoding by default. To turn on
strict validation, there will be a new field on jwkset.ValidateOptions
named StrictPadding.

An example for `1` would be a bug in this project were mandatory leading
padding was absent:
[https://github.com/MicahParks/jwkset/issues/18](https://togithub.com/MicahParks/jwkset/issues/18)

An example for `2` would be a Firebase service that was reported to be
incompatible with this project:
[https://github.com/MicahParks/jwkset/issues/23](https://togithub.com/MicahParks/jwkset/issues/23)

Relevant issues:

-
[https://github.com/MicahParks/jwkset/issues/23](https://togithub.com/MicahParks/jwkset/issues/23)
-
[https://github.com/MicahParks/jwkset/issues/20](https://togithub.com/MicahParks/jwkset/issues/20)
-
[https://github.com/MicahParks/jwkset/issues/18](https://togithub.com/MicahParks/jwkset/issues/18)

Relevant pull requests:

-
[https://github.com/MicahParks/jwkset/pull/24](https://togithub.com/MicahParks/jwkset/pull/24)

</details>

<details>
<summary>MicahParks/keyfunc (github.com/MicahParks/keyfunc/v3)</summary>

###
[`v3.2.9`](https://togithub.com/MicahParks/keyfunc/compare/v3.2.8...v3.2.9)

[Compare
Source](https://togithub.com/MicahParks/keyfunc/compare/v3.2.8...v3.2.9)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 10pm every weekday,before 5am
every weekday,every weekend" (UTC), Automerge - At any time (no schedule
defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/nobl9/nobl9-go).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMzAuMCIsInVwZGF0ZWRJblZlciI6IjM3LjIzMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@MicahParks
Copy link
Owner Author

Firebase responded to the bug report and said they are working on a fix. However, the case got closed and I've been informed to follow the release notes and blog posts to track resolution.

I'm don't plan on tracking resolution, will be find either way for this project with the new less strict validation update.

@MicahParks
Copy link
Owner Author

MicahParks commented Mar 26, 2024

Firebase emailed me and informed me their engineers have removed the leading padding on their JWK sets.

I checked and it looks like that's true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

1 participant