Skip to content

RFC7518 Section 6.2 Byte Count Conformance#91

Open
justin-lovell wants to merge 4 commits intoauthlib:mainfrom
justin-lovell:rfc7518_section_6-2
Open

RFC7518 Section 6.2 Byte Count Conformance#91
justin-lovell wants to merge 4 commits intoauthlib:mainfrom
justin-lovell:rfc7518_section_6-2

Conversation

@justin-lovell
Copy link
Copy Markdown

This pull request improves the handling of EC (Elliptic Curve) key serialization to ensure that integer values are always encoded to the full byte size required by the curve, preventing potential truncation of leading zeros and ensuring compliance with RFC 7518. It also adds comprehensive tests to verify this behavior for different curve types.

Tests Implemented

Introduced new tests in test_ec_key.py to verify that exported EC keys for the P-256, P-384, and P-521 curves always emit base64url-encoded values of the expected full length, regardless of the leading zeros in the underlying integer values.

These changes are to conform to the explicit requirements of
payload sizes according to section 6.2 on RFC7518.

6.2.1.2 and 6.2.1.3
> The length of this octet string MUST be the full size of a
> coordinate for the curve specified in the "crv" parameter.

6.2.2.1
> The length of this octet string MUST be ceiling(log-base-2(n)/8)
> octets (where n is the order of the curve)
Copilot AI review requested due to automatic review settings April 1, 2026 01:49
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves EC JWK serialization to ensure curve integers are encoded at the full, fixed byte length required by RFC 7518 §6.2 (preserving leading zero octets), and adds tests to validate the exported base64url lengths for common NIST curves.

Changes:

  • Extend int_to_base64 to support fixed-width integer encoding via an optional byte_count.
  • Update EC key export to always encode x, y, and d using the curve’s fixed byte size.
  • Add tests asserting fixed base64url string lengths for P-256, P-384, and P-521 exports.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
src/joserfc/util.py Adds optional fixed-width support to integer→base64url encoding used by JWK exporters.
src/joserfc/_rfc7518/ec_key.py Exports EC key parameters using fixed byte counts to avoid truncating leading zeros.
tests/jwk/test_ec_key.py Adds tests ensuring EC JWK exports always emit base64url values of expected fixed length.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@justin-lovell
Copy link
Copy Markdown
Author

I've updated the code according to the feedback provided by Copilot. Python isn't used in my day job, so if there are other improvements please point it out.

The non-conformance to the RFC7518 specification was discovered when inspecting application logs failed to emit valid ephemeral key parameters in ECDH-ES+A256KW payloads.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 3, 2026

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.86%. Comparing base (8aeb191) to head (f881c6a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/joserfc/util.py 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #91      +/-   ##
==========================================
- Coverage   99.93%   99.86%   -0.07%     
==========================================
  Files          47       47              
  Lines        2923     2929       +6     
  Branches      337      339       +2     
==========================================
+ Hits         2921     2925       +4     
- Misses          2        3       +1     
- Partials        0        1       +1     
Flag Coverage Δ
unittests 99.86% <77.77%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants