aws-lc-rs: consume new TLS-specific APIs #1586

ctz · 2023-11-13T15:26:01Z

aws-lc-rs 1.4 adds some TLS-specific APIs which are implemented inside in the FIPS boundary:

TLS1.2 PRF
an AEAD API that notices if rustls messes up the GCM nonce sequence

This PR moves to using those.

codecov · 2023-11-13T15:32:46Z

Codecov Report

Attention: 54 lines in your changes are missing coverage. Please review.

Comparison is base (b8b1327) 95.90% compared to head (2431bf1) 95.73%.

Files	Patch %	Lines
rustls/src/crypto/aws_lc_rs/tls12.rs	88.15%	27 Missing ⚠️
rustls/src/crypto/aws_lc_rs/tls13.rs	88.37%	25 Missing ⚠️
rustls/src/crypto/cipher.rs	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1586      +/-   ##
==========================================
- Coverage   95.90%   95.73%   -0.18%     
==========================================
  Files          77       79       +2     
  Lines       15742    16188     +446     
==========================================
+ Hits        15098    15498     +400     
- Misses        644      690      +46

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ctz · 2023-11-13T16:14:19Z

looking at why the coverage is so bad for this

edit: it was feature unification again :(

rustls/src/crypto/aws_lc_rs/tls12.rs

admin/coverage

cpu

Here's some initial comments, nothing major!

rustls/src/crypto/aws_lc_rs/tls12.rs

provider-example/src/aead.rs

rustls/src/crypto/aws_lc_rs/tls13.rs

rustls/examples/internal/bogo_shim.rs

cpu · 2023-11-17T16:18:54Z

Hm, I think the aws-lc-sys bump in this branch broke the min versions check:

error[E0599]: no method named `skip_exist` found for struct `fs_extra::dir::CopyOptions` in the current scope
   --> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/aws-lc-sys-0.12.0/builder/main.rs:429:18
    |
428 |               let options = fs_extra::dir::CopyOptions::new()
    |  ___________________________-
429 | |                 .skip_exist(true)
    | |                 -^^^^^^^^^^------ help: remove the arguments
    | |                 ||
    | |_________________|field, not a method
    |

I think the upstream crate needs to specify a diff min version in https://github.com/aws/aws-lc-rs/blob/cb030a33fb22cba4441f2b2d17e3c1c702c39895/aws-lc-sys/Cargo.toml#L55C15-L55C15 ?

ctz · 2023-11-17T16:20:22Z

Indeed -- have dropped a note on aws/aws-lc-rs@27d447f#r132855645

djc · 2023-11-20T10:00:54Z

Would be good to rebase this?

ctz · 2023-11-20T12:42:57Z

Would be good to rebase this?

Done!

Hm, I think the aws-lc-sys bump in this branch broke the min versions check:

About to open a PR there for this, but in the meantime #1601 made this irrelevent.

cpu

LGTM 👍

rustls/src/crypto/aws_lc_rs/tls12.rs

aws-lc-rs 1.5 tracked this API change.

This became unused outside of tests, so isn't really paying its rent.

This is necessary if implementations want to keep state between calls -- (eg, *ring*'s `aead::OpeningKey`). The next commit takes advantage of this.

Because this API is not available for chacha20-poly1305, we keep the old aead::UnboundKey implementation for use by that.

This is not useful.

ctz force-pushed the jbp-aws-lc-rs-tls12-prf branch 2 times, most recently from 37d7467 to b96b8c3 Compare November 14, 2023 16:51

djc reviewed Nov 14, 2023

View reviewed changes

rustls/src/crypto/aws_lc_rs/tls12.rs Outdated Show resolved Hide resolved

rustls/src/crypto/aws_lc_rs/tls12.rs Outdated Show resolved Hide resolved

admin/coverage Show resolved Hide resolved

cpu reviewed Nov 14, 2023

View reviewed changes

ctz force-pushed the jbp-aws-lc-rs-tls12-prf branch 2 times, most recently from 0bd10a7 to 52887f8 Compare November 17, 2023 15:16

ctz changed the base branch from main to jbp-remove-quic-feature November 17, 2023 15:17

ctz force-pushed the jbp-aws-lc-rs-tls12-prf branch 3 times, most recently from c994676 to b53915b Compare November 17, 2023 15:31

ctz force-pushed the jbp-remove-quic-feature branch from e958880 to 77ae2c0 Compare November 17, 2023 16:18

Base automatically changed from jbp-remove-quic-feature to main November 17, 2023 19:34

ctz mentioned this pull request Nov 20, 2023

Use -Z direct-minimal-versions for minimal versions checks #1601

Merged

ctz force-pushed the jbp-aws-lc-rs-tls12-prf branch 2 times, most recently from de8e6c1 to e13d6ac Compare November 20, 2023 11:41

cpu approved these changes Nov 20, 2023

View reviewed changes

djc approved these changes Nov 21, 2023

View reviewed changes

rustls/src/crypto/aws_lc_rs/tls12.rs Outdated Show resolved Hide resolved

ctz added 7 commits November 21, 2023 09:23

Take aws-lc-rs 1.5

8745158

Remove ring_shim::digest_output_len

7b2a2ed

aws-lc-rs 1.5 tracked this API change.

Split off tls12 module for aws-lc-rs

9f662e0

Use aws-lc-rs API for TLS1.2 PRF

61a92d5

Remove crypto::aws_lc_rs::hmac

5350635

This became unused outside of tests, so isn't really paying its rent.

Make receiver of cipher::Message{En,De}crypter mutable

2a6f441

This is necessary if implementations want to keep state between calls -- (eg, *ring*'s `aead::OpeningKey`). The next commit takes advantage of this.

aws_lc_rs::tls12: Use TlsRecordSealingKey API

4a5efae

ctz added 8 commits November 21, 2023 09:25

aws_lc_rs::tls12: Use TlsRecordOpeningKey API

487d8a1

Split off tls13 module for aws-lc-rs

032c66e

Rename Tls13MessageEncrypter -> AeadMessageEncrypter

6e760c6

aws_lc_rs::tls13: Use TlsRecord{Open,Seal}ingKey API

13fe0c3

Because this API is not available for chacha20-poly1305, we keep the old aead::UnboundKey implementation for use by that.

Run bogo against both ring and aws-lc-rs providers

77464b3

Make it easier to feature-gate entire bench program

c45d38d

Support aws-lc-rs in integration-level benchmarks

26eebd8

Outlaw and test export_keying_material with empty output

2431bf1

This is not useful.

ctz force-pushed the jbp-aws-lc-rs-tls12-prf branch from e13d6ac to 2431bf1 Compare November 21, 2023 09:25

ctz added this pull request to the merge queue Nov 21, 2023

Merged via the queue into main with commit c1e34d1 Nov 21, 2023
39 of 42 checks passed

ctz deleted the jbp-aws-lc-rs-tls12-prf branch November 21, 2023 10:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

aws-lc-rs: consume new TLS-specific APIs #1586

aws-lc-rs: consume new TLS-specific APIs #1586

ctz commented Nov 13, 2023

codecov bot commented Nov 13, 2023 •

edited

Loading

ctz commented Nov 13, 2023 •

edited

Loading

cpu left a comment

cpu commented Nov 17, 2023

ctz commented Nov 17, 2023

djc commented Nov 20, 2023

ctz commented Nov 20, 2023

cpu left a comment

aws-lc-rs: consume new TLS-specific APIs #1586

aws-lc-rs: consume new TLS-specific APIs #1586

Conversation

ctz commented Nov 13, 2023

codecov bot commented Nov 13, 2023 • edited Loading

Codecov Report

ctz commented Nov 13, 2023 • edited Loading

cpu left a comment

Choose a reason for hiding this comment

cpu commented Nov 17, 2023

ctz commented Nov 17, 2023

djc commented Nov 20, 2023

ctz commented Nov 20, 2023

cpu left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 13, 2023 •

edited

Loading

ctz commented Nov 13, 2023 •

edited

Loading