New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Programmatically generate SSL key and certificate #92

Merged
merged 14 commits into from Sep 26, 2018

Conversation

Projects
None yet
5 participants
@ataylorme

ataylorme commented Aug 23, 2018

Description

  • Adds the npm run generate-ssl-cert command to generate an SSL key and certificate
  • Removes the SSL certificate key and path options from dev/config/themeConfig.js
  • Updates the README with instructions on generating an SSL key and certificate with WP Rig
  • Updates the README with instructions on using your own generated SSL key and certificate with WP Rig

Addresses issue #81

List of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • This pull request relates to a ticket.
  • My code is tested.
  • I want my code added to WP Rig.

@ataylorme ataylorme requested review from bamadesigner and mor10 as code owners Aug 23, 2018

@ataylorme ataylorme changed the base branch from master to v1.1 Aug 23, 2018

@ataylorme

This comment has been minimized.

Show comment
Hide comment
@ataylorme

ataylorme Aug 23, 2018

Note you still need to trust the generated key and certificate but they are good for 5 years and this should simplify enabling HTTPS for WP Rig.

@miklb it would be great if you could help test this.

ataylorme commented Aug 23, 2018

Note you still need to trust the generated key and certificate but they are good for 5 years and this should simplify enabling HTTPS for WP Rig.

@miklb it would be great if you could help test this.

@ataylorme

This comment has been minimized.

Show comment
Hide comment
@ataylorme

ataylorme Aug 23, 2018

Breaking change (fix or feature that would cause existing functionality to not work as expected)

If someone used the previous config to specify their own path for a key and certificate they will need to move them to the WP Rig specific paths listed in the README. I'm not sure if this is a breaking change as those options have only been in this v1.1 branch and not officially released.

ataylorme commented Aug 23, 2018

Breaking change (fix or feature that would cause existing functionality to not work as expected)

If someone used the previous config to specify their own path for a key and certificate they will need to move them to the WP Rig specific paths listed in the README. I'm not sure if this is a breaking change as those options have only been in this v1.1 branch and not officially released.

@mor10

This comment has been minimized.

Show comment
Hide comment
@mor10

mor10 Aug 23, 2018

Contributor

Not a breaking change since 1.1 is still pre-release. I will test (and probably merge) tomorrow.

Contributor

mor10 commented Aug 23, 2018

Not a breaking change since 1.1 is still pre-release. I will test (and probably merge) tomorrow.

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Aug 24, 2018

I would still like to use my own local key vs tracking multiples in my local environment.

Edit that should read my own existing key, vs having to move/rename it to work with browser sync in WP Rig.

miklb commented Aug 24, 2018

I would still like to use my own local key vs tracking multiples in my local environment.

Edit that should read my own existing key, vs having to move/rename it to work with browser sync in WP Rig.

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Aug 24, 2018

Unless this is meant strictly for container development

miklb commented Aug 24, 2018

Unless this is meant strictly for container development

@ataylorme

This comment has been minimized.

Show comment
Hide comment
@ataylorme

ataylorme Aug 24, 2018

that should read my own existing key, vs having to move/rename it to work with browser sync in WP Rig.

@miklb that is a good point. Folks can generate a key/cert once and reuse it across projects. I added the option to supply a custom key/cert path added back in.

I also updated the README to encourage a global key/cert across multiple projects.

ataylorme commented Aug 24, 2018

that should read my own existing key, vs having to move/rename it to work with browser sync in WP Rig.

@miklb that is a good point. Folks can generate a key/cert once and reuse it across projects. I added the option to supply a custom key/cert path added back in.

I also updated the README to encourage a global key/cert across multiple projects.

@ataylorme

This comment has been minimized.

Show comment
Hide comment
@ataylorme

ataylorme Aug 24, 2018

I've tested this thoroughly but would like at least one more person to test.

@miklb do you mind testing with your existing key/cert as well as generating them with WP Rig?

ataylorme commented Aug 24, 2018

I've tested this thoroughly but would like at least one more person to test.

@miklb do you mind testing with your existing key/cert as well as generating them with WP Rig?

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Aug 24, 2018

I will be able to test later today

miklb commented Aug 24, 2018

I will be able to test later today

@mor10

This comment has been minimized.

Show comment
Hide comment
@mor10

mor10 Aug 28, 2018

Contributor

@miklb have you had a chance to test it yet?

Contributor

mor10 commented Aug 28, 2018

@miklb have you had a chance to test it yet?

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Aug 28, 2018

Apologies, I have not since it was updated. I will have time this evening.

miklb commented Aug 28, 2018

Apologies, I have not since it was updated. I will have time this evening.

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Sep 7, 2018

Sorry for the long delay, but I thoroughly tested this PR.

Using my own local domain and cert/key pair I was able to load my test install with no issue.

using the defaults for https: true I still get a insecure warning in Firefox.

using without any changes to SSL I am unable to load my site at all.

Now my local dev environment is nginx running locally via homebrew and this was how I set up using .dev locally https://ssl.indieweb.org/ (before the Chrome issue with using .dev came about, but still works)

So I can't speak to full functionality in more vanilla environment, but passing a cert/key that the browser doesn't trust and requires a general exception for localhost still doesn't seem ideal.

miklb commented Sep 7, 2018

Sorry for the long delay, but I thoroughly tested this PR.

Using my own local domain and cert/key pair I was able to load my test install with no issue.

using the defaults for https: true I still get a insecure warning in Firefox.

using without any changes to SSL I am unable to load my site at all.

Now my local dev environment is nginx running locally via homebrew and this was how I set up using .dev locally https://ssl.indieweb.org/ (before the Chrome issue with using .dev came about, but still works)

So I can't speak to full functionality in more vanilla environment, but passing a cert/key that the browser doesn't trust and requires a general exception for localhost still doesn't seem ideal.

@ataylorme

This comment has been minimized.

Show comment
Hide comment
@ataylorme

ataylorme Sep 7, 2018

Now my local dev environment is nginx running locally via homebrew and this was how I set up using .dev locally ssl.indieweb.org (before the Chrome issue with using .dev came about, but still works)

This shouldn't matter as BrowserSync is proxying that to localhost:8181 so localhost is the domain as far as the browser is concerned.

using the defaults for https: true I still get a insecure warning in Firefox.

I think the browsers have gotten stricter since I tested. Chrome now marks my (previously working) localhost proxy site as insecure as does Firefox.

They both seem to dislike that the certificate is self-signed and not sigend by a known certificate authority.

I'm not sure of a way to get around this. Any thoughts on generating a key and certificate that are properly signed are welcome.

ataylorme commented Sep 7, 2018

Now my local dev environment is nginx running locally via homebrew and this was how I set up using .dev locally ssl.indieweb.org (before the Chrome issue with using .dev came about, but still works)

This shouldn't matter as BrowserSync is proxying that to localhost:8181 so localhost is the domain as far as the browser is concerned.

using the defaults for https: true I still get a insecure warning in Firefox.

I think the browsers have gotten stricter since I tested. Chrome now marks my (previously working) localhost proxy site as insecure as does Firefox.

They both seem to dislike that the certificate is self-signed and not sigend by a known certificate authority.

I'm not sure of a way to get around this. Any thoughts on generating a key and certificate that are properly signed are welcome.

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Sep 7, 2018

This shouldn't matter as BrowserSync is proxying that to localhost:8181 so localhost is the domain as far as the browser is concerned.

Actually I think what I did was set it up so all traffic redirects to *.dev and I just add an entry in my nginx.conf file and the dev site is up.

I'm not sure of a way to get around this. Any thoughts on generating a key and certificate that are properly signed are welcome.

I'm going to keep thinking about it because I think it is important for devs to use local https and I'm excited this project is attempting to tackle that.

miklb commented Sep 7, 2018

This shouldn't matter as BrowserSync is proxying that to localhost:8181 so localhost is the domain as far as the browser is concerned.

Actually I think what I did was set it up so all traffic redirects to *.dev and I just add an entry in my nginx.conf file and the dev site is up.

I'm not sure of a way to get around this. Any thoughts on generating a key and certificate that are properly signed are welcome.

I'm going to keep thinking about it because I think it is important for devs to use local https and I'm excited this project is attempting to tackle that.

@ataylorme

This comment has been minimized.

Show comment
Hide comment
@ataylorme

ataylorme Sep 7, 2018

Perhaps @Dexus would be kind enough to chime in and give us some help with pem :-)

ataylorme commented Sep 7, 2018

Perhaps @Dexus would be kind enough to chime in and give us some help with pem :-)

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Sep 7, 2018

Looks like this might be what is needed https://www.npmjs.com/package/create-cert

it creates the ca cert that you can add to the browser and otherwise uses pem.

miklb commented Sep 7, 2018

Looks like this might be what is needed https://www.npmjs.com/package/create-cert

it creates the ca cert that you can add to the browser and otherwise uses pem.

@ataylorme

This comment has been minimized.

Show comment
Hide comment
@ataylorme

ataylorme Sep 8, 2018

That looks interesting but after tinkering/reading I don't see a way to load the CA cert into the proxy server BrowserSync creates.

ataylorme commented Sep 8, 2018

That looks interesting but after tinkering/reading I don't see a way to load the CA cert into the proxy server BrowserSync creates.

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Sep 8, 2018

That ca cert gets added to the browser or keychain on Mac for Safari. That allows the browser to trust the key & cert

miklb commented Sep 8, 2018

That ca cert gets added to the browser or keychain on Mac for Safari. That allows the browser to trust the key & cert

@ataylorme

This comment has been minimized.

Show comment
Hide comment
@ataylorme

ataylorme Sep 8, 2018

Gotcha - should the CA cert file be saved as wp-rig-browser-sync.pem then?

ataylorme commented Sep 8, 2018

Gotcha - should the CA cert file be saved as wp-rig-browser-sync.pem then?

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Sep 8, 2018

So you would run the gulp task to generate keys, import the ca cert then run main gulp task

miklb commented Sep 8, 2018

So you would run the gulp task to generate keys, import the ca cert then run main gulp task

@ataylorme

This comment has been minimized.

Show comment
Hide comment
@ataylorme

ataylorme Sep 8, 2018

The create-cert package returns key, cert and caCert. I have to use Node's file system to save them. I know that key is .key, cert is .crt but I am not sure what extension I need to save the CA cert as.

ataylorme commented Sep 8, 2018

The create-cert package returns key, cert and caCert. I have to use Node's file system to save them. I know that key is .key, cert is .crt but I am not sure what extension I need to save the CA cert as.

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Sep 8, 2018

the CA cert is also .cert just need to make sure the two are named appropriately that we can document. Something like wprig_root_cert.crt

I already have screenshots from how to add to Firefox for instance.

miklb commented Sep 8, 2018

the CA cert is also .cert just need to make sure the two are named appropriately that we can document. Something like wprig_root_cert.crt

I already have screenshots from how to add to Firefox for instance.

@ataylorme

This comment has been minimized.

Show comment
Hide comment
@ataylorme

ataylorme Sep 8, 2018

I updated to use create-cert and added both the CA cert and regular cert to my KeyChain but get This certificate has not been verified by a third-party in Chrome and The certificate is not trusted because it is self-signed in FireFox.

@miklb want me to commit what I have? Maybe you can get it going. I am not an SSL expert and am beyond my expertise with keys/certs/CA signing.

ataylorme commented Sep 8, 2018

I updated to use create-cert and added both the CA cert and regular cert to my KeyChain but get This certificate has not been verified by a third-party in Chrome and The certificate is not trusted because it is self-signed in FireFox.

@miklb want me to commit what I have? Maybe you can get it going. I am not an SSL expert and am beyond my expertise with keys/certs/CA signing.

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Sep 8, 2018

@ataylorme sounds good to me.

miklb commented Sep 8, 2018

@ataylorme sounds good to me.

@ataylorme

This comment has been minimized.

Show comment
Hide comment
@ataylorme

ataylorme Sep 25, 2018

Can you fix the merge conflict?

Done

Im testing now

@bamadesigner can you update the README (and maybe take screenshots) as you test? I think instructions on adding the certs to browsers is what is missing.

ataylorme commented Sep 25, 2018

Can you fix the merge conflict?

Done

Im testing now

@bamadesigner can you update the README (and maybe take screenshots) as you test? I think instructions on adding the certs to browsers is what is missing.

@@ -22,7 +22,8 @@
"babel-preset-env": "^1.7.0",
"babel-preset-es2015": "^6.24.1",
"babel-register": "^6.26.0",
"browser-sync": "^2.23.6",
"browser-sync": "^2.24.7",
"create-cert": "^1.0.6",

This comment has been minimized.

@bamadesigner

bamadesigner Sep 25, 2018

Contributor

I think we're missing "generate-ssl-cert": "gulp generateCert", under the scripts section.

And maybe we can change it to just be generateCert so: "generateCert": "gulp generateCert",

@bamadesigner

bamadesigner Sep 25, 2018

Contributor

I think we're missing "generate-ssl-cert": "gulp generateCert", under the scripts section.

And maybe we can change it to just be generateCert so: "generateCert": "gulp generateCert",

This comment has been minimized.

@bamadesigner

bamadesigner Sep 25, 2018

Contributor

So then the instructions have to be changed to run npm run generateCert.

@bamadesigner

bamadesigner Sep 25, 2018

Contributor

So then the instructions have to be changed to run npm run generateCert.

This comment has been minimized.

@ataylorme

ataylorme Sep 25, 2018

Good catch, I missed the genrateCert command being lost in resolving the merge conflict. I added it back with the new name generateCert and updated the README.

@ataylorme

ataylorme Sep 25, 2018

Good catch, I missed the genrateCert command being lost in resolving the merge conflict. I added it back with the new name generateCert and updated the README.

This comment has been minimized.

@bamadesigner

bamadesigner Sep 25, 2018

Contributor

Thanks!

@bamadesigner

bamadesigner Sep 25, 2018

Contributor

Thanks!

ataylorme and others added some commits Sep 25, 2018

Show outdated Hide outdated README.md Outdated
@bamadesigner

This comment has been minimized.

Show comment
Hide comment
@bamadesigner

bamadesigner Sep 25, 2018

Contributor

@ataylorme let me know if you're ok with 2d18f63

Contributor

bamadesigner commented Sep 25, 2018

@ataylorme let me know if you're ok with 2d18f63

@ataylorme

This comment has been minimized.

Show comment
Hide comment
@ataylorme

ataylorme Sep 25, 2018

👍 looks good to me @bamadesigner. Were you able to get the green lock without warnings in Chrome and Firefox? I haven't been able to do so with only trusting the certs in Keychain Access and not the browser.

ataylorme commented Sep 25, 2018

👍 looks good to me @bamadesigner. Were you able to get the green lock without warnings in Chrome and Firefox? I haven't been able to do so with only trusting the certs in Keychain Access and not the browser.

@bamadesigner

This comment has been minimized.

Show comment
Hide comment
@bamadesigner

bamadesigner Sep 25, 2018

Contributor

@ataylorme I was able to do so in Chrome and Safari. Not Firefox.

Contributor

bamadesigner commented Sep 25, 2018

@ataylorme I was able to do so in Chrome and Safari. Not Firefox.

@bamadesigner

This comment has been minimized.

Show comment
Hide comment
@bamadesigner

bamadesigner Sep 25, 2018

Contributor

@ataylorme I'm going to merge and then I'll go in and add a few screenshots. It's easier in their editor.

Contributor

bamadesigner commented Sep 25, 2018

@ataylorme I'm going to merge and then I'll go in and add a few screenshots. It's easier in their editor.

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Sep 25, 2018

I can provide steps for adding to Firefox, I tested with FF and it was good to go.

miklb commented Sep 25, 2018

I can provide steps for adding to Firefox, I tested with FF and it was good to go.

@bamadesigner

This comment has been minimized.

Show comment
Hide comment
@bamadesigner

bamadesigner Sep 25, 2018

Contributor

@miklb thanks! If you share them here, I can add them to the readme.

Contributor

bamadesigner commented Sep 25, 2018

@miklb thanks! If you share them here, I can add them to the readme.

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Sep 25, 2018

As soon as I’m back at the computer I’ll upload screenshots & paths to the settings

miklb commented Sep 25, 2018

As soon as I’m back at the computer I’ll upload screenshots & paths to the settings

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Sep 26, 2018

Apologies for not posting these sooner, I had mentioned offering help with the docs and then got sidetracked.
firefox_add_cert_1
firefox_add_cert_2
firefox_add_cert_wprig_3

I didn't add alt tags but text version -

navigate to about:preferences in Firefox, scroll down to bottom of page under Certificates -> View Certificates

First, make sure the Authorize tab is active in the top nav bar. Second, click the middle bottom row Import -> navigate to directory of the of the certificates (wprig defaults to /BrowserSync select the root certificate wp-rig-browser-sync-root-cert.crt and import. If you are unable to select the certificate file, click the options tab to make sure Certificate file type is selected. (Note this last step doesn't seem to be an issue in Firefox Dev Edition I'm currently running, but was necessary a short time back in Firefox Nightly.)

miklb commented Sep 26, 2018

Apologies for not posting these sooner, I had mentioned offering help with the docs and then got sidetracked.
firefox_add_cert_1
firefox_add_cert_2
firefox_add_cert_wprig_3

I didn't add alt tags but text version -

navigate to about:preferences in Firefox, scroll down to bottom of page under Certificates -> View Certificates

First, make sure the Authorize tab is active in the top nav bar. Second, click the middle bottom row Import -> navigate to directory of the of the certificates (wprig defaults to /BrowserSync select the root certificate wp-rig-browser-sync-root-cert.crt and import. If you are unable to select the certificate file, click the options tab to make sure Certificate file type is selected. (Note this last step doesn't seem to be an issue in Firefox Dev Edition I'm currently running, but was necessary a short time back in Firefox Nightly.)

@bamadesigner

This comment has been minimized.

Show comment
Hide comment
@bamadesigner

bamadesigner Sep 26, 2018

Contributor

Thank you @miklb! I hate to say this after you did all that work but this seems like it might be too much/too granular for including in our readme. Thoughts? @ataylorme? I wonder if there are some instructions we can link to instead?

Contributor

bamadesigner commented Sep 26, 2018

Thank you @miklb! I hate to say this after you did all that work but this seems like it might be too much/too granular for including in our readme. Thoughts? @ataylorme? I wonder if there are some instructions we can link to instead?

@ataylorme

This comment has been minimized.

Show comment
Hide comment
@ataylorme

ataylorme Sep 26, 2018

We could move all the HTTPS instructors to a wiki page here on GitHub and link to it from the README. That would condense the README and allow us to go into more depth explaination with screenshots.

My only concern is maintaining such a page as the instructions will surely change as browsers update. We’re already seeing discrepancies in the dev edition vs nightly of Firefox.

ataylorme commented Sep 26, 2018

We could move all the HTTPS instructors to a wiki page here on GitHub and link to it from the README. That would condense the README and allow us to go into more depth explaination with screenshots.

My only concern is maintaining such a page as the instructions will surely change as browsers update. We’re already seeing discrepancies in the dev edition vs nightly of Firefox.

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Sep 26, 2018

Wiki page sounds good & a condensed version in readme sounds fine. I just wanted to be thorough. I documented those screenshots from own experience with my own local cert & had failed to share previously.

miklb commented Sep 26, 2018

Wiki page sounds good & a condensed version in readme sounds fine. I just wanted to be thorough. I documented those screenshots from own experience with my own local cert & had failed to share previously.

@mor10

This comment has been minimized.

Show comment
Hide comment
@mor10

mor10 Sep 26, 2018

Contributor

I like the Wiki page idea. Readme is starting to get a tad overloaded

Contributor

mor10 commented Sep 26, 2018

I like the Wiki page idea. Readme is starting to get a tad overloaded

@miklb

This comment has been minimized.

Show comment
Hide comment
@miklb

miklb Sep 26, 2018

I’m really glad a solution was found & will ship in a future version. Thank you @ataylorme for all of the work on this.

miklb commented Sep 26, 2018

I’m really glad a solution was found & will ship in a future version. Thank you @ataylorme for all of the work on this.

@bamadesigner

This comment has been minimized.

Show comment
Hide comment
@bamadesigner

bamadesigner Sep 26, 2018

Contributor

Oh yea! I always forget about the wiki. I'll add a mention about possible Firefox issues to the readme and put the Firefox instructions in the wiki.

Thanks for being so thorough with your notes @miklb. And to @ataylorme for all the hard work.

Contributor

bamadesigner commented Sep 26, 2018

Oh yea! I always forget about the wiki. I'll add a mention about possible Firefox issues to the readme and put the Firefox instructions in the wiki.

Thanks for being so thorough with your notes @miklb. And to @ataylorme for all the hard work.

@bamadesigner

This comment has been minimized.

Show comment
Hide comment
@bamadesigner

bamadesigner Sep 26, 2018

Contributor

I added all of the BrowserSync info to a wiki page: https://github.com/wprig/wprig/wiki/BrowserSync

And updated the readme to link, instead, to the wiki. I made note in the wiki that the generate certificate feature is still under development.

I'm going to merge. Let me know if anything is amiss.

Thank you for all of the hard work!

Contributor

bamadesigner commented Sep 26, 2018

I added all of the BrowserSync info to a wiki page: https://github.com/wprig/wprig/wiki/BrowserSync

And updated the readme to link, instead, to the wiki. I made note in the wiki that the generate certificate feature is still under development.

I'm going to merge. Let me know if anything is amiss.

Thank you for all of the hard work!

@bamadesigner bamadesigner merged commit 472c166 into wprig:v1.1 Sep 26, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ataylorme

This comment has been minimized.

Show comment
Hide comment
@ataylorme

ataylorme Sep 26, 2018

Thank you @miklb and @bamadesigner for the documentation and testing - this was a team effort!

ataylorme commented Sep 26, 2018

Thank you @miklb and @bamadesigner for the documentation and testing - this was a team effort!

@ataylorme ataylorme deleted the ataylorme:issue-81-generate-ssl-cert branch Oct 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment