bug/badcert-7: Fix SSL Certificate Invalid error

- addCertificate now puts a valid key and self-signed certificate in the database to keep the SSLcertificate.pm module happy. - update will cause a rebuild to populate the self-signed cert in the table. - Minor fix to the cron.weekly/letsencrypt install - Added tests. ssl.t demonstrates the iMSCP::OpenSSL validation of a self-signed certificate. letsencrypt_updateSelfSignedCert.t is a simple unit test of the new _updateSelfSignedCert method.
saygoweb · Sep 7, 2017 · ca24f8b · ca24f8b
1 parent d72be8f
commit ca24f8b
Show file tree

Hide file tree

Showing 9 changed files with 270 additions and 60 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # iMSCP LetsEncrypt Plugin - Changelog
 
+## Version 1.1.1
+
+* Fix #7 Invalid Certificate error
+
 ## Version 1.1.0
 
 * Implement #1 Support for aliases and subdomains

diff --git a/TODO b/TODO
@@ -1,15 +1,20 @@
 
-2017-08 V1.1
-- Have a look at the template for domain overview, copy to letsencrypt
-- Populate the view for alias and subdomains
-  - Need to do the getData / query for each type
-- Start to look at the backend
+
+2017-09 v1.1.1
+- Fake cert issues
+
 - Email address in reseller settings
 - Update the tests to test for add certificate
   - Update the database then run
 
 
-2017-08 V1.0
+2017-08 v1.1
+- DONE Have a look at the template for domain overview, copy to letsencrypt
+- DONE Populate the view for alias and subdomains
+  - DONE Need to do the getData / query for each type
+- DONE Start to look at the backend
+
+2017-08 v1.0
 - DONE Make the certbot-auto-test app to mock certbot-auto for dev
   - DONE Make it honor -d
   - DONE Strip www. to allow for more than one -d option typically with www.domain

diff --git a/backend/LetsEncrypt.pm b/backend/LetsEncrypt.pm
@@ -31,6 +31,7 @@ use iMSCP::Dir;
 use iMSCP::EventManager;
 use iMSCP::Execute;
 use iMSCP::File;
+use iMSCP::OpenSSL;
 use iMSCP::Rights;
 use iMSCP::Service;
 use iMSCP::TemplateParser;
@@ -116,23 +117,12 @@ sub update
     $rs ||= $self->_letsencryptConfig( 'configure' );
 
     # Trigger a rebuild on all domains with LetsEncrypt enabled
-    if (version->parse( $fromVersion ) < version->parse( '1.0.1' )) {
-        my $rows = $self->{'db'}->doQuery(
-            'letsencrypt_id',
-            "
-                SELECT letsencrypt_id, domain_id, alias_id, subdomain_id, cert_name, http_forward, status
-                FROM letsencrypt WHERE status IN('ok')
-            "
+    if (version->parse( $fromVersion ) < version->parse( '1.1.1' )) {
+        $self->{'db'}->doQuery(
+            'q',
+            "UPDATE letsencrypt SET status='tochange' WHERE status IN('ok')"
         );
-        unless (ref $rows eq 'HASH') {
-            error( $rows );
-            return 1;
-        }
-
-        my @sql;
-        for(values %{$rows}) {
-            $rs ||= $self->_triggerDomainOnChange($_->{'domain_id'});
-        }
+        $self->run();
     }
 
     return $rs if $rs;
@@ -288,7 +278,9 @@ sub _init
 {
     my $self = shift;
 
-    $self->{'testmode'} = 0;
+    # testmode enables the mocked certbot-auto that creates self-signed certificates
+    # enabled = 1, disabled = 0
+    $self->{'testmode'} = 1;
 
     $self->{'db'} = iMSCP::Database->factory();
     $self->{'httpd'} = Servers::httpd->factory();
@@ -345,46 +337,19 @@ sub _domainTypeAndId
 sub _addCertificate
 {
     my ($self, $type, $id, $certName) = @_;
-    # This action must be idempotent ( this allow to handle 'tochange' status which include key renewal )
+    # This action must be idempotent ( this allows us to handle 'tochange' status which include key renewal )
 
     debug("_addCertificate ");
 
-    # Fake out the SSL_SUPPORT in the Domains module by updating the ssl_certs table and creating a fake cert file
-    my $rs = 0;
-    my $ssl_cert = $self->{'db'}->doQuery(
-        'domain_id', 'SELECT * FROM ssl_certs WHERE domain_type = ? AND domain_id = ?',
-        $type, $id
-    );
-    if (exists $ssl_cert->{$id}) {
-        $rs = $self->{'db'}->doQuery(
-            'u',
-            "UPDATE ssl_certs SET status = 'ok' WHERE domain_type = ? AND domain_id = ? ",
-            $type, $id
-        );
-    } else {
-        $rs = $self->{'db'}->doQuery(
-            'i',
-            "INSERT INTO ssl_certs (status, domain_type, domain_id) VALUES ('ok', ?, ?)",
-            $type, $id
-        );
-    }
-    unless (ref $rs eq 'HASH') {
-        error( $rs );
-        return 1;
-    }
-
-    # TODO This will not work for test domains (I think) CP 2017-09-05
-    # TODO This function can go to the type of the function CP 2017-09-05
     if (!$self->{'testmode'} && !gethostbyname($certName)) {
         error("Cannot resolve $certName");
         return 1;
     }
 
-    # Create the fake cert file, its not valid PEM but that doesn't matter.
-    # TODO Does this have to be before the certbot call? If not, we could put it below and use links. CP 2017-09-05
-    my $certificate = iMSCP::File->new( filename => "$main::imscpConfig{'GUI_ROOT_DIR'}/data/certs/$certName.pem");
-    $certificate->set('LetsEncrypt Dummy Certificate');
-    $certificate->save();
+    # Fake out the SSL_SUPPORT in the Domains module by updating the ssl_certs table and creating a fake cert file
+    my $rs = 0;
+    $rs = $self->_updateSelfSignedCertificate($type, $id);
+    $rs == 0 or return $rs;
 
     my $certNameWWW = 'www.' . $certName;
     my $haveWWW = gethostbyname($certNameWWW) ? 1 : 0;
@@ -410,8 +375,71 @@ sub _addCertificate
     # Trigger an onchange to rebuild the domain, our event listener will then help process the domain config rebuild.
     $self->_triggerDomainOnChange($type, $id);
 
-    # my $rs = $self->_deleteCertificate( $domainId, $aliasId, $domain );
-    # return $rs if $rs;
+    0;
+}
+
+sub _updateSelfSignedCertificate
+{
+    my ($self, $type, $id) = @_;
+    my $rs = 0;
+    my $result = $self->{'db'}->doQuery(
+        'cert_id',
+        'SELECT * FROM ssl_certs WHERE domain_type = ? AND domain_id = ?',
+        $type, $id
+    );
+    my $keyTempFile = File::Temp->new(UNLINK => 1);
+    my $keyFile = iMSCP::File->new(filename => $keyTempFile->filename);
+    my $certTempFile = File::Temp->new(UNLINK => 1);
+    my $certFile = iMSCP::File->new(filename => $certTempFile->filename);
+    if (%{$result}) {
+        my $certId = each %{$result};
+        my $record = $result->{$certId};
+        # If we have a key or certificate check to see if they are valid
+        if ($record->{'private_key'} || $record->{'certificate'}) {
+            $keyFile->set($record->{'private_key'});
+            $keyFile->save();
+            $certFile->set($record->{'certificate'});
+            $certFile->save();
+            my $openSSL = iMSCP::OpenSSL->new(
+                private_key_container_path     => $keyTempFile->filename,
+                certificate_container_path     => $certTempFile->filename
+            );
+            return 0 if $openSSL->validateCertificateChain() == 0;
+        }
+    }
+
+    # Create a new key and self signed certificate
+    my $cmd = [
+        'openssl', 'req', '-x509', '-nodes', '-days', '36500', '-subj', '/CN=test1.local', '-newkey', 'rsa',
+        '-keyout', $keyTempFile,
+        '-out', $certTempFile
+    ];
+    $rs = execute($cmd, \ my $stdout, \ my $stderr);
+    print($stdout . "\n") if $stdout;
+    print($stderr . "\n") if $stderr;
+
+    my $key = $keyFile->get();
+    my $cert = $certFile->get();
+
+    if (%{$result}) {
+        # Update
+        $rs = $self->{'db'}->doQuery(
+            'u',
+            "UPDATE ssl_certs SET status='tochange', private_key=?, certificate=? WHERE domain_type=? AND domain_id=? ",
+            $key, $cert, $type, $id
+        );
+    } else {
+        # Insert
+        $rs = $self->{'db'}->doQuery(
+            'i',
+            "INSERT INTO ssl_certs (status, private_key, certificate, domain_type, domain_id) VALUES ('toadd', ?, ?, ?, ?)",
+            $key, $cert, $type, $id
+        );
+    }
+    unless (ref $rs eq 'HASH') {
+        error($rs);
+        return 1;
+    }
 
     0;
 }
@@ -588,7 +616,7 @@ if [ -f /usr/sbin/csf ]; then
 fi
 EOF
 
-    my $cron = iMSCP::File->new("/etc/cron.weekly/letsencrypt");
+    my $cron = iMSCP::File->new( filename => '/etc/cron.weekly/letsencrypt');
     $cron->set($cronContent);
     $cron->save();
     $cron->mode(0755);

diff --git a/info.php b/info.php
@@ -21,9 +21,9 @@
 return array(
     'author' => 'Cambell Prince',
     'email' => '[email protected]',
-    'version' => '1.1.0',
+    'version' => '1.1.1',
     'require_api' => '1.0.5',
-    'date' => '2017-09-05',
+    'date' => '2017-09-06',
     'name' => 'LetsEncrypt',
     'desc' => 'Plugin that provides LetsEncrypt SSL certificates.',
     'url' => 'https://github.com/saygoweb/imscp-plugin-letsencrypt'

diff --git a/test/backend/all.t b/test/backend/all.t
@@ -0,0 +1,17 @@
+use diagnostics; # this gives you more debugging information
+use warnings;    # this warns you of bad practices
+use strict;      # this prevents silly errors
+
+use TAP::Harness;
+
+my %args = (
+    verbosity => 1,
+    color => 1,
+);
+
+my $harness = TAP::Harness->new (\%args);
+$harness->runtests(
+    # 'install.t',
+    'run.t',
+);
+
diff --git a/test/backend/install.t b/test/backend/install.t
@@ -0,0 +1,25 @@
+use diagnostics; # this gives you more debugging information
+use warnings;    # this warns you of bad practices
+use strict;      # this prevents silly errors
+use Cwd 'abs_path';
+use Test::More qw( no_plan ); # for the is() and isnt() functions
+
+use lib (abs_path('../../backend'), abs_path('../../../../../engine/PerlLib'));
+
+use iMSCP::Bootstrapper;
+
+use LetsEncrypt;
+
+my $bootstrapper = iMSCP::Bootstrapper->getInstance();
+$bootstrapper->getInstance()->boot(
+    {
+        mode            => 'backend',
+        nolock          => 1,
+        norequirements  => 1,
+        config_readonly => 1
+    }
+);
+
+my $plugin = Plugin::LetsEncrypt->getInstance();
+is ($plugin->install(), 0, "install ok");
+ok (-e '/usr/local/bin/certbot-auto', 'certbot-auto exists');
diff --git a/test/backend/letsencrypt_updateSelfSignedCert.t b/test/backend/letsencrypt_updateSelfSignedCert.t
@@ -0,0 +1,54 @@
+use diagnostics; # this gives you more debugging information
+use warnings;    # this warns you of bad practices
+use strict;      # this prevents silly errors
+use Cwd 'abs_path';
+use Test::More qw( no_plan ); # for the is() and isnt() functions
+
+use lib (abs_path('../../backend'), abs_path('../../../../../engine/PerlLib'));
+
+use iMSCP::Bootstrapper;
+
+use LetsEncrypt;
+
+my $bootstrapper = iMSCP::Bootstrapper->getInstance();
+$bootstrapper->getInstance()->boot(
+    {
+        mode            => 'backend',
+        nolock          => 1,
+        norequirements  => 1,
+        config_readonly => 1
+    }
+);
+
+my $rs = 0;
+my $result = 0;
+my $db = iMSCP::Database->factory();
+my $plugin = Plugin::LetsEncrypt->getInstance();
+
+$db->doQuery(
+    'q',
+    'TRUNCATE ssl_certs'
+);
+
+# Should insert a certificate
+$rs = $plugin->_updateSelfSignedCertificate('dmn', 1);
+is($rs, 0, "LetsEncrypt::_updateSelfSignedCertificate No records");
+
+$result = $db->doQuery(
+    'cert_id',
+    'SELECT * FROM ssl_certs'
+);
+is (scalar keys %{$result}, 1, "DB has one record");
+
+# Should pass validation and do no harm
+$rs = $plugin->_updateSelfSignedCertificate('dmn', 1);
+is($rs, 0, "LetsEncrypt::_updateSelfSignedCertificate Existing record");
+
+# Remove the private key and certificate from an exiting record.
+$result = $db->doQuery(
+    'q',
+    "UPDATE ssl_certs SET private_key='',certificate='' WHERE domain_type=? AND domain_id=?",
+    'dmn', 1
+);
+$rs = $plugin->_updateSelfSignedCertificate('dmn', 1);
+is($rs, 0, "LetsEncrypt::_updateSelfSignedCertificate Invalid record");
diff --git a/test/backend/run.t b/test/backend/run.t
@@ -0,0 +1,24 @@
+use diagnostics; # this gives you more debugging information
+use warnings;    # this warns you of bad practices
+use strict;      # this prevents silly errors
+use Cwd 'abs_path';
+use Test::More qw( no_plan ); # for the is() and isnt() functions
+
+use lib (abs_path('../../backend'), abs_path('../../../../../engine/PerlLib'));
+
+use iMSCP::Bootstrapper;
+
+use LetsEncrypt;
+
+my $bootstrapper = iMSCP::Bootstrapper->getInstance();
+$bootstrapper->getInstance()->boot(
+    {
+        mode            => 'backend',
+        nolock          => 1,
+        norequirements  => 1,
+        config_readonly => 1
+    }
+);
+
+my $plugin = Plugin::LetsEncrypt->getInstance();
+is ($plugin->run(), 0, "run ok");