F5Networks / k8s-bigip-ctlr

Repository for F5 Container Ingress Services for Kubernetes & OpenShift.
Apache License 2.0
365 stars 195 forks source link

TLS iRule fails after recent browser updates #3402

Closed htioekb closed 5 months ago

htioekb commented 7 months ago

Setup Details

CIS Version : 2.16.1
Build: f5networks/k8s-bigip-ctlr:latest
BIGIP Version: Big IP 15.1.10.3
AS3 Version: 3.48
Agent Mode: AS3
Orchestration: K8S
Orchestration Version:
Pool Mode: Nodeport
Additional Setup details: -

Description

A recent chrome update has modified the TLS handshake to include random grease, resulting in some TLS payloads being split into two packets. When the TLS server name extension is in the second packet, the generated iRule fails and the connection is reset.

Original iRule (part):

        when CLIENT_DATA {
            # Byte 0 is the content type.
            # Bytes 1-2 are the TLS version.
            # Bytes 3-4 are the TLS payload length.
            # Bytes 5-$tls_payload_len are the TLS payload.
            binary scan [TCP::payload] cSS tls_content_type tls_version tls_payload_len
            if { ! [ expr { [info exists tls_content_type] && [string is integer -strict $tls_content_type] } ] }  { reject ; event disable all; return; }
            if { ! [ expr { [info exists tls_version] && [string is integer -strict $tls_version] } ] }  { reject ; event disable all; return; }

Fixed iRule (provided by F5 support team):

        when CLIENT_DATA {      
            # Byte 0 is the content type.
            # Bytes 1-2 are the TLS version.
            # Bytes 3-4 are the TLS payload length.
            # Bytes 5-$tls_payload_len are the TLS payload.

           set payload [TCP::payload]
           set payloadlen [TCP::payload length]

           if {![info exists payloadscan]} {
               set payloadscan [binary scan $payload cSS tls_content_type tls_version tls_payload_len ]
           }

           if {($payloadscan == 3)} {
               if {($tls_payload_len < 0 || $tls_payload_len > 16389)} {  
                   log local0.warn "[IP::remote_addr] : parsed SSL/TLS record length ${tls_payload_len} outside of handled length (0..16389)"
                   reject
                   return
               } elseif {($payloadlen < $tls_payload_len+5)} {
                   TCP::collect [expr {$tls_payload_len +5 - $payloadlen}]
                   return
               }

            if { ! [ expr { [info exists tls_content_type] && [string is integer -strict $tls_content_type] } ] }  { reject ; event disable all; return; }
            if { ! [ expr { [info exists tls_version] && [string is integer -strict $tls_version] } ] }  { reject ; event disable all; return; }

Steps To Reproduce

1) Deploy some k8s application with nodeport and TLS reencrypt 2) Access resources on the application through the virtual server

Expected Result

All resources are loaded without errors and connection resets

Actual Result

Some resources fail to load, connection reset.

Diagnostic Information

LTM log message when iRule is executed and connection resets

Apr 24 10:59:06 XXX err tmm[17484]: 01220001:3: TCL error: /f5cis/Shared/crd_IP_443_tls_irule <SERVER_CONNECTED> - can't read "rc": no such variable     while executing "set i $rc"

Observations (if any)

The observed problem occurs in all browsers (Chrome, Edge, Firefox) and the F5 support team confirmed, that recent updates cause the change in behaviour. The iRule provided by F5 reads the whole TLS payload, even if it is split into two packets and allows the TLS payload to be parsed correctly.

vincentmli commented 7 months ago

@trinaths do you know where is the irule deployed by CIS ? I could not find

htioekb commented 7 months ago

@vincentmli it probably is in https://github.com/F5Networks/k8s-bigip-ctlr/blob/4088633fecf61276eb8f502dd9cd33dc244e7071/pkg/controller/routing.go#L715-L723

trinaths commented 7 months ago

@htioekb Please share CIS configuration and error log, steps to reproduce this issue to automation_toolchain_pm automation_toolchain_pm@f5.com

trinaths commented 7 months ago

Created [CONTCNTR-4710] for internal tracking.

vincentmli commented 7 months ago

@trinaths @vklohiya fyi, below is the tested code working for users, please review in case anything missing


when CLIENT_ACCEPTED { TCP::collect }

        proc select_ab_pool {path default_pool domainpath} {
            set last_slash [string length $path]
            set ab_class "/f5cis-dev-infra-tkc/Shared/crd_143_245_148_17_443_ab_deployment_dg"
            while {$last_slash >= 0} {
                if {[class match $path equals $ab_class]} then {
                    break
                }
                set last_slash [string last "/" $path $last_slash]
                incr last_slash -1
                set path [string range $path 0 $last_slash]
            }
            if {$last_slash >= 0} {
                set ab_rule [class match -value $path equals $ab_class]
                if {$ab_rule != ""} then {
                    set weight_selection [expr {rand()}]
                    set service_rules [split $ab_rule ";"]
                    set active_pool ""
                    foreach service_rule $service_rules {
                        set fields [split $service_rule ","]
                        set pool_name [lindex $fields 0]
                        if { [active_members $pool_name] >= 1 } {
                            set active_pool $pool_name
                        }
                        set weight [expr {double([lindex $fields 1])}]
                        if {$weight_selection <= $weight} then {
                            #check if active pool members are available
                            if { [active_members $pool_name] >= 1 } {
                                return $pool_name
                            } else {
                                  # select the any of pool with active members
                                  if {$active_pool!= ""} then {
                                      return $active_pool
                                  }
                            }
                        }
                    }
                }
                # If we had a match, but all weights were 0 then
                # retrun a 503 (Service Unavailable)
                HTTP::respond 503
            }
            return $default_pool
        }

        when CLIENT_DATA {      
            # Byte 0 is the content type.
            # Bytes 1-2 are the TLS version.
            # Bytes 3-4 are the TLS payload length.
            # Bytes 5-$tls_payload_len are the TLS payload.

           set payload [TCP::payload]
           set payloadlen [TCP::payload length]

           if {![info exists payloadscan]} {
               set payloadscan [binary scan $payload cSS tls_content_type tls_version tls_payload_len ]
           }

       if {($payloadscan == 3)} {
               if {($tls_payload_len < 0 || $tls_payload_len > 16389)} {  
                   log local0.warn "[IP::remote_addr] : parsed SSL/TLS record length ${tls_payload_len} outside of handled length (0..16389)"
                   reject
                   return
               } elseif {($payloadlen < $tls_payload_len+5)} {
                   TCP::collect [expr {$tls_payload_len +5 - $payloadlen}]
                   return
               }

            if { ! [ expr { [info exists tls_content_type] && [string is integer -strict $tls_content_type] } ] }  { reject ; event disable all; return; }
            if { ! [ expr { [info exists tls_version] && [string is integer -strict $tls_version] } ] }  { reject ; event disable all; return; }
            switch -exact $tls_version {
                "769" -
                "770" -
                "771" {
                    # Content type of 22 indicates the TLS payload contains a handshake.
                    if { $tls_content_type == 22 } {
                        # Byte 5 (the first byte of the handshake) indicates the handshake
                        # record type, and a value of 1 signifies that the handshake record is
                        # a ClientHello.
                        binary scan $payload @5c tls_handshake_record_type
                        if { ! [ expr { [info exists tls_handshake_record_type] && [string is integer -strict $tls_handshake_record_type] } ] }  { reject ; event disable all; return; }
                        if { $tls_handshake_record_type == 1 } {
                            # Bytes 6-8 are the handshake length (which we ignore).
                            # Bytes 9-10 are the TLS version (which we ignore).
                            # Bytes 11-42 are random data (which we ignore).

                            # Byte 43 is the session ID length.  Following this are three
                            # variable-length fields which we shall skip over.
                            set record_offset 43

                            # Skip the session ID.
                            binary scan $payload @${record_offset}c tls_session_id_len
                            if { ! [ expr { [info exists tls_session_id_len] && [string is integer -strict $tls_session_id_len] } ] }  { reject ; event disable all; return; }
                            incr record_offset [expr {1 + $tls_session_id_len}]

                            # Skip the cipher_suites field.
                            binary scan $payload @${record_offset}S tls_cipher_suites_len
                            if { ! [ expr { [info exists tls_cipher_suites_len] && [string is integer -strict $tls_cipher_suites_len] } ] }  { reject ; event disable all; return; }
                            incr record_offset [expr {2 + $tls_cipher_suites_len}]

                            # Skip the compression_methods field.
                            binary scan $payload @${record_offset}c tls_compression_methods_len
                            if { ! [ expr { [info exists tls_compression_methods_len] && [string is integer -strict $tls_compression_methods_len] } ] }  { reject ; event disable all; return; }
                            incr record_offset [expr {1 + $tls_compression_methods_len}]

                            # Get the number of extensions, and store the extensions.
                            binary scan $payload @${record_offset}S tls_extensions_len
                            if { ! [ expr { [info exists tls_extensions_len] && [string is integer -strict $tls_extensions_len] } ] }  { reject ; event disable all; return; }
                            incr record_offset 2
                            binary scan $payload @${record_offset}a* tls_extensions
                            if { ! [info exists tls_extensions] }  { reject ; event disable all; return; }
                            for { set extension_start 0 }
                                    { $tls_extensions_len - $extension_start == abs($tls_extensions_len - $extension_start) }
                                    { incr extension_start 4 } {
                                # Bytes 0-1 of the extension are the extension type.
                                # Bytes 2-3 of the extension are the extension length.
                                binary scan $tls_extensions @${extension_start}SS extension_type extension_len
                                if { ! [ expr { [info exists extension_type] && [string is integer -strict $extension_type] } ] }  { reject ; event disable all; return; }
                                if { ! [ expr { [info exists extension_len] && [string is integer -strict $extension_len] } ] }  { reject ; event disable all; return; }

                                # Extension type 00 is the ServerName extension.
                                if { $extension_type == "00" } {
                                    # Bytes 4-5 of the extension are the SNI length (we ignore this).

                                    # Byte 6 of the extension is the SNI type.
                                    set sni_type_offset [expr {$extension_start + 6}]
                                    binary scan $tls_extensions @${sni_type_offset}S sni_type
                                    if { ! [ expr { [info exists sni_type] && [string is integer -strict $sni_type] } ] }  { reject ; event disable all; return; }

                                    # Type 0 is host_name.
                                    if { $sni_type == "0" } {
                                        # Bytes 7-8 of the extension are the SNI data (host_name)
                                        # length.
                                        set sni_len_offset [expr {$extension_start + 7}]
                                        binary scan $tls_extensions @${sni_len_offset}S sni_len
                                        if { ! [ expr { [info exists sni_len] && [string is integer -strict $sni_len] } ] }  { reject ; event disable all; return; }

                                        # Bytes 9-$sni_len are the SNI data (host_name).
                                        set sni_start [expr {$extension_start + 9}]
                                        binary scan $tls_extensions @${sni_start}A${sni_len} tls_servername
                                    }
                                }

                                incr extension_start $extension_len
                            }
                            if { [info exists tls_servername] } {
                                set servername_lower [string tolower $tls_servername]
                                set domain_length [llength [split $servername_lower "."]]
                                set domain_wc [domain $servername_lower [expr {$domain_length - 1}] ]
                                # Set wc_host with the wildcard domain
                                set wc_host ".$domain_wc"
                                set passthru_class "/f5cis-dev-infra-tkc/Shared/crd_143_245_148_17_443_ssl_passthrough_servername_dg"
                                if { [class exists $passthru_class] } {
                                    # check if the passthrough data group has a record with the servername
                                    set passthru_dg_key [class match $servername_lower equals $passthru_class]
                                    set passthru_dg_wc_key [class match $wc_host equals $passthru_class]
                                    if { $passthru_dg_key != 0 || $passthru_dg_wc_key != 0 } {
                                        SSL::disable serverside
                                        set dflt_pool_passthrough ""

                                        # Disable Serverside SSL for Passthrough Class
                                        set dflt_pool_passthrough [class match -value $servername_lower equals $passthru_class]
                                        # If no match, try wildcard domain
                                        if { $dflt_pool_passthrough == "" } {
                                            if { [class match $wc_host equals $passthru_class] } {
                                                    set dflt_pool_passthrough [class match -value $wc_host equals $passthru_class]
                                            }
                                        }
                                        if { not ($dflt_pool_passthrough equals "") } {
                                            SSL::disable
                                            HTTP::disable
                                        }

                                        set ab_class "/f5cis-dev-infra-tkc/Shared/crd_143_245_148_17_443_ab_deployment_dg"
                                        if { not [class exists $ab_class] } {
                                            if { $dflt_pool_passthrough == "" } then {
                                                log local0.debug "Failed to find pool for $servername_lower $"
                                            } else {
                                                pool $dflt_pool_passthrough
                                            }
                                        } else {
                                            set selected_pool [call select_ab_pool $servername_lower $dflt_pool_passthrough ""]
                                            if { $selected_pool == "" } then {
                                                log local0.debug "Failed to find pool for $servername_lower"
                                            } else {
                                                pool $selected_pool
                                            }
                                        }
                                    }
                                }
                            }
                        }
                    }
                }
            }

            TCP::release
                    }
        }

        when CLIENTSSL_HANDSHAKE {
            SSL::collect
        }

         when CLIENTSSL_DATA {
            if { [llength [split [SSL::payload]]] < 1 }{
                reject ; event disable all; return;
                }
            set sslpath [lindex [split [SSL::payload]] 1]
            set domainpath $sslpath
            set routepath ""
            set wc_routepath ""

            if { [info exists tls_servername] } {
                set servername_lower [string tolower $tls_servername]
                set domain_length [llength [split $servername_lower "."]]
                set domain_wc [domain $servername_lower [expr {$domain_length - 1}] ]
                set wc_host ".$domain_wc"
                # Set routepath as combination of servername and url path
                append routepath $servername_lower $sslpath
                append wc_routepath $wc_host $sslpath
                set routepath [string tolower $routepath]
                set wc_routepath [string tolower $wc_routepath]
                set sslpath $routepath
                # Find the number of "/" in the routepath
                set rc 0
                foreach x [split $routepath {}] {
                   if {$x eq "/"} {
                       incr rc
                   }
                }
                # Disable serverside ssl and enable only for reencrypt routes
                SSL::disable serverside
                set reencrypt_class "/f5cis-dev-infra-tkc/Shared/crd_143_245_148_17_443_ssl_reencrypt_servername_dg"
                set edge_class "/f5cis-dev-infra-tkc/Shared/crd_143_245_148_17_443_ssl_edge_servername_dg"
                if { [class exists $reencrypt_class] || [class exists $edge_class] } {
                    # Compares the routepath with the entries in ssl_reencrypt_servername_dg and
                    # ssl_edge_servername_dg.
                    for {set i $rc} {$i >= 0} {incr i -1} {
                        if { [class exists $reencrypt_class] } {
                            set reen_pool [class match -value $routepath equals $reencrypt_class]
                            # Check for wildcard domain
                            if { $reen_pool equals "" } {
                                if { [class match $wc_routepath equals $reencrypt_class] } {
                                    set reen_pool [class match -value $wc_routepath equals $reencrypt_class]
                                }
                            }
                            if { not ($reen_pool equals "") } {
                                set dflt_pool $reen_pool
                                SSL::enable serverside
                            }
                        }
                        if { [class exists $edge_class] } {
                            set edge_pool [class match -value $routepath equals $edge_class]
                            # Check for wildcard domain
                            if { $edge_pool equals "" } {
                                if { [class match $wc_routepath equals $edge_class] } {
                                    set edge_pool [class match -value $wc_routepath equals $edge_class]
                                }
                            }
                            if { not ($edge_pool equals "") } {
                                set dflt_pool $edge_pool
                            }
                        }
                        if { not [info exists dflt_pool] } {
                            set routepath [
                                string range $routepath 0 [
                                    expr {[string last "/" $routepath]-1}
                                ]
                            ]
                            set wc_routepath [
                                string range $wc_routepath 0 [
                                    expr {[string last "/" $wc_routepath]-1}
                                ]
                            ]
                        }
                        else {
                            break
                        }
                    }
                }
                # handle the default pool for virtual server
                set default_class "/f5cis-dev-infra-tkc/Shared/crd_143_245_148_17_443_default_pool_servername_dg"
                 if { [class exists $default_class] } {
                    set dflt_pool [class match -value "defaultPool" equals $default_class]
                 }

                # Handle requests sent to unknown hosts.
                # For valid hosts, Send the request to respective pool.
                if { not [info exists dflt_pool] } then {
                     # Allowing HTTP2 traffic to be handled by policies and closing the connection for HTTP/1.1 unknown hosts.
                     if { not ([SSL::payload] starts_with "PRI * HTTP/2.0") } {
                        reject ; event disable all;
                        log local0.debug "Failed to find pool for $servername_lower"
                        return;
                    }
                } else {
                    pool $dflt_pool
                }
                set ab_class "/f5cis-dev-infra-tkc/Shared/crd_143_245_148_17_443_ab_deployment_dg"
                if { [class exists $ab_class] } {
                    set selected_pool [call select_ab_pool $servername_lower $dflt_pool $domainpath]
                    if { $selected_pool == "" } then {
                        log local0.debug "Unable to find pool for $servername_lower"
                    } else {
                        pool $selected_pool
                    }
                }
            }
            SSL::release
        }

        when SERVER_CONNECTED {
            set reencryptssl_class "/f5cis-dev-infra-tkc/Shared/crd_143_245_148_17_443_ssl_reencrypt_serverssl_dg"
            set edgessl_class "/f5cis-dev-infra-tkc/Shared/crd_143_245_148_17_443_ssl_edge_serverssl_dg"
            if { [info exists sslpath] and [class exists $reencryptssl_class] } {
                # Find the nearest child path which matches the reencrypt_class
                for {set i $rc} {$i >= 0} {incr i -1} {
                    if { [class exists $reencryptssl_class] } {
                        set reen [class match -value $sslpath equals $reencryptssl_class]
                        # check for wildcard domain match
                        if { $reen equals "" } {
                            if { [class match $wc_routepath equals $reencryptssl_class] } {
                                set reen [class match -value $wc_routepath equals $reencryptssl_class]
                            }
                        }
                        if { not ($reen equals "") } {
                                set sslprofile $reen
                        }
                    }
                    if { [class exists $edgessl_class] } {
                        set edge [class match -value $sslpath equals $edgessl_class]
                        # check for wildcard domain match
                        if { $edge equals "" } {
                            if { [class match $wc_routepath equals $edgessl_class] } {
                                set edge [class match -value $wc_routepath equals $edgessl_class]
                            }
                        }
                        if { not ($edge equals "") } {
                                set sslprofile $edge
                        }

                    }
                    if { not [info exists sslprofile] } {
                        set sslpath [
                            string range $sslpath 0 [
                                expr {[string last "/" $sslpath]-1}
                            ]
                        ]
                        set wc_routepaath [
                            string range $wc_routepath 0 [
                                expr {[string last "/" $wc_routepath]-1}
                            ]
                        ]
                    }
                    else {
                        break
                    }
                }
                # Assign respective SSL profile based on ssl_reencrypt_serverssl_dg
                if { not ($sslprofile equals "false") } {
                        SSL::profile $reen
                }
            }
        }
shkarface commented 6 months ago

We're also affected by this issue

vklohiya commented 6 months ago

@shkarface , Please use this build and verify the fix.

Build: cisbot/k8s-bigip-ctlr:browserUpdateIssue

Note:- This is a dev build and where only the smoke tests are performed.

shkarface commented 6 months ago

I have build the image from source after changing the irules in the source code to what you have provided, and it's working like a charm

trinaths commented 6 months ago

@shkarface , Please use this build and verify the fix.

Build: cisbot/k8s-bigip-ctlr:browserUpdateIssue

Note:- This is a dev build and where only the smoke tests are performed.

@shkarface Is this build working good for this issue ?

shkarface commented 6 months ago

I have not checked this build tbh, as mentioned before we have a custom build that fixes this issue, because we're also affected by another bug #3401 so we need a custom build

pmilot commented 6 months ago

@shkarface , Please use this build and verify the fix.

Build: cisbot/k8s-bigip-ctlr:browserUpdateIssue

Note:- This is a dev build and where only the smoke tests are performed.

@vklohiya Was this dev build built using 2.16.1 + irule fix or was it built from source prior to 2.16.1 ?

vklohiya commented 6 months ago

@pmilot , It's build on top of 2.16.1.

trinaths commented 6 months ago

@pmilot This fix will be a part of CIS 2.17

lavahot commented 5 months ago

Any idea when this fix will be released?

vklohiya commented 5 months ago

@lavahot , It will be released next week.

vklohiya commented 5 months ago

Fixed with CIS 2.17.0 release, Closing the issue.