Re-decode the ASN.1 TBSCertificate to accept a negative/mis-encoded serial number

[anushasunkada]

Summary by CodeRabbit

• Bug Fixes

  • Improved certificate processing to accept certificates with negative-encoded serial numbers.
  • Preserved the original certificate bytes, ensuring consistent certificate fingerprints.
  • Continued returning clear parsing errors for malformed certificates.

• Tests

  • Added regression coverage for partner certificates using negative ASN.1 serial numbers.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bfdd82fb-9be9-4eee-88ff-e716d294c5fd

📥 Commits

Reviewing files that changed from the base of the PR and between 9ad73f2 and 9976fe1.

📒 Files selected for processing (2)

• esignet-service/internal/keymanager/certparse.go
• esignet-service/internal/keymanager/certparse_test.go

---

Walkthrough

The change adds tolerant DER certificate parsing for negative-encoded serial numbers. It rebuilds and reparses affected certificates, preserves original DER and TBS bytes, validates the behavior with a regression test, and wires the parser into key manager certificate handling.

Changes

Certificate parsing

Layer / File(s)	Summary
Negative serial repair and validation esignet-service/internal/keymanager/certparse.go, esignet-service/internal/keymanager/certparse_test.go	The parser repairs negative serial encodings, preserves the original Raw and RawTBSCertificate bytes, and reports malformed certificate structures. The regression test verifies parsing, subject extraction, and byte preservation.
Key manager parser integration esignet-service/internal/keymanager/service.go	Raw DER and PEM-decoded certificate bytes now use parseCertificateTolerant.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant KeyManager
  participant parseCertificateTolerant
  participant DERReconstruction
  participant x509ParseCertificate
  KeyManager->>parseCertificateTolerant: parse DER or PEM-decoded bytes
  parseCertificateTolerant->>x509ParseCertificate: parse original certificate
  x509ParseCertificate-->>parseCertificateTolerant: negative serial error
  parseCertificateTolerant->>DERReconstruction: rebuild certificate DER
  DERReconstruction-->>parseCertificateTolerant: corrected DER
  parseCertificateTolerant->>x509ParseCertificate: reparse corrected certificate
  x509ParseCertificate-->>parseCertificateTolerant: parsed certificate
  parseCertificateTolerant-->>KeyManager: certificate with original bytes

Loading

Poem

A negative serial meets a careful repair,
DER is rebuilt with control.
The parser reads the corrected form,
While original bytes stay whole.
Tests confirm the subject and TBS.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: tolerant parsing of certificates with negative or mis-encoded serial numbers.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 65.45455% with 19 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (develop-go@31479c6). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
esignet-service/internal/keymanager/certparse.go	66.03%	9 Missing and 9 partials ⚠️
esignet-service/internal/keymanager/service.go	50.00%	1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@              Coverage Diff              @@
##             develop-go    #2341   +/-   ##
=============================================
  Coverage              ?   66.38%           
=============================================
  Files                 ?      124           
  Lines                 ?     8041           
  Branches              ?      110           
=============================================
  Hits                  ?     5338           
  Misses                ?     2279           
  Partials              ?      424           

Flag	Coverage Δ
go	64.90% <65.45%> (?)
npm	92.39% <ø> (?)

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

☔ 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@esignet-service/internal/keymanager/certparse.go`:
- Around line 32-36: Update certparse.go around reencodeNonNegativeSerial to
extract the exact original DER encoding of the TBSCertificate sequence and
assign it to cert.RawTBSCertificate after parsing, rather than replacing only
cert.Raw with the original certificate bytes. In certparse_test.go, add coverage
using the fixture that asserts cert.RawTBSCertificate equals the original
TBSCertificate DER.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9e94943e-ca67-4319-9a05-c24492c4629a

📥 Commits

Reviewing files that changed from the base of the PR and between 31479c6 and 9ad73f2.

📒 Files selected for processing (3)

• esignet-service/internal/keymanager/certparse.go
• esignet-service/internal/keymanager/certparse_test.go
• esignet-service/internal/keymanager/service.go