SSL CA certificate validation BUG

Submitted by george.asenov on Mon, 04/30/2018 - 02:35

Hello,

We have discovered a bug in virtualmin regarding CA chain validation.

The issue we faced was that user pasted chain consisting two certificates in virtualmin but the "END CERTIFICATE" line in the second cert was like this:

-----END CERTIFICATE——

but should be

-----END CERTIFICATE-----

The Key word here is the SECOND cert.

After I reviewed the code that virtualmin use and test the whole chain with opensssl like virtualmin does it I realise that virtualmin check only the first certificate in the chain.

the command VM is using to check certs is :

$out = &backquote_command("openssl x509 -in ".quotemeta($temp)." -issuer -subject -enddate 2>&1");

where in $temp is the all the pasted content with removed new lines.

I tested and if I execute

openssl x509 -in "cert.txt" -issuer -subject -enddate

when cert.txt contin the two certificates in the chain it only check the first one. And it went through without issues. While there are issues in the second cert end line.

but if cert.txt contin the only the broken cert it throw an error

</error:0906D066:PEM routines:PEM_read_bio:bad end line:pem_lib.c:802:>

which is expected.

Here is the code in virtualmin which isn't correct

file: /usr/libexec/webmin/virtual-server/newchain.cgi lines 46-60

elsif ($in{'mode'} == 3) {
    # New pasted text
    $in{'paste'} =~ s/\r//g;
    $in{'paste'} || &error($text{'chain_epaste'});
    $err = &check_certificate_data($in{'paste'});
    $err && &error(&text('chain_ecert', $err));
    $chain = &default_certificate_file($d, 'ca');
    &lock_file($chain);
    &unlink_file_as_domain_user($d, $chain);
    &open_tempfile_as_domain_user($d, CERT, ">$chain");
    &print_tempfile(CERT, $in{'paste'});
    &close_tempfile_as_domain_user($d, CERT);
    &set_permissions_as_domain_user($d, 0755, $chain);
    &unlock_file($chain);
    }

then the function which checks the certificate "check_certificate_data"

file: feature-ssl.pl lines:1353 -1372

sub check_certificate_data
{
local ($data) = @_;
local $temp = &transname();
&open_tempfile(CERTDATA, ">$temp", 0, 1);
&print_tempfile(CERTDATA, $data);
&close_tempfile(CERTDATA);
local $out = &backquote_command("openssl x509 -in ".quotemeta($temp)." -issuer -subject -enddate 2>&1");
local $ex = $?;
&unlink_file($temp);
if ($ex) {
    return "<tt>".&html_escape($out)."</tt>";
    }
elsif ($out !~ /subject\s*=\s*.*(CN|O)=/) {
    return $text{'cert_esubject'};
    }
else {
    return undef;
    }
}

Configuring broken certificate cause Apache to crash on graceful restart - which is the next logical thing to do after installing new certificate.

Status: 
Closed (fixed)

Comments

Submitted by JamieCameron on Tue, 05/01/2018 - 15:11

Yeah, we should really validate each cert in the file separately. I'll fix this in the next Virtualmin release.

Submitted by IssueBot on Thu, 10/11/2018 - 20:07

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.