Skip to content

ext/curl: Add CURLOPT_SSL_SIGNATURE_ALGORITHMS option #18692

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ext/curl: Add CURLOPT_SSL_SIGNATURE_ALGORITHMS option #18692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions ext/curl/curl.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3325,6 +3325,13 @@
* @cvalue CURLOPT_SSL_EC_CURVES
*/
const CURLOPT_SSL_EC_CURVES = UNKNOWN;
#if LIBCURL_VERSION_NUM >= 0x080e00 /* Available since 8.14.0 */
/**
* @var int
* @cvalue CURLOPT_SSL_SIGNATURE_ALGORITHMS
*/
const CURLOPT_SSL_SIGNATURE_ALGORITHMS = UNKNOWN;
#endif
/**
* @var int
* @cvalue CURLPX_BAD_ADDRESS_TYPE
Expand Down
5 changes: 4 additions & 1 deletion ext/curl/curl_arginfo.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions ext/curl/interface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2000,6 +2000,9 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue
case CURLOPT_USERPWD:
case CURLOPT_USERNAME:
case CURLOPT_PASSWORD:
#if LIBCURL_VERSION_NUM >= 0x080e00 /* Available since 8.14.0 */
case CURLOPT_SSL_SIGNATURE_ALGORITHMS:
#endif
{
if (Z_ISNULL_P(zvalue)) {
error = curl_easy_setopt(ch->cp, option, NULL);
Expand Down
5 changes: 5 additions & 0 deletions ext/curl/tests/Caddyfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,8 @@ basic_auth /http-basic-auth {
# bcrypt password hash for "password", calculated with 'caddy hash-password'
user $2a$14$yUKl9SGqVTAAqPTzLup.DefsbXXx3kfreNnzpJOUHcIrKnr5lgef2
}

route /ping {
templates
respond `pong`
}
40 changes: 40 additions & 0 deletions ext/curl/tests/curl_setopt_CURLOPT_SSL_SIGNATURE_ALGORITHMS.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
--TEST--
Curl option CURLOPT_SSL_SIGNATURE_ALGORITHMS
--EXTENSIONS--
curl
--SKIPIF--
<?php
$curl_version = curl_version();
if ($curl_version['version_number'] < 0x080e00) die("skip: test works only with curl >= 8.14.0");

include 'skipif-nocaddy.inc';
Copy link
Member

Choose a reason for hiding this comment

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

hmm, this might change things in terms of setting up caddy for windows... I guess it's better than openssl s_server but even better solution would be to use openssl client server test like we use for some http stream tests: for example https://github.com/php/php-src/blob/master/ext/standard/tests/http/ghsa-hgf5-96fm-v528-001.phpt . Then it would work on windows too.

Copy link
Member Author

@Ayesh Ayesh May 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I remember you also commented on #16093 that the whole Caddy server setup on Windows isn't worth it.

Right now, even on Linux builds, we barely test any of the recent Curl stuff because on GitHub Actions, the Curl version is Curl 8.5 or 8.6 while Windows has 8.12.0 or so at the moment. So having these tests run on Windows will be really nice. I'm not really that familiar with OpenSSL s_server/client features, so I could use any help, or take some time to test things around. I have a Windows machine to test things, I'll try what I can :)

That said, I wonder if we really need these tests. We can test if setting a sig-alg works, setting null clears it, check if all the constants exist, etc. But beyond that, we are merely testing Curl's functionality itself. Curl surely has tests for all the variations we can think about, including various TLS backends, so perhaps we can simplify the test to see if the constants exist and if curl_setopt works. We skip the curl_exec part because once we set the options, it's Curl's job to do the actual signature negotiations. What do you think?

?>
--FILE--
<?php

$ch = curl_init('https://localhost/ping');
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false);

var_dump(curl_exec($ch));

var_dump(curl_setopt($ch, CURLOPT_SSL_SIGNATURE_ALGORITHMS, 'invalid-value'));
var_dump(curl_exec($ch));
var_dump(curl_error($ch));

var_dump(curl_setopt($ch, CURLOPT_SSL_SIGNATURE_ALGORITHMS, 'ECDSA+SHA256:RSA+SHA256:DSA+SHA256:ed25519'));
var_dump(curl_exec($ch));

var_dump(curl_setopt($ch, CURLOPT_SSL_SIGNATURE_ALGORITHMS, null));
Copy link
Member

Choose a reason for hiding this comment

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

it might be nice to also test some working signature algorithm...

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right @bukka it could use a positive test path. Thank you.

I added ECDSA+SHA256:RSA+SHA256:DSA+SHA256:ed25519 that should cover all modern cipher-suits and should be future-proof enough.

var_dump(curl_exec($ch));

?>
--EXPECT--
string(4) "pong"
bool(true)
bool(false)
string(52) "failed setting signature algorithms: 'invalid-value'"
bool(true)
string(4) "pong"
bool(true)
string(4) "pong"