ISPConfig installer should detect, recognize and use existing SSL certs for the server if there are already created and should not even attempt to create other SSL certs, LE or SS. As quoted below, my.server.tld SSL certs already detected, recognized and the server was already said to be using it, but then, it still proceed with creating the LE certs using webroot method but it failed of course because I was behind NAT and SS SSL certs was created instead.
Ok. As I said in the other thread (Installing ISPConfig 3.29 development), the code, which is in installer_base.lib.php seems to be in there, at line 2992: Code: if ((@file_exists($ssl_crt_file) && ($crt_subject == $crt_issuer)) || (!@is_dir($acme_cert_dir) || !@file_exists($check_acme_file) || !@file_exists($ssl_crt_file) || md5_file($check_acme_file) != md5_file($ssl_crt_file)) && $ip_address_match == true) { ... // Attempt to use Neilpang acme.sh first, as it is now the preferred LE client if (is_executable($acme)) { ... acme.sh LE method ... // Else, we attempt to use the official LE certbot client certbot } else { // But only if it is otherwise available if(is_executable($le_client)) { ... certbot LE method ... } } } if(!is_dir($acme_cert_dir) || !file_exists($check_acme_file) || !$issued_successfully) { if(!$issued_successfully) { swriteln('Could not issue letsencrypt certificate, falling back to self-signed.'); } else { swriteln('Issuing certificate seems to have succeeded but ' . $check_acme_file . ' seems to be missing. Falling back to self-signed.'); } ... openssl self-signed method ... } Both $acme_cert_dir and $check_acme_file were defined in earlier lines from 2959: Code: $acme_cert_dir = '/usr/local/ispconfig/server/scripts/' . $hostname; $check_acme_file = $acme_cert_dir . '/' . $hostname . '.cer'; if(!@is_dir($acme_cert_dir)) { $acme_cert_dir = '/root/.acme.sh/' . $hostname; $check_acme_file = $acme_cert_dir . '/' . $hostname . '.cer'; if(!@is_dir($acme_cert_dir)) { $acme_cert_dir = '/etc/letsencrypt/live/' . $hostname; $check_acme_file = $acme_cert_dir . '/cert.pem'; } } Based on the output, $acme_cert_dir directory already found, that also suggests $check_acme_file file was found too, which should, supposedly, stop any execution of LE clients and openssl but it did not. To my note these two codes fail to filter accordingly: For both LE clients, the first one: Code: if ((@file_exists($ssl_crt_file) && ($crt_subject == $crt_issuer)) || (!@is_dir($acme_cert_dir) || !@file_exists($check_acme_file) || !@file_exists($ssl_crt_file) || md5_file($check_acme_file) != md5_file($ssl_crt_file)) && $ip_address_match == true) For OpenSSL, the second one: Code: if(!is_dir($acme_cert_dir) || !file_exists($check_acme_file) || !$issued_successfully) For the later, it was $issued_successfully code problem, but I am still not sure about the first one yet and I will investigate later.
Code: if ((@file_exists($ssl_crt_file) && ($crt_subject == $crt_issuer)) || (!@is_dir($acme_cert_dir) || !@file_exists($check_acme_file) || !@file_exists($ssl_crt_file) || md5_file($check_acme_file) != md5_file($ssl_crt_file)) && $ip_address_match == true){ After several testing I only found that this code only has minor problems, but not really related to mine. On the last part of it, of which both files compared need to exist, before comparison is being made, so I'll suggest it to be improvised to: Code: if ((@file_exists($ssl_crt_file) && ($crt_subject == $crt_issuer)) || (!@is_dir($acme_cert_dir) || !@file_exists($check_acme_file) || !@file_exists($ssl_crt_file) || (@file_exists($ssl_crt_file) && @file_exists($check_acme_file) && md5_file($check_acme_file) != md5_file($ssl_crt_file))) && $ip_address_match == true) { Furthermore, I don't think it is in the right place nor do I think we need the whole code as it is or as suggested above. The original is: Code: if ((@file_exists($ssl_crt_file) && ($crt_subject == $crt_issuer)) || (!@is_dir($acme_cert_dir) || !@file_exists($check_acme_file) || !@file_exists($ssl_crt_file) || md5_file($check_acme_file) != md5_file($ssl_crt_file)) && $ip_address_match == true) { // This script is needed earlier to check and open http port 80 or standalone might fail // Make executable and temporary symlink latest letsencrypt pre, post and renew hook script before install if(file_exists(ISPC_INSTALL_ROOT . '/server/scripts/letsencrypt_pre_hook.sh') && !file_exists('/usr/local/bin/letsencrypt_pre_hook.sh')) { if(is_link('/usr/local/bin/letsencrypt_pre_hook.sh')) { unlink('/usr/local/bin/letsencrypt_pre_hook.sh'); } symlink(ISPC_INSTALL_ROOT . '/server/scripts/letsencrypt_pre_hook.sh', '/usr/local/bin/letsencrypt_pre_hook.sh'); chown('/usr/local/bin/letsencrypt_pre_hook.sh', 'root'); chmod('/usr/local/bin/letsencrypt_pre_hook.sh', 0700); } if(file_exists(ISPC_INSTALL_ROOT . '/server/scripts/letsencrypt_post_hook.sh') && !file_exists('/usr/local/bin/letsencrypt_post_hook.sh')) { if(is_link('/usr/local/bin/letsencrypt_post_hook.sh')) { unlink('/usr/local/bin/letsencrypt_post_hook.sh'); } symlink(ISPC_INSTALL_ROOT . '/server/scripts/letsencrypt_post_hook.sh', '/usr/local/bin/letsencrypt_post_hook.sh'); chown('/usr/local/bin/letsencrypt_post_hook.sh', 'root'); chmod('/usr/local/bin/letsencrypt_post_hook.sh', 0700); } if(file_exists(ISPC_INSTALL_ROOT . '/server/scripts/letsencrypt_renew_hook.sh') && !file_exists('/usr/local/bin/letsencrypt_renew_hook.sh')) { if(is_link('/usr/local/bin/letsencrypt_renew_hook.sh')) { unlink('/usr/local/bin/letsencrypt_renew_hook.sh'); } symlink(ISPC_INSTALL_ROOT . '/server/scripts/letsencrypt_renew_hook.sh', '/usr/local/bin/letsencrypt_renew_hook.sh'); chown('/usr/local/bin/letsencrypt_renew_hook.sh', 'root'); chmod('/usr/local/bin/letsencrypt_renew_hook.sh', 0700); } // Check http port 80 status as it cannot be determined at post hook stage $port80_status=exec('true &>/dev/null </dev/tcp/127.0.0.1/80 && echo open || echo close'); // Set pre-, post- and renew hook $pre_hook = "--pre-hook \"letsencrypt_pre_hook.sh\""; $renew_hook = " --renew-hook \"letsencrypt_renew_hook.sh\""; if($port80_status == 'close') { $post_hook = " --post-hook \"letsencrypt_post_hook.sh\""; $hook = $pre_hook . $post_hook . $renew_hook; } else { $hook = $pre_hook . $renew_hook; } $which_certbot = shell_exec('which certbot /root/.local/share/letsencrypt/bin/letsencrypt /opt/eff.org/certbot/venv/bin/certbot letsencrypt'); // Get the default LE client name and version $le_client = explode("\n", $which_certbot ? $which_certbot : ''); $le_client = reset($le_client); $which_acme = shell_exec('which acme.sh /usr/local/ispconfig/server/scripts/acme.sh /root/.acme.sh/acme.sh'); // Check for Neilpang acme.sh as well $acme = explode("\n", $which_acme ? $which_acme : ''); $acme = reset($acme); if((!$acme || !is_executable($acme)) && (!$le_client || !is_executable($le_client))) { $success = $this->install_acme(); if(!$success) { swriteln('Failed installing acme.sh. Will not be able to issue certificate during install.'); } else { $acme = explode("\n", shell_exec('which acme.sh /usr/local/ispconfig/server/scripts/acme.sh /root/.acme.sh/acme.sh')); $acme = reset($acme); if($acme && is_executable($acme)) { swriteln('Installed acme.sh and using it for certificate creation during install.'); // we do this even on install to enable automatic updates $this->update_acme(); } else { swriteln('Failed installing acme.sh. Will not be able to issue certificate during install.'); } } } $restore_conf_symlink = false; // we only need this for apache, so use fixed conf index $vhost_conf_dir = $conf['apache']['vhost_conf_dir']; $vhost_conf_enabled_dir = $conf['apache']['vhost_conf_enabled_dir']; // first of all create the acme vhosts if not existing if($conf['nginx']['installed'] == true) { swriteln('Using nginx for certificate validation'); $server = 'nginx'; } elseif($conf['apache']['installed'] == true) { swriteln('Using apache for certificate validation'); if($this->is_update == false && @is_link($vhost_conf_enabled_dir.'/000-ispconfig.conf')) { $restore_conf_symlink = true; unlink($vhost_conf_enabled_dir.'/000-ispconfig.conf'); } $server = 'apache'; } if($conf[$server]['installed'] == true && $conf[$server]['init_script'] != '') { if($this->is_update) { system($this->getinitcommand($conf[$server]['init_script'], 'force-reload').' &> /dev/null || ' . $this->getinitcommand($conf[$server]['init_script'], 'restart').' &> /dev/null'); } else { system($this->getinitcommand($conf[$server]['init_script'], 'restart').' &> /dev/null'); } } $issued_successfully = false; // Backup existing ispserver ssl files // // We may find valid or broken symlinks or actual files here. // // - dangling links are broken and get perm renamed (should just delete?). // possibly web server can't start because vhost file points to non-existing cert files, // we're not trying to catch or fix that (and not making it worse) // // - link to valid file is tmp renamed, and file copied to original name. // if cert request is successful, remove the old symlink; // if cert request fails, remove file copy and rename symlink to original name // // - actual file copied to tmp name. // if cert request is successful, rename tmp copy to perm rename; // if cert request fails, delete tmp copy $cert_files = array( $ssl_crt_file, $ssl_key_file, $ssl_pem_file ); foreach ($cert_files as $f) { if (is_link($f) && ! file_exists($f)) { rename($f, $f.'-'.$date->format('YmdHis').'.bak'); } elseif (is_link($f)) { rename($f, $f.'-temporary.bak'); copy($f.'-temporary.bak', $f); } elseif(file_exists($f)) { copy($f, $f.'-temporary.bak'); } } // Attempt to use Neilpang acme.sh first, as it is now the preferred LE client if (is_executable($acme)) { I believe that it should be just above "// Backup existing ispserver ssl files" as all codes before that seem necessary, thus all them should be processed whether or not any SSL certs already existed / created. Plus the whole logic of each every condition should be re-examined properly before any of them is to be modified. However, the above is not the main problem that I face when I opened this thread though I think it is related. The main problem lies in here from line 3119: Code: // Attempt to use Neilpang acme.sh first, as it is now the preferred LE client if (is_executable($acme)) { $acme_cert_dir = dirname($acme) . '/' . $hostname; swriteln('acme.sh is installed, overriding certificate path to use ' . $acme_cert_dir); # acme.sh does not set umask, resulting in incorrect permissions (ispconfig issue #6015) $old_umask = umask(0022); // Switch from zerossl to letsencrypt CA exec("$acme --set-default-ca --server letsencrypt"); $out = null; $ret = null; if($conf['nginx']['installed'] == true || $conf['apache']['installed'] == true) { exec("$acme --issue --log $acme_log -w /usr/local/ispconfig/interface/acme -d " . escapeshellarg($hostname) . " $renew_hook", $out, $ret); } // Else, it is not webserver, so we use standalone else { exec("$acme --issue --log $acme_log --standalone -d " . escapeshellarg($hostname) . " $hook", $out, $ret); } if($ret == 0 || ($ret == 2 && file_exists($check_acme_file))) { // acme.sh returns with 2 on issue for already existing certificate $check_acme_file = $ssl_crt_file; // Define LE certs name and path, then install them //$acme_cert = "--cert-file $acme_cert_dir/cert.pem"; $acme_key = "--key-file " . escapeshellarg($ssl_key_file); $acme_chain = "--fullchain-file " . escapeshellarg($ssl_crt_file); exec("$acme --install-cert --log $acme_log -d " . escapeshellarg($hostname) . " $acme_key $acme_chain"); $issued_successfully = true; umask($old_umask); // Make temporary backup of self-signed certs permanent foreach ($cert_files as $f) { if (is_link($f.'-temporary.bak')) { unlink($f.'-temporary.bak'); } elseif(file_exists($f.'-temporary.bak')) { rename($f.'-temporary.bak', $f.'-'.$date->format('YmdHis').'.bak'); } } } else { swriteln('Issuing certificate via acme.sh failed. Please check that your hostname can be verified by letsencrypt'); umask($old_umask); // Restore/cleanup temporary backup of self-signed certs foreach ($cert_files as $f) { if (is_link($f.'-temporary.bak')) { @unlink($f); rename($f.'-temporary.bak', $f); } elseif(file_exists($f.'-temporary.bak')) { unlink($f.'-temporary.bak'); } } } // Else, we attempt to use the official LE certbot client certbot } else { // But only if it is otherwise available if(is_executable($le_client)) { $out = null; $ret = null; // Get its version info due to be used for webroot arguement issues $le_info = exec($le_client . ' --version 2>&1', $ret, $val); if(preg_match('/^(\S+|\w+)\s+(\d+(\.\d+)+)$/', $le_info, $matches)) { $le_version = $matches[2]; } // Define certbot commands $acme_version = '--server https://acme-v0' . (($le_version >=0.22) ? '2' : '1') . '.api.letsencrypt.org/directory'; $certonly = 'certonly --agree-tos --non-interactive --expand --rsa-key-size 4096'; if((!@is_dir($acme_cert_dir) || !@file_exists($check_acme_file)) && $ip_address_match == true) { // If this is a webserver if($conf['nginx']['installed'] == true || $conf['apache']['installed'] == true) { exec("$le_client $certonly $acme_version --authenticator webroot --webroot-path /usr/local/ispconfig/interface/acme --email " . escapeshellarg('postmaster@' . $hostname) . " -d " . escapeshellarg($hostname) . " $renew_hook", $out, $ret); } // Else, it is not webserver, so we use standalone else { exec("$le_client $certonly $acme_version --standalone --email " . escapeshellarg('postmaster@' . $hostname) . " -d " . escapeshellarg($hostname) . " $hook", $out, $ret); } } if($ret == 0 || @is_dir($acme_cert_dir) || @file_exists($check_acme_file)) { // certbot returns with 0 on issue for already existing certificate $acme_cert_dir = '/etc/letsencrypt/live/' . $hostname; foreach (array( $ssl_crt_file, $ssl_key_file) as $f) { if (file_exists($f) && ! is_link($f)) { unlink($f); } } symlink($acme_cert_dir . '/fullchain.pem', $ssl_crt_file); symlink($acme_cert_dir . '/privkey.pem', $ssl_key_file); $issued_successfully = true; // Make temporary backup of self-signed certs permanent foreach ($cert_files as $f) { if (is_link($f.'-temporary.bak')) { unlink($f.'-temporary.bak'); } elseif(file_exists($f.'-temporary.bak')) { rename($f.'-temporary.bak', $f.'-'.$date->format('YmdHis').'.bak'); } } } else { swriteln('Issuing certificate via certbot failed. Please check log files and make sure that your hostname can be verified by letsencrypt'); // Restore/cleanup temporary backup of self-signed certs foreach ($cert_files as $f) { if (is_link($f.'-temporary.bak')) { @unlink($f); rename($f.'-temporary.bak', $f); } elseif(file_exists($f.'-temporary.bak')) { unlink($f.'-temporary.bak'); } } } } else { swriteln('Did not find any valid acme client (acme.sh or certbot)'); } } if($restore_conf_symlink) { if(!@is_link($vhost_conf_enabled_dir.'/000-ispconfig.conf')) { symlink($vhost_conf_dir.'/ispconfig.conf', $vhost_conf_enabled_dir.'/000-ispconfig.conf'); } } } else { if($ip_address_match) { // the directory already exists so we have to assume that it was created previously $issued_successfully = true; } } It is specifically mentioned in line 3142 that acme.sh returns with 2 on issue for already existing certificate, but certbot does not have the same feature so it will only return 0. Due to this, I suggest some codes from line 3196 be modified as follows: Code: if((!@is_dir($acme_cert_dir) || !@file_exists($check_acme_file)) && $ip_address_match == true) { // If this is a webserver if($conf['nginx']['installed'] == true || $conf['apache']['installed'] == true) { exec("$le_client $certonly $acme_version --authenticator webroot --webroot-path /usr/local/ispconfig/interface/acme --email " . escapeshellarg('postmaster@' . $hostname) . " -d " . escapeshellarg($hostname) . " $renew_hook", $out, $ret); } // Else, it is not webserver, so we use standalone else { exec("$le_client $certonly $acme_version --standalone --email " . escapeshellarg('postmaster@' . $hostname) . " -d " . escapeshellarg($hostname) . " $hook", $out, $ret); } } if($ret == 0 || @is_dir($acme_cert_dir) || @file_exists($check_acme_file)) { This modification added a check before certbot webroot or standalone is executed, thus preventing unnecessary execution. It also allows the server to used already issued LE certs for the server and processed them accordingly. This also means $issued_successfully that I mentioned earlier was not the real problem.
Could you open an issue in the git system and do a merge request? I think we should just address the specific issue you localized in the last past of your post, as the current code works generally fine now for most users and I did not have any issues on any of my installations in 3.2.8, We had many issues in the past with the LE code, so we should not open up new cans of worms and only modify what's really needed.
