Skip to content

Conversation

@x2018
Copy link
Contributor

@x2018 x2018 commented Nov 7, 2025

This PR does a precaution for possible malicious certs_num from peer. By doing so, we do not need to worry about the possible issues of the cast for num_certs, or malicious servers which send abnomal number of certs.
Note that MAX_ALLOWED_CERT_AMOUNT is based on the code of https://github.com/curl/curl/blob/master/lib/vtls/openssl.c#L352.
The handling is also a copy of code from openssl.c:

  numcerts = sk_X509_num(sk);
  if(numcerts > MAX_ALLOWED_CERT_AMOUNT) {
    failf(data, "%d certificates is more than allowed (%u)", (int)numcerts,
          MAX_ALLOWED_CERT_AMOUNT);
    return CURLE_SSL_CONNECT_ERROR;
  }

Other:
The type of the second parameter of rustls_connection_get_peer_certificate is size_t, so we do not need to cast the num_certs to int there, just like the calling at line 1232(cert = rustls_connection_get_peer_certificate(rconn, i);).

@github-actions github-actions bot added the TLS label Nov 7, 2025
@x2018 x2018 marked this pull request as draft November 7, 2025 13:39
@x2018 x2018 marked this pull request as ready for review November 7, 2025 14:06
@x2018
Copy link
Contributor Author

x2018 commented Nov 7, 2025

The backend wolfssl and mbedtls uses specific APIs to handle the certs. I am not familiar with them so I just keep them as they are.

@x2018 x2018 force-pushed the rustls_integer_of_or_dos branch from 0b315a1 to 0f7fe69 Compare November 7, 2025 14:49
@testclutch

This comment was marked as outdated.

@x2018
Copy link
Contributor Author

x2018 commented Nov 8, 2025

The backend wolfssl and mbedtls uses specific APIs to handle the certs. I am not familiar with them so I just keep them as they are.

I made an attempt. If anything is improper, I will reset the changes for them.

wolfSSL_CTX_load_verify_buffer() have a buffer size limitation already. By the way, I add a precaution check for preventing invalid socket for wolfssl.

For mbedtls, it sets the verification callback via mbedtls_ssl_conf_verify(). Our local function is mbed_verify_cb(). I find that it does not have a return, so I just skip to handle the certinfo if the num exceeds MAX_ALLOWED_CERT_AMOUNT .

@bagder bagder closed this in 9c0ccd2 Nov 8, 2025
@bagder
Copy link
Member

bagder commented Nov 8, 2025

Thanks!

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

Labels

Development

Successfully merging this pull request may close these issues.

4 participants