Skip to content
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

apiclient: update tungstenite to v0.20.1 #3491

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

arnaldo2792
Copy link
Contributor

Description of changes:

Newer versions of tungstenite implemented the v13 version of the Web Sockets protocol (described in rfc6455). In this version, new headers are required in the upgrade request. With this update, now the upgrade request sent by apiclient include the missing headers.

In the same version, the 'frame' message was added. Neither apiserver nor apiclient send this type of message, so they are ignored by apiclient.

I found the required headers here. For the values I followed the RFC:

The request MUST contain a |Host| header field whose value contains /host/ plus optionally ":" followed by /port/ (when not using the default port).

The request MUST contain an |Upgrade| header field whose value MUST include the "websocket" keyword

The request MUST contain a |Connection| header field whose value MUST include the "Upgrade" token.

The request MUST include a header field with the name |Sec-WebSocket-Key|. The value of this header field MUST be a nonce consisting of a randomly selected 16-byte value that has been base64-encoded (see Section 4 of [RFC4648]). The nonce MUST be selected randomly for each connection.

The request MUST include a header field with the name |Sec-WebSocket-Version|. The value of this header field MUST be 13.

Testing done:
I built the aws-ecs-1 and aws-ecs-2 variants, and confirmed I could exec commands through the admin container:

[ssm-user@control]$ apiclient get os
{
  "os": {
    "arch": "x86_64",
    "build_id": "14d6c7d1",
    "pretty_name": "Bottlerocket OS 1.16.0 (aws-ecs-2)",
    "variant_id": "aws-ecs-2",
    "version_id": "1.16.0"
  }
}
[ssm-user@control]$ apiclient exec admin sheltie cat /etc/os-release
NAME=Bottlerocket
ID=bottlerocket
VERSION="1.16.0 (aws-ecs-2)"
PRETTY_NAME="Bottlerocket OS 1.16.0 (aws-ecs-2)"
VARIANT_ID=aws-ecs-2
VERSION_ID=1.16.0
BUILD_ID=14d6c7d1
HOME_URL="https://github.com/bottlerocket-os/bottlerocket"
SUPPORT_URL="https://github.com/bottlerocket-os/bottlerocket/discussions"
BUG_REPORT_URL="https://github.com/bottlerocket-os/bottlerocket/issues"
DOCUMENTATION_URL="https://bottlerocket.dev"

I also confirmed that I can enter the admin container:

[ssm-user@control]$ enter-admin-container
Confirming admin container is enabled...
Waiting for admin container to start...
Entering admin container
          Welcome to Bottlerocket's admin container!
    ╱╲
   ╱┄┄╲   This container provides access to the Bottlerocket host
   │▗▖│   filesystems (see /.bottlerocket/rootfs) and contains common
  ╱│  │╲  tools for inspection and troubleshooting.  It is based on
  │╰╮╭╯│  Amazon Linux 2, and most things are in the same places you
    ╹╹    would find them on an AL2 host.

To permit more intrusive troubleshooting, including actions that mutate the
running state of the Bottlerocket host, we provide a tool called "sheltie"
(`sudo sheltie`).  When run, this tool drops you into a root shell in the
Bottlerocket host's root filesystem.
[root@admin]#

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Comment on lines 80 to 82
# tungstenite uses a newer version of base64, but must of the
# first party crates are still in this version
{ name = "base64", version = "=0.13.1" }
Copy link
Member

Choose a reason for hiding this comment

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

After the next tough release, we can switch everything to 0.21.

sources/api/apiclient/Cargo.toml Outdated Show resolved Hide resolved
Copy link
Member

@webern webern left a comment

Choose a reason for hiding this comment

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

I think in the past we had a case where apiclient processes became zombies and maybe tungstenite was to blame? Anyone remember? Should we check for zombies?

@bcressey
Copy link
Contributor

Zombies showed up in #1384 and #1507 but I think that was before apiclient exec.

sources/deny.toml Outdated Show resolved Hide resolved
Newer versions of tungstenite implemented the v13 version of the Web
Sockets protocol (described in https://www.rfc-editor.org/rfc/rfc6455).
In this version, new headers are required in the upgrade request. With
this update, now the upgrade request sent by apiclient include the
missing headers.

In the same version, the 'frame' message was added. Neither apiserver
nor apiclient send this type of message, so they are ignored by
apiclient.

Signed-off-by: Arnaldo Garcia Rincon <agarrcia@amazon.com>
@arnaldo2792
Copy link
Contributor Author

(Forced push fixes the comments in the last revision)

@arnaldo2792 arnaldo2792 merged commit 4304f33 into bottlerocket-os:develop Sep 28, 2023
48 checks passed
@arnaldo2792 arnaldo2792 deleted the tungstenite-update branch January 29, 2024 16:45
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.

None yet

5 participants