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

security/acme-client: bugfix release #76

Merged
merged 8 commits into from
Mar 14, 2017
Merged

Conversation

fraenki
Copy link
Member

@fraenki fraenki commented Feb 9, 2017

New features

  • Show acme status for each certificate in GUI

Bugfixes

  • Avoid API exception by skipping HAProxy integration if validation method is not configured
  • Don't log an error if no restart action was specified
  • Rename field "CA Account" to "LE Account" to avoid confusion

Example: certificate status in GUI

certstatus2

@fichtner
Copy link
Member

fichtner commented Feb 20, 2017

Attempting to enable HAProxy integration in LE main settings without a validation method using HAProxy:

sxmbhei

Exception: Error at /usr/local/opnsense/mvc/app/controllers/OPNsense/AcmeClient/Api/SettingsController.php:331 - Trying to get property of non-object (errno=8) in /usr/local/opnsense/mvc/app/controllers/OPNsense/Base/ApiControllerBase.php:84
Stack trace:
#0 /usr/local/opnsense/mvc/app/controllers/OPNsense/AcmeClient/Api/SettingsController.php(331): OPNsense\Base\ApiControllerBase->APIErrorHandler(8, 'Trying to get p...', '/usr/local/opns...', 331, Array)
#1 [internal function]: OPNsense\AcmeClient\Api\SettingsController->fetchHAProxyIntegrationAction()
#2 [internal function]: Phalcon\Dispatcher->callActionMethod(Object(OPNsense\AcmeClient\Api\SettingsController), 'fetchHAProxyInt...', Array)
#3 [internal function]: Phalcon\Dispatcher->_dispatch()
#4 [internal function]: Phalcon\Dispatcher->dispatch()
#5 /usr/local/opnsense/www/api.php(26): Phalcon\Mvc\Application->handle()
#6 {main}
PHP Warning:  cert_action_validator(): Node no longer exists in /usr/local/opnsense/scripts/OPNsense/AcmeClient/certhelper.php on line 176
PHP Warning:  cert_action_validator(): Node no longer exists in /usr/local/opnsense/scripts/OPNsense/AcmeClient/certhelper.php on line 243
PHP Warning:  cert_action_validator(): Node no longer exists in /usr/local/opnsense/scripts/OPNsense/AcmeClient/certhelper.php on line 123

@fraenki
Copy link
Member Author

fraenki commented Mar 8, 2017

@fichtner 6013add should fix the API exception.

@fraenki
Copy link
Member Author

fraenki commented Mar 8, 2017

PHP Warning:  cert_action_validator(): Node no longer exists in /usr/local/opnsense/scripts/OPNsense/AcmeClient/certhelper.php on line 176
PHP Warning:  cert_action_validator(): Node no longer exists in /usr/local/opnsense/scripts/OPNsense/AcmeClient/certhelper.php on line 243
PHP Warning:  cert_action_validator(): Node no longer exists in /usr/local/opnsense/scripts/OPNsense/AcmeClient/certhelper.php on line 123

I have no idea why and how this can occur.

@fichtner
Copy link
Member

fichtner commented Mar 8, 2017

@fraenki looks like a new warning in PHP 7.0 we cannot easily mute (except for removing it from PHP, but that is not an option because it will bite us later).

I have been unable to find any recent info on how to fix this. The iterator lifetime is over before the iteration completed. It could also be a reference count issue in PHP itself: data is removed before it is fully released. We will have to see, maybe double-check / reassess with PHP 7.1.

@fraenki fraenki changed the title [WIP] DO NOT MERGE! security/acme-client: next bugfix release security/acme-client: bugfix release Mar 8, 2017
@fraenki
Copy link
Member Author

fraenki commented Mar 8, 2017

@fichtner Feel free to merge this PR if the next OPNsense release is close. Otherwise I'll just keep on submitting fixes. :)

@fichtner fichtner self-assigned this Mar 8, 2017
@fichtner
Copy link
Member

fichtner commented Mar 8, 2017

Will take till next week. Leaving it open and thanks for your work! :)

@fraenki
Copy link
Member Author

fraenki commented Mar 9, 2017

Houston, we have a new feature ;)

@fichtner fichtner merged commit 3dac746 into opnsense:master Mar 14, 2017
@fichtner
Copy link
Member

Merged,thank you!

@fichtner
Copy link
Member

Backport commit: 2df9225

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

Successfully merging this pull request may close these issues.

2 participants