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

Allow adding/changing Jetty server certificates via REST API #2905

Closed
wants to merge 2 commits into from

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Apr 9, 2022

Depends on openhab/openhab-distro#1383

This adds a REST resource to add/change the Jetty server certificate. An empty DTO resets the certificate to a self-signed certificate.

Originally I thought about a WatchService for certificate files but this seems unnecessary. If certificates are changed on a regular basis (like with Let's encrypt), it is easy to add a post-hook script that uses keytool to insert the certificates retrieved in the keystore. For changing long-term certificates, the REST API is enough.

@J-N-K J-N-K requested a review from a team as a code owner April 9, 2022 18:00
@J-N-K J-N-K force-pushed the feature-certificatefilewatcher branch from edf38e8 to 274861e Compare April 20, 2022 17:41
@wborn wborn added the enhancement An enhancement or new feature of the Core label Apr 22, 2022
@splatch
Copy link
Contributor

splatch commented May 3, 2022

My sense of what this change is tells me that it could deserve a 'feature' label as it brings a certificate management to upper layers than they were before.

@J-N-K J-N-K added the REST/SSE label May 16, 2022
@J-N-K J-N-K force-pushed the feature-certificatefilewatcher branch from 274861e to 413fe19 Compare May 16, 2022 21:29
@J-N-K J-N-K force-pushed the feature-certificatefilewatcher branch from 413fe19 to 0528af9 Compare January 3, 2023 20:17
J-N-K added 2 commits May 28, 2023 10:57
Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K J-N-K force-pushed the feature-certificatefilewatcher branch from c076598 to ed073a3 Compare May 28, 2023 12:05
@wborn
Copy link
Member

wborn commented Aug 20, 2023

Why not use the existing settings mechanism?

@wborn
Copy link
Member

wborn commented Aug 20, 2023

A separate REST endpoint might make sense if we can also use it to add other self signed certificates to the keystore via the UI: openhab/openhab-addons#10446

@ghys
Copy link
Member

ghys commented Aug 20, 2023

I appreciate the general principle, but I actually have professional experience in this (PKIs and stuff), and I would only make this word of caution because I just have to: you're allowing a security feature to be altered by an API, so the weakest link becomes the API, and the OH REST API is not the strongest... most of the instances are not even accessed with HTTPS.

I would strongly advise you to reconsider this or secure it properly as the certificate and private key provide more security than this endpoint (because it allows to change them).

@J-N-K J-N-K closed this Aug 20, 2023
@J-N-K J-N-K deleted the feature-certificatefilewatcher branch August 20, 2023 07:47
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/feedback-on-installing-openhab-with-latest-versions/154238/16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core REST/SSE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants