Issue 3138 - Conformance Tests for BackendTLSPolicy - normative #3212
+595
−49
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,13 +23,15 @@ import ( | |
| "encoding/pem" | ||
| "fmt" | ||
| "io" | ||
| "net" | ||
| "net/http" | ||
| "os" | ||
| "regexp" | ||
| "strconv" | ||
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/paultag/sniff/parser" | ||
| "golang.org/x/net/http2" | ||
| "golang.org/x/net/http2/h2c" | ||
| "golang.org/x/net/websocket" | ||
|
|
@@ -48,15 +50,17 @@ type RequestAssertions struct { | |
| Context `json:",inline"` | ||
|
|
||
| TLS *TLSAssertions `json:"tls,omitempty"` | ||
| SNI string `json:"sni"` | ||
| } | ||
|
|
||
| // TLSAssertions contains information about the TLS connection. | ||
| type TLSAssertions struct { | ||
| Version string `json:"version"` | ||
| PeerCertificates []string `json:"peerCertificates,omitempty"` | ||
| // ServerName is the name sent from the peer using SNI. | ||
| ServerName string `json:"serverName"` | ||
| NegotiatedProtocol string `json:"negotiatedProtocol,omitempty"` | ||
| CipherSuite string `json:"cipherSuite"` | ||
| } | ||
|
|
||
| type preserveSlashes struct { | ||
|
|
@@ -109,6 +113,7 @@ func main() { | |
| httpMux.HandleFunc("/health", healthHandler) | ||
| httpMux.HandleFunc("/status/", statusHandler) | ||
| httpMux.HandleFunc("/", echoHandler) | ||
| httpMux.HandleFunc("/backendTLS", echoHandler) | ||
| httpMux.Handle("/ws", websocket.Handler(wsHandler)) | ||
| httpHandler := &preserveSlashes{httpMux} | ||
|
|
||
|
|
@@ -124,11 +129,13 @@ func main() { | |
|
|
||
| go runH2CServer(h2cPort, errchan) | ||
|
|
||
| // Enable HTTPS if server certificate and private key are given. (TLS_SERVER_CERT, TLS_SERVER_PRIVKEY) | ||
| // Enable secure backend if CA certificate and key are given. (CA_CERT, CA_CERT_KEY) | ||
| if os.Getenv("TLS_SERVER_CERT") != "" && os.Getenv("TLS_SERVER_PRIVKEY") != "" || | ||
| os.Getenv("CA_CERT") != "" && os.Getenv("CA_CERT_KEY") != "" { | ||
| go func() { | ||
| fmt.Printf("Starting server, listening on port %s (https)\n", httpsPort) | ||
| err := listenAndServeTLS(fmt.Sprintf(":%s", httpsPort), os.Getenv("TLS_SERVER_CERT"), os.Getenv("TLS_SERVER_PRIVKEY"), os.Getenv("CA_CERT"), httpHandler) | ||
| if err != nil { | ||
| errchan <- err | ||
| } | ||
|
|
@@ -201,15 +208,29 @@ func runH2CServer(h2cPort string, errchan chan<- error) { | |
| } | ||
|
|
||
| func echoHandler(w http.ResponseWriter, r *http.Request) { | ||
| var sni string | ||
|
|
||
| fmt.Printf("Echoing back request made to %s to client (%s)\n", r.RequestURI, r.RemoteAddr) | ||
|
|
||
| // If the request has form ?delay=[:duration] wait for duration | ||
| // For example, ?delay=10s will cause the response to wait 10s before responding | ||
| err := delayResponse(r) | ||
| if err != nil { | ||
| processError(w, err, http.StatusInternalServerError) | ||
| return | ||
| } | ||
|
|
||
| // If the request was made to URI backendTLS, then get the server name indication and | ||
| // add it to the RequestAssertions. It will be echoed back later. | ||
| if strings.Contains(r.RequestURI, "backendTLS") { | ||
| sni, err = sniffForSNI(r.RemoteAddr) | ||
| if err != nil { | ||
| // TODO: research if for some test cases there won't be SNI available. | ||
|
There was a problem hiding this comment. For this TODO we should either: a) resolve it Ideally, we resolve it but iterative PRs are OK too as long as we have good follow-ups between them. |
||
| processError(w, err, http.StatusBadGateway) | ||
| return | ||
| } | ||
| } | ||
|
|
||
| requestAssertions := RequestAssertions{ | ||
| r.RequestURI, | ||
| r.Host, | ||
|
|
@@ -220,6 +241,7 @@ func echoHandler(w http.ResponseWriter, r *http.Request) { | |
| context, | ||
|
|
||
| tlsStateToAssertions(r.TLS), | ||
| sni, | ||
| } | ||
|
|
||
| js, err := json.MarshalIndent(requestAssertions, "", " ") | ||
|
|
@@ -296,6 +318,41 @@ func listenAndServeTLS(addr string, serverCert string, serverPrivKey string, cli | |
| return srv.ListenAndServeTLS(serverCert, serverPrivKey) | ||
| } | ||
|
|
||
| // sniffForSNI uses the request address to listen for the incoming TLS connection, | ||
| // and tries to find the server name indication from that connection. | ||
| func sniffForSNI(addr string) (string, error) { | ||
| var sni string | ||
|
|
||
| // Listen to get the SNI, and store in config. | ||
| listener, err := net.Listen("tcp", addr) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| defer listener.Close() | ||
|
|
||
| for { | ||
| conn, err := listener.Accept() | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| data := make([]byte, 4096) | ||
| _, err = conn.Read(data) | ||
| if err != nil { | ||
| return "", fmt.Errorf("could not read socket: %v", err) | ||
| } | ||
| // Take an incoming TLS Client Hello and return the SNI name. | ||
| sni, err = parser.GetHostname(data) | ||
| if err != nil { | ||
| return "", fmt.Errorf("error getting SNI: %v", err) | ||
| } | ||
| if sni == "" { | ||
| return "", fmt.Errorf("no server name indication found") | ||
| } else { //nolint:revive | ||
| return sni, nil | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
Comment on lines
+321
to
+355
There was a problem hiding this comment. Do we potentially wanna use http.ListenAndServeTLS here instead of a TCP listener so that we can record (and then assert) the client payload? There was a problem hiding this comment. I'm not sure I understand. Right now all we want is the SNI in this function. There was a problem hiding this comment. So take this as a question not a statement, but what I was getting at was: wouldn't we want to verify receipt of the payload we sent (which should be unique), in addition to the SNI such that we can be certain the request received was the one intended? |
||
| func tlsStateToAssertions(connectionState *tls.ConnectionState) *TLSAssertions { | ||
| if connectionState != nil { | ||
| var state TLSAssertions | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| /* | ||
| Copyright 2024 The Kubernetes Authors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package tests | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "k8s.io/apimachinery/pkg/types" | ||
|
|
||
| "sigs.k8s.io/gateway-api/conformance/utils/http" | ||
| "sigs.k8s.io/gateway-api/conformance/utils/kubernetes" | ||
| "sigs.k8s.io/gateway-api/conformance/utils/suite" | ||
| "sigs.k8s.io/gateway-api/pkg/features" | ||
| ) | ||
|
|
||
| func init() { | ||
| ConformanceTests = append(ConformanceTests, BackendTLSPolicy) | ||
| } | ||
|
|
||
| var BackendTLSPolicy = suite.ConformanceTest{ | ||
| ShortName: "BackendTLSPolicy", | ||
| Description: "A single service that is targeted by a BackendTLSPolicy must successfully complete TLS termination", | ||
| Features: []features.FeatureName{ | ||
| features.SupportGateway, | ||
| features.SupportBackendTLSPolicy, | ||
| }, | ||
| Manifests: []string{"tests/backendtlspolicy.yaml"}, | ||
| Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { | ||
| ns := "gateway-conformance-infra" | ||
| routeNN := types.NamespacedName{Name: "gateway-conformance-infra-test", Namespace: ns} | ||
| gwNN := types.NamespacedName{Name: "gateway-backendtlspolicy", Namespace: ns} | ||
|
|
||
| kubernetes.NamespacesMustBeReady(t, suite.Client, suite.TimeoutConfig, []string{ns}) | ||
|
|
||
| gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) | ||
|
|
||
| serverStr := "abc.example.com" | ||
| headers := make(map[string]string) | ||
| headers["Host"] = serverStr | ||
|
|
||
| // Verify that the response to a call to /backendTLS will return the matching SNI. | ||
| t.Run("Simple request targeting BackendTLSPolicy should reach infra-backend", func(t *testing.T) { | ||
| http.MakeHTTPSRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, | ||
| http.ExpectedResponse{ | ||
| Request: http.Request{ | ||
| Headers: headers, | ||
| Host: serverStr, | ||
| Path: "/backendTLS", | ||
| }, | ||
| Response: http.Response{StatusCode: 200}, | ||
| Namespace: "gateway-conformance-infra", | ||
| ServerName: serverStr, | ||
| }) | ||
| }) | ||
shaneutt marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }, | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get how this works. Are we supposed to send an HTTP request and then send a TLS request later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are just serving this as TLS already you can just do
r.TLS.ServerNameI thinkThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is that you send an HTTP or HTTPS request and the BackendTLSPolicy adds what is needed for the backend request to succeed (certificate and SNI). The echo service finds the SNI and echoes it back to the test to prove the BackendTLSPolicy was used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still need to dig into this a bit deeper, but I think #3212 (comment) is relevant to this question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic seems to be: on an HTTP request, on a new TCP listener, accept a new connection, get the TLS handshake data, then close the connection.
Should it not be: accept an HTTPS request and report the SNI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two connections - one terminates at the gateway and one authenticates with the backend. I was attempting to get the SNI from the second connection here. Maybe this isn't correct, but just accepting one connection is also not correct. Advice gladly accepted @shaneutt and @howardjohn.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the backend though. We don't want to make a connection to ourself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What we want is
Test binary -- (1) TLS 1-->Gateway --(2) TLS 2 --> test appConnection 1 is establihsed by the go test. Connection 2 is established by the gateway implementation under test.
The test app does not establish a connection, it accepts one.