Sometimes, if something is wrong with the SSL configuration, apache can break because the configuration is pointing to a file that doesn't exist.

This will happen if you try to enable letsencrypt locally, for example. Provision will still write the config even if the task fails, and apache will not start because the files don't exist.

In platforms.tpl.php, we check if .htaccess is readable before writing it to the template. I think we should do the same thing with the certs.

CommentFileSizeAuthor
#16 3020747-https-check-cert-readable-provision.patch4.33 KBJon Pugh
#14 3020747-https-check-cert-readable-https.patch1.98 KBJon Pugh
#3 3020747-https-check-cert-readable-provision.patch685 bytesJon Pugh
#3 3020747-https-check-cert-readable-https.patch752 bytesJon Pugh
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jon Pugh created an issue. See original summary.

  • Jon Pugh committed 4b19a6c on 3020747-https-check-cert-readable 
    Issue #3020747: Don't add SSL config to configuration files if the crt...
Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location Newburgh, NY
CreditAttribution: Jon Pugh at DevShop Support commented
Status: Active » Needs review
FileSize
3020747-https-check-cert-readable-https.patch752 bytes
3020747-https-check-cert-readable-provision.patch685 bytes
Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location Newburgh, NY
CreditAttribution: Jon Pugh at DevShop Support commented
Title: Don't add SSL config to configuration files if the crt files aren't there/aren't readable. » Don't add SSL config to configuration files if the crt files aren't there/aren't readable. (especially redirects)
colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
CreditAttribution: colan at Consensus Enterprises, Colan Schwartz Consulting commented
Status: Needs review » Reviewed & tested by the community

  • Jon Pugh committed 4b19a6c on 7.x-3.x 
    Issue #3020747: Don't add SSL config to configuration files if the crt...
helmo’s picture

helmo CreditAttribution: helmo as a volunteer commented
Status: Reviewed & tested by the community » Fixed

Merged

bgm’s picture

bgm CreditAttribution: bgm commented
Status: Fixed » Needs work

This caused a regression for me. The tpl $this->https_enabled was set to false, but the LE cert had been correctly generated.

Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location Newburgh, NY
CreditAttribution: Jon Pugh at DevShop Support commented
Priority: Normal » Major

My fault.

I'll be able to get this fixed in the next hour.

bgm’s picture

bgm CreditAttribution: bgm commented

For those looking for a quickfix, edit:

/var/aegir/hostmaster-7.x-3.170/profiles/hostmaster/modules/aegir/hosting_https/drush/Provision/Service/http/https.php

find and comment out this code starting at line 74:

      // Turn off https and redirection if the cert key is not readable or doesn't exist for some reason.
      # if (!is_readable($data['https_cert_key'])) {
        # $this->context->https_enabled = FALSE;
        # $data['ssl_redirection'] = FALSE;
      # }
Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location Newburgh, NY
CreditAttribution: Jon Pugh at DevShop Support commented

Ok. The file doesn't even exist at this point.

I think I have an alternative. Stand by.

Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location Newburgh, NY
CreditAttribution: Jon Pugh at DevShop Support commented

Frustrated to discover #2955062: Do not let nginx configuration reference missing cert files. This was dealt with in NGINIX back in March.

Please lets make sure to apply changes like this to all of the hosting plugins? This whole thing would have been avoided.

  • Jon Pugh committed 306ccd5 on 3020747-https-check-cert-readable 
    Issue #3020747: Revert disabling of https here.
Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location Newburgh, NY
CreditAttribution: Jon Pugh at DevShop Support commented
FileSize
3020747-https-check-cert-readable-https.patch1.98 KB
bgm’s picture

bgm CreditAttribution: bgm commented

I tested creating a site, then enabled https = required, and it worked as expected.

However, I don't have Apache servers for testing the behaviour when the LE cert failed.

Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location Newburgh, NY
CreditAttribution: Jon Pugh at DevShop Support commented
FileSize
3020747-https-check-cert-readable-provision.patch4.33 KB

Patch for Provision SSL attached.

I fully tested this one.

Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location Newburgh, NY
CreditAttribution: Jon Pugh at DevShop Support commented
Status: Needs work » Reviewed & tested by the community

bgm: I do and it worked, so I'm marking it RTBC.

  • Jon Pugh committed 306ccd5 on 7.x-3.x 
    Issue #3020747: Revert disabling of https here.
Jon Pugh’s picture

Jon Pugh
he/him
Primary language English
Location Newburgh, NY
CreditAttribution: Jon Pugh at DevShop Support commented
Status: Reviewed & tested by the community » Fixed

Merged

Status: Fixed » Closed (fixed)

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