Skip to content

Fix smtp starttls - #13048

Closed
Flakebi wants to merge 1 commit into
curl:masterfrom
Flakebi:fix-smtps
Closed

Flakebi wants to merge 1 commit into
curl:masterfrom
Flakebi:fix-smtps

Conversation

@Flakebi

@Flakebi Flakebi commented Mar 5, 2024

Copy link
Copy Markdown
Contributor

In cases where the connection was fast, curl sometimes failed to open a connection.
This fixes a regression of c2d9736.

The regression triggered in these steps:

  1. Create an smtp connection

  2. Use STARTTLS

  3. Receive the response

  4. We are inside the loop in smtp_statemachine, calling smtp_state_starttls_resp

  5. In the good flow, we exit the loop, re-enter smtp_statemachine and run smtp_perform_upgrade_tls at the start of the function.

    In the bad flow, we stay in the while loop, calling Curl_pp_readresp, which reads part of the TLS handshake and things go wrong.

The reason is that Curl_pp_moredata changed behavior and always returns true, so we stay in the loop in smtp_statemachine. With a slow connection Curl_pp_readresp cannot read new data and returns CURL_AGAIN, so we leave the loop and re-enter smtp_statemachine.

With a fast connection, Curl_pp_readresp reads new data from the tcp connection, which is part of the TLS handshake.

The fix is in Curl_pp_moredata, which needs to take the final line into account and return false if only the final line is stored.

A test would be nice, but I’m not accustomed to the testing system, so I there’s no test in this change.

In cases where the connection was fast, curl sometimes failed to open a
connection.
This fixes a regression of c2d9736.

The regression triggered in these steps:
1. Create an smtp connection
2. Use STARTTLS
3. Receive the response
4. We are inside the loop in `smtp_statemachine`, calling `smtp_state_starttls_resp`
5. In the good flow, we exit the loop, re-enter `smtp_statemachine` and
   run `smtp_perform_upgrade_tls` at the start of the function.

   In the bad flow, we stay in the while loop, calling `Curl_pp_readresp`,
   which reads part of the TLS handshake and things go wrong.

The reason is that `Curl_pp_moredata` changed behavior and always
returns `true`, so we stay in the loop in `smtp_statemachine`. With a
slow connection `Curl_pp_readresp` cannot read new data and returns
`CURL_AGAIN`, so we leave the loop and re-enter `smtp_statemachine`.

With a fast connection, `Curl_pp_readresp` reads new data from the tcp
connection, which is part of the TLS handshake.

The fix is in `Curl_pp_moredata`, which needs to take the final line
into account and return `false` if only the final line is stored.
@bagder

bagder commented Mar 5, 2024

Copy link
Copy Markdown
Member

Thanks!

@Flakebi
Flakebi deleted the fix-smtps branch March 5, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants