Skip to content

librepgp: reject packet tag 0 as required by RFC 4880 - #2489

Open
ronaldtse wants to merge 1 commit into
mainfrom
fix-2397-reject-tag0
Open

ronaldtse wants to merge 1 commit into
mainfrom
fix-2397-reject-tag0

Conversation

@ronaldtse

Copy link
Copy Markdown
Contributor

Summary

  • RFC 4880, section 4.3, states that a packet tag must not have the value 0, which is reserved. The parser previously accepted such a header and skipped the packet as unknown, while GnuPG rejects the same input, and the difference was found through differential fuzzing and reported by @afldl in Reserved packet tag 0 accepted — RFC 4880 §4.3 MUST NOT violation #2397.
  • stream_peek_packet_hdr() now returns RNP_ERROR_BAD_FORMAT when the decoded tag is 0, for both the new-format (0xc0) and the old-format (0x80) header spellings, which covers the parsing, dumping and keyring load paths that share this header gate.
  • Other reserved tag values, which the RFC does not single out in the same way, continue to be skipped as unknown, and this change does not affect them.
  • The new test_issue_2397_tag0 test feeds both header spellings to the packet dump and the key import paths and asserts that each fails.

Test plan

  • rnp_tests.test_issue_2397_tag0 passes against both spellings of the reserved tag
  • the parser-related tests (load_pgp, load_g10, load_g23, load_kbx, stream_key_load, the dump tests and fuzz_dump) continue to pass
  • full CI matrix

RFC 4880, section 4.3, states that a packet tag must not have the value
0, which is reserved. The packet parser previously accepted such a
header and skipped the packet as unknown, while GnuPG rejects the same
input, so the behaviour was reported as an interop difference found by
differential fuzzing. stream_peek_packet_hdr() now returns
RNP_ERROR_BAD_FORMAT for both the new-format and the old-format
spellings of tag 0, which covers the parsing, dumping and keyring load
paths that share this header gate. Other reserved tag values, which the
RFC does not single out in the same way, continue to be skipped.

Fixes #2397.
@codecov

codecov Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.43%. Comparing base (26482f6) to head (8c4d1f9).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2489      +/-   ##
==========================================
- Coverage   85.45%   85.43%   -0.03%     
==========================================
  Files         125      125              
  Lines       23042    23042              
==========================================
- Hits        19691    19685       -6     
- Misses       3351     3357       +6     

☔ View full report in Codecov by Harness.
📢 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.

@ronaldtse
ronaldtse requested a review from ni4 September 21, 2026 07:29
@ronaldtse

Copy link
Copy Markdown
Contributor Author

@ni4 a small batch of CI and process pull requests has been prepared from the issue backlog, each of which needs only your approval:

Whenever time allows, a look at any of these would be much appreciated.

ni4 pushed a commit that referenced this pull request Sep 24, 2026
The aggregated coverage report is assembled from uploads of many CI
legs across backends and platforms, and rounding differences between
those uploads shift the project percentage by a few hundredths of a
point. This makes the strict zero-drift project gate fail on pull
requests that do not reduce coverage, for example #2489 and #2492.
A 0.1 percent threshold absorbs the aggregate noise while genuine
regressions still fail the check.

@ni4 ni4 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

This branch has not been deployed

No deployments
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