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

VSOCK socket client would almost always fail #576

Closed
solokacher opened this issue Jun 2, 2023 · 4 comments
Closed

VSOCK socket client would almost always fail #576

solokacher opened this issue Jun 2, 2023 · 4 comments
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue

Comments

@solokacher
Copy link

solokacher commented Jun 2, 2023

Describe the bug

Function s_update_local_endpoint(struct aws_socket *socket) defined at
source/posix/socket.c#L273 has the following logic:

        /* VSOCK port is 32bit, but aws_socket_endpoint.port is only 16bit.
         * Hopefully this isn't an issue, since users can only pass in 16bit values.
         * But if it becomes an issue, we'll need to make aws_socket_endpoint more flexible */
        if (s->svm_port > UINT16_MAX) {
            AWS_LOGF_ERROR(
                AWS_LS_IO_SOCKET,
                "id=%p fd=%d: aws_socket_endpoint can't deal with VSOCK port > UINT16_MAX",
                (void *)socket,
                socket->io_handle.data.fd);
            return aws_raise_error(AWS_IO_SOCKET_INVALID_ADDRESS);
        }
        tmp_endpoint.port = (uint16_t)s->svm_port;

When it is used as a client, it would almost always fail, as the ephemeral port grabbed from the system is likely be large number. How did we test this code?

Expected Behavior

aws_socket_connect() should succeed

Current Behavior

Failure with aws_socket_endpoint can't deal with VSOCK port > UINT16_MAX

Reproduction Steps

Create a client and connect to any VSOCK address

Possible Solution

use 32 bit port number

Additional Information/Context

No response

aws-c-io version used

version containing df07e42

Compiler and version used

clang14

Operating System and version

Linux

@solokacher solokacher added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 2, 2023
@graebm graebm added the feature-request A feature should be added or improved. label Jun 6, 2023
@graebm
Copy link
Contributor

graebm commented Jun 6, 2023

VSOCK support isn't 100% complete, as you've discovered. Someone submitted this partial support, and we integrated it to unblock them, but never bound it out to any other languages.

Finishing support would mean widening aws_socket_endpoint.port, add error-checking anywhere we cast it gets narrowed to 16bits, widen the value in our other language bindings, etc, etc, etc. It's not on our current roadmap.

I assume you're trying to use this directly from C? What's your use case?

@graebm graebm removed needs-triage This issue or PR still needs to be triaged. bug This issue is a bug. labels Jun 6, 2023
@yasminetalby yasminetalby added response-requested Waiting on additional info and feedback. Will move to 'closing-soon' in 7 days. p2 This is a standard priority issue labels Jun 23, 2023
meerd added a commit to meerd/aws-nitro-enclaves-sdk-c that referenced this issue Jul 28, 2023
Newer aws-c-io versions after v0.11.0 create run-time issues due to a
limitation described here:
awslabs/aws-c-io#576

We need to fallback to the older version of aws-c-io until this
limitation gets addressed and solved.

Signed-off-by: Erdem Meydanli <[email protected]>
eugkoira pushed a commit to aws/aws-nitro-enclaves-sdk-c that referenced this issue Aug 7, 2023
Newer aws-c-io versions after v0.11.0 create run-time issues due to a
limitation described here:
awslabs/aws-c-io#576

We need to fallback to the older version of aws-c-io until this
limitation gets addressed and solved.

Signed-off-by: Erdem Meydanli <[email protected]>
@github-actions
Copy link

github-actions bot commented Aug 9, 2023

Greetings! It looks like this issue hasn’t been active in longer than a week. We encourage you to check if this is still an issue in the latest release. Because it has been longer than a week since the last update on this, and in the absence of more information, we will be closing this issue soon. If you find that this is still a problem, please feel free to provide a comment or add an upvote to prevent automatic closure, or if the issue is already closed, please feel free to open a new one.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Aug 9, 2023
@graebm
Copy link
Contributor

graebm commented Dec 9, 2023

I'm working on a fix...

@graebm
Copy link
Contributor

graebm commented Jan 5, 2024

It is done
port is 32bits now everywhere

@graebm graebm closed this as completed Jan 5, 2024
dwhjames added a commit to dwhjames/aws-nitro-enclaves-sdk-c that referenced this issue Jun 1, 2024
Release [v0.4.1](https://github.com/aws/aws-nitro-enclaves-sdk-c/releases/tag/v0.4.1)
at commit [550f731](aws@550f731) (PR [aws#121](aws#121))
pinned the version of [awslabs/[email protected]](https://github.com/awslabs/aws-c-io/releases/tag/v0.11.0).

This issue was reported in this project in Issue [aws#122](aws#122) and upstream in Issue [awslabs/aws-c-io#576](awslabs/aws-c-io#576).

**Fixed versions**

Issue [awslabs/aws-c-io#576](awslabs/aws-c-io#576) was fixed in [awslabs/aws-c-io#613](awslabs/aws-c-io#613). The fix commit [749c87e](awslabs/aws-c-io@749c87e) was first released in [v0.14.0](https://github.com/awslabs/aws-c-io/releases/tag/v0.14.0).

There were a further 3 cascading changes.

1. PR [awslabs/aws-c-common#1079](awslabs/aws-c-common#1079) where fix commit [8eaa098](awslabs/aws-c-common@8eaa098) was first released in [v0.9.12](https://github.com/awslabs/aws-c-common/releases/tag/v0.9.12).
2. PR [awslabs/aws-c-http#457](awslabs/aws-c-http#457) where fix commit [6a1c157](awslabs/aws-c-http@6a1c157) was first released in [v0.8.0](https://github.com/awslabs/aws-c-http/releases/tag/v0.8.0).
3. PR [awslabs/aws-c-auth#220](awslabs/aws-c-auth#220) where fix commit [6ba7a0f](awslabs/aws-c-auth@6ba7a0f) was first released in [v0.7.10](https://github.com/awslabs/aws-c-auth/releases/tag/v0.7.10)

---

**Remaining changes**

[awslabs/aws-c-sdkutils](https://github.com/awslabs/aws-c-sdkutils) was at [v0.1.2](https://github.com/awslabs/aws-c-sdkutils/releases/tag/v0.1.2). The latest compatible patch release is [v0.1.15](https://github.com/awslabs/aws-c-sdkutils/releases/tag/v0.1.15).
The next patch release [v0.1.16](https://github.com/awslabs/aws-c-sdkutils/releases/tag/v0.1.16) breaks due to paired changes in [awslabs/aws-c-sdkutils#39](awslabs/aws-c-sdkutils#39) and [awslabs/aws-c-common#1105](awslabs/aws-c-common#1105).

[awslabs/aws-c-compression](https://github.com/awslabs/aws-c-compression) was at [v0.2.14](https://github.com/awslabs/aws-c-compression/releases/tag/v0.2.14). The latest patch release is [v0.2.18](https://github.com/awslabs/aws-c-compression/releases/tag/v0.2.18).

[awslabs/aws-c-cal](https://github.com/awslabs/aws-c-cal) was at [v0.5.18](https://github.com/awslabs/aws-c-cal/releases/tag/v0.5.18). Linking compatibility now requires at least [v0.6.0](https://github.com/awslabs/aws-c-cal/releases/tag/v0.6.0) due dependencies on the changes in [awslabs/aws-c-cal#152](awslabs/aws-c-cal#152). The latest patch release is [v0.6.15](https://github.com/awslabs/aws-c-cal/releases/tag/v0.6.15).

[aws/s2n-tls](https://github.com/aws/s2n-tls) was at [v1.3.46](https://github.com/aws/s2n-tls/releases/tag/v1.3.46). At [v1.4.0](https://github.com/aws/s2n-tls/releases/tag/v1.4.0) it changed its version pinning for [aws/aws-lc](https://github.com/aws/aws-lc) to [v1.17.4](https://github.com/aws/aws-lc/releases/tag/v1.17.4).

The latest release of [json-c](https://github.com/json-c/json-c) is [json-c-0.17-20230812](https://github.com/json-c/json-c/releases/tag/json-c-0.17-20230812).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

3 participants