Skip to content
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

Implements #8490 - "pfSense-pkg-acme: acme_certificates_edit.php - Ad… #518

Closed

Conversation

mr-intj
Copy link
Contributor

@mr-intj mr-intj commented May 2, 2018

Implements #8490 - "pfSense-pkg-acme: acme_certificates_edit.php - Add ability to specify (vs generate) private key"

This is required to allow certificates to work in environments where HPKP is deployed across subdomains.

UI (Services / Acme / Certificates / Edit Certificate Options):

  • Added "Custom..." option at the end of the Key Size drop-down.

  • Renamed this field from "Key Size" to "Private Key", since it now contains both generate and paste-custom options.

  • Selecting "Custom..." from the list adds (makes visible) a "Custom Private Key" textarea field in the next section.

  • Selecting any other of the other (key-size) options from the Private Key drop-down hides the "Custom Private Key" field again.

  • Key sizes in the drop-down have been renamed for clarity, e.g. "2048" => "2048-bit RSA", "ec-384" => "384-bit ECDSA", etc.

  • Fixed a few typos/misspellings in the files I worked on (e.g. "atempting" => "attempting")

Back End:

  • Like other user-entered X.509 PEM formatted data, the user-entered Custom Private Key text is stored base64_encoded in the configuration.

  • The private-key-generation logic is skipped when the user pastes in their private key.

  • When visible, "Custom Private Key" is a required field, and is also verified to contain "-----BEGIN PRIVATE KEY-----" and "-----END PRIVATE KEY-----" strings.

Additional Notes:

  • security/pfSense-pkg-acme/Makefile

    • I bumped the version number from 0.2.8 to 0.2.9. If you're using semver (i.e. last digit is bug fixes), then that should probably be 0.3.0.
  • security/pfSense-pkg-acme/files/usr/local/pkg/acme/acme.inc:795 - getCertificatePSK function

    • This function appears to be the locus of a bug (needs investigating): Once a private key has been generated for a certificate, it will not be re-generated when the certificate is edited, even if a user changes the key-size.

    • This function is misnamed; it doesn't get a Pre-Shared Key, it generates a private key.

…d ability to specify (vs generate) private key"

This is required to allow certificates to work in environments where HPKP is deployed across subdomains.

UI (Services / Acme / Certificates / Edit Certificate Options:
==============================================================

* Added "Custom..." option at the end of the Key Size drop-down.

* Renamed this field from "Key Size" to "Private Key", since it now contains both generate and paste-custom options.

* Selecting "Custom..." from the list adds (makes visible) a "Custom Private Key" textarea field in the next section.

* Selecting any other of the other (key-size) options from the Private Key drop-down hides the "Custom Private Key"
  field again.

* Key sizes in the drop-down have been renamed for clarity, e.g. "2048" => "2048-bit RSA", "ec-384" => "384-bit ECDSA", etc.

* Fixed a few typos/misspellings in the files I worked on (e.g. "atempting" => "attempting")

Back End:
=========

* Like other user-entered X.509 PEM formatted data, the user-entered Custom Private Key text is stored base64_encoded
  in the configuration.

* The private-key-generation logic is skipped when the user pastes in their private key.

* When visible, "Custom Private Key" is a required field, and is also verified to contain "-----BEGIN PRIVATE KEY-----"
  and "-----END PRIVATE KEY-----" strings.

Additional Notes:
=================

* security/pfSense-pkg-acme/Makefile

  - I bumped the version number from 0.2.8 to 0.2.9. If you're using semver (i.e. last digit is bug fixes), then that
    should probably be 0.3.0.

* security/pfSense-pkg-acme/files/usr/local/pkg/acme/acme.inc:795 - getCertificatePSK function

  - This function appears to be the locus of a bug (needs investigating): Once a private key has been generated for
    a certificate, it will not be re-generated when the certificate is edited, even if a user changes the key-size.

  - This function is misnamed; it doesn't get a Pre-Shared Key, it generates a private key.
@jim-p
Copy link
Contributor

jim-p commented May 2, 2018

Redmine link: https://redmine.pfsense.org/issues/8490

Copy link
Contributor

@jim-p jim-p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK, needs some testing though.

@jim-p
Copy link
Contributor

jim-p commented May 2, 2018

I have seen that key size change issue before when I was working on https://redmine.pfsense.org/issues/8305 but at the time it seemed like a natural limit since you wouldn't normally change the private key after creating a certificate. It should be possible to detect the change and regenerate, but that might be worthy of a confirmation dialog to make sure it doesn't happen accidentally.

@plinss
Copy link

plinss commented May 2, 2018

Looks OK, needs some testing though.

I've tested locally and works as advertised.

This was also very useful as I have HPKP deployed on my root domain and it finally allows me to use Let's Encrypt certificates on the firewall.

@plinss
Copy link

plinss commented May 2, 2018

I have seen that key size change issue before when I was working on https://redmine.pfsense.org/issues/8305 but at the time it seemed like a natural limit since you wouldn't normally change the private key after creating a certificate

Changing the private key periodically is standard practice. Some people replace the private key every time they renew the certificate, though keeping it for a longer period is certainly valid and useful when using HPKP and/or DANE (especially with short-lived certificates like from Let's Encrypt).

However, when the user explicitly changes the key size it would be unexpected for the key to not be regenerated to reflect the new size (or type). A confirmation dialog would be fine.

Additionally, it would be useful to to be able to regenerate the private without having to delete and re-create the certificate (or awkwardly toggle through different sizes). Either an explicit control to regenerate the key (using the same size) and/or and expiration time when the key will automatically be regenerated would be better than simply keeping the same private key forever.

@jim-p
Copy link
Contributor

jim-p commented May 2, 2018

I agree it would be good to have a way to change that I just think it needs a higher barrier than simply changing the size. Perhaps a button that at least does a JS confirm to regenerate a new private key. That's for a different redmine issue/PR though, it's not related closely enough to this to warrant tacking it on.

@jim-p
Copy link
Contributor

jim-p commented May 2, 2018

Tried it out and it seems to test OK here, too.

@sbeaver-netgate
Copy link
Contributor

Please resolve the conflicts so that I can merge this PR.

@jim-p
Copy link
Contributor

jim-p commented May 17, 2018

I'm not sure why this isn't showing as merged, it's in the package now. I merged it and made some other unrelated changes a couple weeks ago.

@jim-p jim-p closed this May 17, 2018
@mr-intj mr-intj deleted the 8490-add-ability-to-specify-private-key branch May 17, 2018 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants