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

TLS iRule fails after recent browser updates #3402

Open
htioekb opened this issue May 2, 2024 · 13 comments
Open

TLS iRule fails after recent browser updates #3402

htioekb opened this issue May 2, 2024 · 13 comments

Comments

@htioekb
Copy link

htioekb commented May 2, 2024

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.

@htioekb htioekb added bug untriaged no JIRA created labels May 2, 2024
@vincentmli
Copy link
Contributor

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

@htioekb
Copy link
Author

htioekb commented May 2, 2024

@vincentmli it probably is in

iRule := fmt.Sprintf(`
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; }

@trinaths
Copy link
Contributor

trinaths commented May 3, 2024

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

@trinaths
Copy link
Contributor

trinaths commented May 3, 2024

Created [CONTCNTR-4710] for internal tracking.

@trinaths trinaths added JIRA and removed untriaged no JIRA created labels May 3, 2024
@vincentmli
Copy link
Contributor

@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
Copy link

We're also affected by this issue

@vklohiya
Copy link
Contributor

vklohiya commented May 7, 2024

@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
Copy link

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
Copy link
Contributor

trinaths commented May 8, 2024

@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
Copy link

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
Copy link

pmilot commented May 16, 2024

@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
Copy link
Contributor

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

@trinaths
Copy link
Contributor

@pmilot This fix will be a part of CIS 2.17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants