Skip to content

Implement BIO_dump #2331

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 6 commits into from
May 15, 2025
Merged

Implement BIO_dump #2331

merged 6 commits into from
May 15, 2025

Conversation

kingstjo
Copy link
Contributor

Issues:

Addresses #CryptoAlg-3030

Description of changes:

Implemented BIO_dump() function in AWS-LC, matching OpenSSL 1.1.1's functionality. This function enables hexadecimal dumping of data through the BIO interface, providing compatibility for applications that rely on this OpenSSL API.

  • Uses existing hexdump infrastructure in crypto/bio/hexdump.c
  • Maintains OpenSSL 1.1.1 API compatibility
  • Returns total bytes written to match OpenSSL behavior

Call-outs:

  • Function maintains compatibility with OpenSSL 1.1.1 API specification
  • Returns total bytes written, matching OpenSSL's behavior

Testing:

Added test in crypto/bio/bio_test.cc

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.

@kingstjo kingstjo requested a review from a team as a code owner April 11, 2025 02:03
@codecov-commenter
Copy link

codecov-commenter commented Apr 11, 2025

Codecov Report

Attention: Patch coverage is 88.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 78.87%. Comparing base (eafd940) to head (ed1eef0).
Report is 55 commits behind head on main.

Files with missing lines Patch % Lines
crypto/bio/hexdump.c 82.35% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2331      +/-   ##
==========================================
+ Coverage   78.76%   78.87%   +0.11%     
==========================================
  Files         620      621       +1     
  Lines      107861   108458     +597     
  Branches    15319    15401      +82     
==========================================
+ Hits        84953    85550     +597     
+ Misses      22252    22239      -13     
- Partials      656      669      +13     

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

@samuel40791765 samuel40791765 self-requested a review April 15, 2025 20:18
kingstjo added 2 commits May 12, 2025 11:25
- Modified BIO_dump implementation to use a memory BIO for capturing the formatted output
- Return exact bytes written instead of an estimate
- Added specific test assertions in bio_test.cc to verify the return value
- Updated documentation in bio.h to clarify the return value behavior
…handling

- Replace C-style arrays with std::array for better type safety
- Add explicit expected output strings to verify exact hexdump format
- Add comprehensive edge case testing (NULL data, NULL BIO, negative length)
- Improve assertions with descriptive error messages
- Add multi-line output testing for larger data sets
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

There were too many comments to post at once. Showing the first 10 out of 11. Check the log or trigger a new build to see more.

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

@kingstjo kingstjo requested a review from justsmth May 13, 2025 18:24
@samuel40791765 samuel40791765 merged commit 96cab72 into aws:main May 15, 2025
110 of 114 checks passed
skmcgrail added a commit that referenced this pull request May 23, 2025
## What's Changed
* Fix prefix build when path has spaces by @justsmth in
#2400
* Prepare v1.51.2 by @justsmth in
#2401
* Set OPENSSL_NO_EXTERNAL_PSK_TLS13 to indicate lack of TLS 1.3 PSK by
@WillChilds-Klein in #2399
* BIO datagram functions by @justsmth in
#2321
* Reject NewSessionTicket messages with empty tickets in TLS 1.3 by
@justsmth in #2367
* Ensure that AVX512 is not used on macOS by @justsmth in
#2363
* Fix socket test issues by @torben-hansen in
#2404
* Remove python CI patch for main by @WillChilds-Klein in
#2407
* Remove xmlsec patch by @smittals2 in
#2405
* Fix clang tidy ci by @justsmth in
#2375
* Mark fallible container operations as `nodiscard` by @justsmth in
#2366
* Remove extra va_end in err_add_error_vdata by @justsmth in
#2364
* Check for QUIC in SSL_process_quic_post_handshake by @justsmth in
#2365
* Add missing symbols for Unbound by @nhatnghiho in
#2352
* Update mlkem-native by @hanno-becker in
#2406
* CI for iOS by @justsmth in #2389
* Squelch clang-tidy by @justsmth in
#2414
* Clang-tidy is still noisy by @justsmth in
#2417
* Add back two rules for clang-tidy by @smittals2 in
#2418
* Implement BIO_dump by @kingstjo in
#2331
* Make ASN1_get_object a direct call by @samuel40791765 in
#2332
* Add Python 3.9 CI patch by @WillChilds-Klein in
#2415
* Rework memory BIOs and implement BIO_seek by @nhatnghiho in
#2380
* ML-DSA: ASN.1 Module - add parsing of BOTH private key format by
@jakemas in #2416
* Detection of unused results by @justsmth in
#2411
* Fix gtest_util.sh failure detection by @justsmth in
#2423
* Remove unused docs/configs by @torben-hansen in
#2427
* ML-DSA: Add ML-DSA keyGen to break-kat.go by @jakemas in
#2422
* Fix CI for mingw by @justsmth in
#2428
* Bump AWSLC_API_VERSION for X509_STORE_CTX_set_verify_crit_oids by
@samuel40791765 in #2426
* Revert "Rework memory BIOs and implement BIO_seek (#2380)" by
@samuel40791765 in #2432
* Resolve SSL_PRIVATE_METHOD and certificate slots functionality by
@skmcgrail in #2429

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.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants