10000 Rework memory BIOs and implement BIO_seek by nhatnghiho · Pull Request #2380 · aws/aws-lc · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Rework memory BIOs and implement BIO_seek #2380

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
May 15, 2025
Merged

Conversation

nhatnghiho
Copy link
Contributor
@nhatnghiho nhatnghiho commented Apr 30, 2025

Issues:

Resolves #CryptoAlg-3111

Description of changes:

This PR creates a wrapper around BUF_MEM and adds a new field off to indicate the current offset of the read pointer. This enables us to implement BIO_seek, which is also in this PR.

Testing:

Unit tests are modified and added to cover more test cases.

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.

@codecov-commenter
Copy link
8000 codecov-commenter commented Apr 30, 2025

Codecov Report

Attention: Patch coverage is 93.54839% with 6 lines in your changes missing coverage. Please review.

Project coverage is 78.84%. Comparing base (1ef205e) to head (c320788).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crypto/bio/bio_mem.c 90.32% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2380      +/-   ##
==========================================
+ Coverage   78.82%   78.84%   +0.01%     
==========================================
  Files         621      621              
  Lines      108431   108500      +69     
  Branches    15389    15397       +8     
==========================================
+ Hits        85469    85543      +74     
+ Misses      22292    22288       -4     
+ Partials      670      669       -1     

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

@nhatnghiho nhatnghiho marked this pull request as ready for review April 30, 2025 17:15
@nhatnghiho nhatnghiho requested a review from a team as a code owner April 30, 2025 17:15
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 21. Check the log or trigger a new build to see more.

github-actions[bot]

This comment was marked as duplicate.

github-actions[bot]

This comment was marked as duplicate.

github-actions[bot]

This comment was marked as duplicate.

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

justsmth
justsmth previously approved these changes May 14, 2025
@WillChilds-Klein WillChilds-Klein requested a review from justsmth May 14, 2025 20:49
@nhatnghiho nhatnghiho merged commit 5a74835 into aws:main May 15, 2025
113 of 115 checks passed
samuel40791765 pushed a commit to samuel40791765/aws-lc that referenced this pull request May 21, 2025
samuel40791765 added a commit that referenced this pull request May 21, 2025
This reverts commit 5a74835.

Our Ruby CI checks with 3.4-3.2 started failing with this commit. This
reverts the commit until we know what's up.

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 added a commit to nhatnghiho/aws-lc that referenced this pull request May 21, 2025
### Issues:
Resolves #CryptoAlg-3111

### Description of changes: 
This PR creates a wrapper around `BUF_MEM` and adds a new field `off` to
indicate the current offset of the read pointer. This enables us to
implement `BIO_seek`, which is also in this PR.

### Testing:
Unit tests are modified and added to cover more test cases.

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.
justsmth pushed a commit that referenced this pull request May 23, 2025
### Issues:
Resolves #CryptoAlg-3111

### Description of changes: 
Same as #2380. 

Fixed the state bug that was causing Ruby failures. The failures came
from `BIO_reset`, which tries to "rewind" the R/O buffer to its original
state. The old implementation only works when the buffer pointer and
read pointer are aligned. But in the new memory BIO, this is not always
the case. The buffer pointer (`b->data`) does not need to align with the
read pointer (since we can have a positive `bbm->read_off`). Our tests
didn't cover this because the buffer pointer was synced with read
pointer after every test case.

### Testing: 
Added tests for `BIO_reset` and `BIO_seek` functions that attempt to
rewind the buffer internally. The tests cover both cases -- when buffer
pointer and read pointer are synced and when they are not.

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.
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.
nhatnghiho added a commit to nhatnghiho/aws-lc that referenced this pull request Jun 6, 2025
Resolves #CryptoAlg-3111

Same as aws#2380.

Fixed the state bug that was causing Ruby failures. The failures came
from `BIO_reset`, which tries to "rewind" the R/O buffer to its original
state. The old implementation only works when the buffer pointer and
read pointer are aligned. But in the new memory BIO, this is not always
the case. The buffer pointer (`b->data`) does not need to align with the
read pointer (since we can have a positive `bbm->read_off`). Our tests
didn't cover this because the buffer pointer was synced with read
pointer after every test case.

Added tests for `BIO_reset` and `BIO_seek` functions that attempt to
rewind the buffer internally. The tests cover both cases -- when buffer
pointer and read pointer are synced and when they are not.

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 added a commit to nhatnghiho/aws-lc that referenced this pull request Jun 10, 2025
Resolves #CryptoAlg-3111

Same as aws#2380.

Fixed the state bug that was causing Ruby failures. The failures came
from `BIO_reset`, which tries to "rewind" the R/O buffer to its original
state. The old implementation only works when the buffer pointer and
read pointer are aligned. But in the new memory BIO, this is not always
the case. The buffer pointer (`b->data`) does not need to align with the
read pointer (since we can have a positive `bbm->read_off`). Our tests
didn't cover this because the buffer pointer was synced with read
pointer after every test case.

Added tests for `BIO_reset` and `BIO_seek` functions that attempt to
rewind the buffer internally. The tests cover both cases -- when buffer
pointer and read pointer are synced and when they are not.

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