Skip to content

Conversation

nhatnghiho
Copy link
Contributor

Issues:

Resolves #CryptoAlg-3386

Description of changes:

Implement the following options for x509 cli command:

  • extfile
  • CA
  • CAkey
  • outform
  • extensions
  • pubkey

Call-outs:

I made several edits to the test utilities functions that involve comparing AWS-LC and OpenSSL's certificate outputs. These functions are moved to a centralized location to make the code look cleaner.

Testing:

Unit tests added, including missing coverage for some existing x509 options.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@nhatnghiho nhatnghiho requested a review from a team as a code owner October 8, 2025 17:00
Copy link
Contributor

@justsmth justsmth left a comment

Choose a reason for hiding this comment

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

Would you rebase this onto the latest on main?

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@codecov-commenter
Copy link

codecov-commenter commented Oct 9, 2025

Codecov Report

❌ Patch coverage is 29.74576% with 829 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.64%. Comparing base (6d8caa7) to head (21f7808).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
tool-openssl/x509_test.cc 20.12% 512 Missing ⚠️
tool-openssl/test_util.cc 10.08% 209 Missing and 5 partials ⚠️
tool-openssl/x509.cc 73.89% 53 Missing ⚠️
tool-openssl/verify_test.cc 26.22% 45 Missing ⚠️
tool-openssl/req_test.cc 0.00% 3 Missing ⚠️
tool-openssl/rehash_test.cc 94.44% 0 Missing and 1 partial ⚠️
tool-openssl/test_util.h 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2735      +/-   ##
==========================================
- Coverage   78.93%   78.64%   -0.29%     
==========================================
  Files         677      678       +1     
  Lines      115525   116334     +809     
  Branches    16250    16302      +52     
==========================================
+ Hits        91187    91492     +305     
- Misses      23542    24053     +511     
+ Partials      796      789       -7     

☔ 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.

@justsmth
Copy link
Contributor

There are a few failing in the X509 comparison tests: https://github.com/aws/aws-lc/actions/runs/18593051373/job/53012312901?pr=2735

[  FAILED  ] 4 tests, listed below:
[  FAILED  ] X509ComparisonTest.X509ToolCompareReqSignkeyDaysOpenSSL
[  FAILED  ] X509ComparisonTest.X509ToolCompareSignkeyOutformPEMOpenSSL
[  FAILED  ] X509ComparisonTest.X509ToolCompareSignkeyOutformDEROpenSSL
[  FAILED  ] X509ComparisonTest.X509ToolCompareReqCAOpenSSL

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@nhatnghiho nhatnghiho requested a review from smittals2 October 17, 2025 18:32
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.

4 participants