michael/webmin
1
0
Fork 0
mirror of https://github.com/webmin/webmin.git synced 2026-09-15 10:00:40 +01:00
Code Issues Actions 3 Packages Projects Releases Wiki Activity

Fix TLS client certificate verification

Browse Source 
ⓘ Prevent certificate-chain errors from being overwritten while preserving password-login fallback for invalid optional certificates.

Reset verification state for every TLS handshake and add regression coverage.
This commit is contained in:
Ilia Ross
2026-09-13 03:17:13 +02:00
parent d4c14d8d5e
commit afc256831c
3 changed files with 136 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -18,6 +18,7 @@
* Fix module name validation to prevent loading files outside installed modules
* Fix ACL operations bypassing configured allowed paths in File Manager
* Fix arbitrary file reads in Software Packages module
* Fix TLS client certificate verification
#### 2.660 (August 20, 2026)
* Add support for creating `vfsv1` Linux quota files for limits above 4 TiB, while preserving existing quota file formats

32
miniserv-lib.pl
View File

@@ -2510,11 +2510,19 @@ foreach $d (@_) {
sub verify_client
{
local $cert = Net::SSLeay::X509_STORE_CTX_get_current_cert($_[1]);
if ($cert) {
local $errnum = Net::SSLeay::X509_STORE_CTX_get_error($_[1]);
$verified_client = 1 if (!$errnum);
local ($preverify_ok, $ctx) = @_;
local $cert = Net::SSLeay::X509_STORE_CTX_get_current_cert($ctx);
local $errnum = Net::SSLeay::X509_STORE_CTX_get_error($ctx);
if (!$preverify_ok || $errnum) {
# Once any certificate fails, later successful checks must not
# mark the whole chain as verified.
$verified_client = 0;
}
elsif ($cert && !defined($verified_client)) {
$verified_client = 1;
}
# Continue the handshake so an invalid optional client certificate can fall
# back to another login method instead of preventing access altogether.
return 1;
}
@@ -3625,11 +3633,9 @@ if ($@) {
if ($client_certs) {
Net::SSLeay::CTX_load_verify_locations(
$ssl_ctx, $config{'ca'}, "");
eval {
Net::SSLeay::set_verify(
$ssl_ctx, &Net::SSLeay::VERIFY_PEER, \&verify_client);
};
if ($@) {
# Register verification on the context so every SSL connection created
# from it invokes the client-certificate callback.
if (defined(&Net::SSLeay::CTX_set_verify)) {
Net::SSLeay::CTX_set_verify(
$ssl_ctx, &Net::SSLeay::VERIFY_PEER, \&verify_client);
}
@@ -3710,6 +3716,13 @@ if (!$sn) {
local (undef, $myip, undef) = &get_address_ip($sn, $ipv6);
local $ssl_ctx = $ssl_contexts{$myip} || $ssl_contexts{"*"};
local $ssl_con = Net::SSLeay::new($ssl_ctx->{'ctx'});
if ($client_certs && !defined(&Net::SSLeay::CTX_set_verify)) {
# Older Net::SSLeay versions only provide the connection-level API.
# Register it on the SSL object rather than passing an incompatible
# context.
Net::SSLeay::set_verify(
$ssl_con, &Net::SSLeay::VERIFY_PEER, \&verify_client);
}
if ($config{'ssl_cipher_list'}) {
# Force use of ciphers
eval "Net::SSLeay::set_cipher_list(
@@ -3722,6 +3735,7 @@ if ($config{'ssl_cipher_list'}) {
# Accept the SSL connection
Net::SSLeay::set_fd($ssl_con, fileno($sock));
$verified_client = undef;
alarm(10);
$SIG{'ALRM'} = sub { die "timeout" };
my $ok = Net::SSLeay::accept($ssl_con);

112
t/miniserv.t
View File

@@ -16,6 +16,7 @@ use warnings;
use Test::More;
use File::Basename qw(dirname);
use File::Spec;
use File::Temp qw(tempfile);
my $lib = File::Spec->rel2abs(
File::Spec->catfile(dirname(__FILE__), '..', 'miniserv-lib.pl'));
@@ -786,6 +787,117 @@ subtest 'password_crypt' => sub {
'DES stored hash + correct password verifies');
};
# verify_client — preserve optional-certificate fallback without allowing a
# successful certificate check to hide an error elsewhere in the chain.
subtest 'verify_client' => sub {
no warnings qw(redefine once);
my $has_cert = 1;
my $error = 0;
local *Net::SSLeay::X509_STORE_CTX_get_current_cert = sub {
return $has_cert;
};
local *Net::SSLeay::X509_STORE_CTX_get_error = sub {
return $error;
};
# A fully valid chain is eligible for certificate authentication.
{
local $miniserv::verified_client;
is(miniserv::verify_client(1, 'ctx'), 1,
'valid issuer check continues the handshake');
is(miniserv::verify_client(1, 'ctx'), 1,
'valid leaf check continues the handshake');
is($miniserv::verified_client, 1,
'all-valid chain is marked verified');
}
# OpenSSL checks from the issuer toward the leaf, so a bad leaf must clear
# an earlier success while still allowing password authentication fallback.
{
local $miniserv::verified_client;
$error = 0;
miniserv::verify_client(1, 'ctx');
$error = 10;
is(miniserv::verify_client(0, 'ctx'), 1,
'invalid leaf still continues the handshake');
is($miniserv::verified_client, 0,
'invalid leaf clears successful issuer state');
}
# Do not let a later successful callback erase an earlier chain error.
{
local $miniserv::verified_client;
$error = 20;
miniserv::verify_client(0, 'ctx');
$error = 0;
miniserv::verify_client(1, 'ctx');
is($miniserv::verified_client, 0,
'chain remains invalid after a later successful check');
}
# Treat pre-verification failure as authoritative even if a binding reports
# no numeric error.
{
local $miniserv::verified_client;
$error = 0;
miniserv::verify_client(0, 'ctx');
is($miniserv::verified_client, 0,
'pre-verification failure rejects the chain');
}
# A successful callback without a current certificate cannot establish
# client identity, while an error without one still rejects the chain.
{
local $miniserv::verified_client;
$has_cert = 0;
$error = 0;
miniserv::verify_client(1, 'ctx');
is($miniserv::verified_client, undef,
'callback without a certificate does not verify a client');
$error = 20;
miniserv::verify_client(0, 'ctx');
is($miniserv::verified_client, 0,
'error without a current certificate rejects the chain');
}
};
# create_ssl_context — client-certificate verification belongs to the SSL
# context, not to an individual SSL connection object.
subtest 'create_ssl_context client verification' => sub {
no warnings qw(redefine once);
my ($keyfh, $keyfile) = tempfile();
my ($certfh, $certfile) = tempfile();
close($keyfh);
close($certfh);
my @verify_args;
my $legacy_calls = 0;
local $miniserv::client_certs = 1;
local %miniserv::config = (
'ca' => $certfile,
'dhparams_file' => "$certfile.missing",
);
local *Net::SSLeay::new_x_ctx = sub { return 'ctx'; };
local *Net::SSLeay::VERIFY_PEER = sub { return 1; };
local *Net::SSLeay::FILETYPE_PEM = sub { return 1; };
local *Net::SSLeay::CTX_load_verify_locations = sub { return 1; };
local *Net::SSLeay::CTX_set_verify = sub { @verify_args = @_; };
local *Net::SSLeay::set_verify = sub { $legacy_calls++; };
local *Net::SSLeay::CTX_use_PrivateKey_file = sub { return 1; };
local *Net::SSLeay::CTX_use_certificate_file = sub { return 1; };
local *Net::SSLeay::CTX_set_options = sub { return 1; };
local *miniserv::cert_names = sub { return { 'alt' => [] }; };
my $result = miniserv::create_ssl_context($keyfile, $certfile, 'none');
is($result->{'ctx'}, 'ctx', 'SSL context is created');
is($verify_args[0], 'ctx', 'verification is registered on the context');
is($verify_args[1], 1,
'peer verification is enabled');
is($verify_args[2], \&miniserv::verify_client,
'client-certificate callback is registered');
is($legacy_calls, 0, 'connection-level verification API is not used');
};
# hash_session_id — three independent code paths, picked by which crypto
# globals are set. Each branch gets its own subtest so the cache and globals
# can be reset cleanly via `local`.