Skip to content

Conversation

sayboras
Copy link
Member

Please refer to individual commit for more details. The first commit is to simplify the current policy, and pave the way for the second one.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 21, 2025
@github-actions github-actions bot added cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary labels Jan 21, 2025
@sayboras sayboras added the release-note/ci This PR makes changes to the CI. label Jan 21, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 21, 2025
@sayboras sayboras marked this pull request as ready for review January 21, 2025 11:54
@sayboras sayboras requested a review from a team as a code owner January 21, 2025 11:54
@sayboras
Copy link
Member Author

/test

@sayboras sayboras requested review from a team as code owners January 21, 2025 12:57
@sayboras
Copy link
Member Author

/test

Copy link
Member

@jrajahalme jrajahalme left a comment

Choose a reason for hiding this comment

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

Suggestion for improvement.

@sayboras sayboras requested a review from jrajahalme January 22, 2025 00:18
@sayboras
Copy link
Member Author

/test

@sayboras sayboras enabled auto-merge January 22, 2025 22:56
@sayboras
Copy link
Member Author

Rebased to pick up the fix for ci-eks #37184

We don't really need to toFQDNs rule for as the feature under test is
TLS SNI.

Signed-off-by: Tam Mach <tam.mach@cilium.io>
This commit is to add the negative test case for TLS SNI feature with
another external target (e.g. cilium.io). There should be no policy
dropped but SSL error from curl command.

```
$ kex -n cilium-test-1 client-645b68dcf7-n4xx7 -- curl -v https://cilium.io
* Host cilium.io:443 was resolved.
* IPv6: (none)
* IPv4: 104.198.14.52
*   Trying 104.198.14.52:443...
* Connected to cilium.io (104.198.14.52) port 443
* ALPN: curl offers h2,http/1.1
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
*  CAfile: /etc/ssl/certs/ca-certificates.crt
*  CApath: /etc/ssl/certs
* Recv failure: Connection reset by peer
* OpenSSL SSL_connect: Connection reset by peer in connection to cilium.io:443
* Closing connection
curl: (35) Recv failure: Connection reset by peer
command terminated with exit code 35
```

Signed-off-by: Tam Mach <tam.mach@cilium.io>
@sayboras
Copy link
Member Author

/test

@sayboras sayboras added this pull request to the merge queue Jan 23, 2025
Merged via the queue into main with commit e4e5f91 Jan 23, 2025
210 of 212 checks passed
@sayboras sayboras deleted the pr/tammach/tls-sni branch January 23, 2025 11:45
sayboras added a commit to sayboras/cilium that referenced this pull request Jan 31, 2025
The current SNI denied test sends request to cilium.io with serverNames
as one.one.one.one, and expects TLS error. However, cilium.io might not
be as reliable compared to one.one.one.one, hence causes timeout issue
(e.g. 28) instead of expected SSL error code (e.g. 35) as observed in
cilium#37381.

This commit is to reverse the test to use one.one.one.one as
external target, however, new CNP client-egress-tls-sni-other will only
allow serverNames with ExternalOtherTarget (defaults to cilium.io).

Relates: cilium#37122, cilium#37381
Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit to sayboras/cilium that referenced this pull request Jan 31, 2025
The current SNI denied test sends request to cilium.io with serverNames
as one.one.one.one, and expects TLS error. However, cilium.io might not
be as reliable compared to one.one.one.one, hence causes timeout issue
(e.g. 28) instead of expected SSL error code (e.g. 35) as observed in
the issue cilium#37381.

This commit is to reverse the test to use one.one.one.one as
external target, however, new CNP client-egress-tls-sni-other will only
allow serverNames with ExternalOtherTarget (defaults to cilium.io).

Relates: cilium#37122, cilium#37381
Signed-off-by: Tam Mach <tam.mach@cilium.io>
sayboras added a commit to sayboras/cilium that referenced this pull request Jan 31, 2025
The current SNI denied test sends request to cilium.io with serverNames
as one.one.one.one, and expects TLS error. However, cilium.io might not
be as reliable compared to one.one.one.one, hence causes timeout issue
(e.g. 28) instead of expected SSL error code (e.g. 35) as observed in
the issue cilium#37381.

This commit is to reverse the test to use one.one.one.one as
external target, however, new CNP client-egress-tls-sni-other will only
allow serverNames with ExternalOtherTarget (defaults to cilium.io).

Relates: cilium#37122, cilium#37381
Signed-off-by: Tam Mach <tam.mach@cilium.io>
github-merge-queue bot pushed a commit that referenced this pull request Feb 1, 2025
The current SNI denied test sends request to cilium.io with serverNames
as one.one.one.one, and expects TLS error. However, cilium.io might not
be as reliable compared to one.one.one.one, hence causes timeout issue
(e.g. 28) instead of expected SSL error code (e.g. 35) as observed in
the issue #37381.

This commit is to reverse the test to use one.one.one.one as
external target, however, new CNP client-egress-tls-sni-other will only
allow serverNames with ExternalOtherTarget (defaults to cilium.io).

Relates: #37122, #37381
Signed-off-by: Tam Mach <tam.mach@cilium.io>
jongj pushed a commit to jongj/cilium that referenced this pull request Feb 11, 2025
The current SNI denied test sends request to cilium.io with serverNames
as one.one.one.one, and expects TLS error. However, cilium.io might not
be as reliable compared to one.one.one.one, hence causes timeout issue
(e.g. 28) instead of expected SSL error code (e.g. 35) as observed in
the issue cilium#37381.

This commit is to reverse the test to use one.one.one.one as
external target, however, new CNP client-egress-tls-sni-other will only
allow serverNames with ExternalOtherTarget (defaults to cilium.io).

Relates: cilium#37122, cilium#37381
Signed-off-by: Tam Mach <tam.mach@cilium.io>
karina-ranadive pushed a commit to karina-ranadive/cilium that referenced this pull request Jun 24, 2025
The current SNI denied test sends request to cilium.io with serverNames
as one.one.one.one, and expects TLS error. However, cilium.io might not
be as reliable compared to one.one.one.one, hence causes timeout issue
(e.g. 28) instead of expected SSL error code (e.g. 35) as observed in
the issue cilium#37381.

This commit is to reverse the test to use one.one.one.one as
external target, however, new CNP client-egress-tls-sni-other will only
allow serverNames with ExternalOtherTarget (defaults to cilium.io).

Relates: cilium#37122, cilium#37381
Signed-off-by: Tam Mach <tam.mach@cilium.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants