Skip to content

Conversation

ronf
Copy link
Contributor

@ronf ronf commented Aug 30, 2025

@ronf
Copy link
Contributor Author

ronf commented Aug 30, 2025

Hrm. The "AWS-LC" build failure seems to relate to it not finding the function SSL_CTX_set1_client_sigalgs_list. However, I don't see any sign of that only being in more recent OpenSSL releases. Is there something unusual about the version of OpenSSL used there?

@ronf
Copy link
Contributor Author

ronf commented Aug 30, 2025

Ah - I see AWS-LC combines code from BoringSSL and OpenSSL. I guess we'll need to find some way to detect AWS-LC and raise an exception if the call to set the client sigalgs is used, at least for whatever version of AWS-LC triggered this failure.

@ronf
Copy link
Contributor Author

ronf commented Aug 30, 2025

I looked through the AWS-LC code and can confirm it doesn't appear to support SSL_CTX_set1_client_sigalgs_list, even in the latest version. However, I'm not really sure how in the code to test for AWS-LC at build time or run time (either in the main SSL module or in the unit tests). I found some references claiming OPENSSL_IS_AWSLC should exist as a preprocessor symbol, but I don't see any evidence of that being used in CPython at the moment.

@ronf
Copy link
Contributor Author

ronf commented Aug 31, 2025

I decided to go ahead and code up an ssl.get_sigalgs() function which looks like the following:

>>> import ssl
>>> ssl.get_sigalgs()
['ecdsa_secp256r1_sha256', 'ecdsa_secp384r1_sha384', 'ecdsa_secp521r1_sha512',
 'ed25519', 'ed448', 'ecdsa_sha224', 'ecdsa_sha1', 'ecdsa_brainpoolP256r1tls13_sha256',
 'ecdsa_brainpoolP384r1tls13_sha384', 'ecdsa_brainpoolP512r1tls13_sha512', 'rsa_pss_rsae_sha256',
 'rsa_pss_rsae_sha384', 'rsa_pss_rsae_sha512', 'rsa_pss_pss_sha256', 'rsa_pss_pss_sha384',
 'rsa_pss_pss_sha512', 'rsa_pkcs1_sha256', 'rsa_pkcs1_sha384', 'rsa_pkcs1_sha512', 'rsa_pkcs1_sha224',
 'rsa_pkcs1_sha1', 'dsa_sha256', 'dsa_sha384', 'dsa_sha512', 'dsa_sha224', 'dsa_sha1']

It's not checked in yet, but I can do so if we decide to include it, at which point I'll go back and fill in docs and unit tests for it. It seems like it might be useful for a client/server to know the available signature algorithms that it can choose from, since it may vary depending on the OpenSSL implementation in use and what providers are loaded.

Like SSLContext.get_groups(), this method returns a list of strings. The only difference is that this is a global list of available signature algs dependent only on which OpenSSL providers are loaded, rather than being sensitive to state in the SSLContext such as the TLS min & max versions. That's why it ends up being a module-level function.

@picnixz
Copy link
Member

picnixz commented Sep 2, 2025

Is there something unusual about the version of OpenSSL used there?

As you observed, AWS-LC is a fork of BoringSSL & LibreSSL. We decided to try supporting other flavors of libssl implementations because it might come handy for some users that don't want to use OpenSSL.

I looked through the AWS-LC code and can confirm it doesn't appear to support SSL_CTX_set1_client_sigalgs_list, even in the latest version

If this is the case, I suggest one of the following to do:

  • Fix this upstream or collaborate with @WillChilds-Klein on this matter.
  • Detect at configure time whether this is supported or not by the host.
  • Use OPENSSL_IS_AWSLC as claimed. We don't use it currently because everything seems to work on all ssl implementations. If something is specific to OpenSSL, then I would suggest using this macro.

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. You could refactor server_sigalg and client_sigalg into a single function as follows:

static PyObject *
ssl_socket_signame_impl(PySSLSocket *socket,
                        enum py_ssl_server_or_client self_socket_type)
{
#if OPENSSL_VERSION_NUMBER >= 0x30500000L
    int ret;
    const char *sigalg;

    if (socket->ssl == NULL) {
        Py_RETURN_NONE;
    }
    ret = (socket->socket_type == self_socket_type)
        ? SSL_get0_signature_name(socket->ssl, &sigalg)
        : SSL_get0_peer_signature_name(socket->ssl, &sigalg);
    if (ret == 0) {
        Py_RETURN_NONE;
    }
    return PyUnicode_DecodeFSDefault(sigalg);
#else
    PyErr_SetString(PyExc_NotImplementedError,
                    "Getting sig algorithms requires OpenSSL 3.5 or later.");
    return NULL;
#endif
}

and then implement:

static PyObject *
_ssl__SSLSocket_client_sigalg_impl(PySSLSocket *self)
{
    return ssl_socket_signame_impl(self, PY_SSL_CLIENT);
}

static PyObject *
_ssl__SSLSocket_server_sigalg_impl(PySSLSocket *self)
{
    return ssl_socket_signame_impl(self, PY_SSL_SERVER);
}

@ronf
Copy link
Contributor Author

ronf commented Sep 2, 2025

If this is the case, I suggest one of the following to do:

  • Fix this upstream or collaborate with @WillChilds-Klein on this matter.
  • Detect at configure time whether this is supported or not by the host.
  • Use OPENSSL_IS_AWSLC as claimed. We don't use it currently because everything seems to work on all ssl implementations. If something is specific to OpenSSL, then I would suggest using this macro.

I was able to get OPENSSL_IS_AWSLC to work here. I used that in the SSL module and then used the following in the unit tests (with skipUnless):

CAN_SET_CLIENT_SIGALGS = "AWS-LC" not in ssl.OPENSSL_VERSION

With that, the tests are all passing.

@ronf
Copy link
Contributor Author

ronf commented Sep 3, 2025

@picnixz, any thoughts on the proposed ssl.get_sigalgs() function? Should I go ahead and finish the work on that?

@ronf
Copy link
Contributor Author

ronf commented Sep 7, 2025

I had some time this weekend, so I've gone ahead and added support for ssl.get_sigalgs() to return a list of all available TLS signature algorithms in commit 2d47a6c. I've also updated issue #138252 to mention this function.

@ronf
Copy link
Contributor Author

ronf commented Sep 7, 2025

The failure here appears to be in whatsnew/3.15.pst not being able to see the new ssl.get_sigalgs function. That file has no problem seeing other things from the SSL module, though. Also, other documentation files seem to have no issue with it.

Any idea what might be happening here? Do I need an explicit import somewhere? I don't see any such thing for other module references in whatsnew.

@ronf
Copy link
Contributor Author

ronf commented Sep 7, 2025

@picnixz, Anything further you'd like me to change? I think I've covered all the current review comments.

@picnixz
Copy link
Member

picnixz commented Sep 7, 2025

No I think we're good. I'll have a final check tomorrow, and if needed, directly commit to your branch, and then merge it.

@picnixz picnixz enabled auto-merge (squash) September 8, 2025 08:31
@picnixz picnixz merged commit 6401823 into python:main Sep 8, 2025
45 checks passed
@vstinner
Copy link
Member

vstinner commented Sep 8, 2025

Would it be possible to mention the OpenSSL version requirements in the documentation?

@picnixz
Copy link
Member

picnixz commented Sep 8, 2025

That's what I plan to do (I'm actually working on it now): #138633.

@ronf ronf deleted the ssl-sigalg-support branch September 8, 2025 23:56
lkollar pushed a commit to lkollar/cpython that referenced this pull request Sep 9, 2025
…S signature algorithms (python#138269)

The signature algorithms allowed for certificate-based client authentication or
for the server to complete the TLS handshake can be defined on a SSL context via
`ctx.set_client_sigalgs()` and `ctx.set_server_sigalgs()`.

With OpenSSL 3.4 or later, the list of available TLS algorithms can be retrieved
by `ssl.get_sigalgs()`.

With OpenSSL 3.5 or later, the selected signature algorithms can be retrieved from
SSL sockets via `socket.client_sigalg()` and `socket.server_sigalg()`.

This commit also partially amends 377b787
by using `PyUnicode_DecodeFSDefault` instead of `PyUnicode_DecodeASCII` in
`_ssl._SSLContext.get_groups`, so that functions consistently decode strings
obtained from OpenSSL.

---------

Co-authored-by: Bénédikt Tran <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants