[tls] Add a PEMTrustManager to deal with different PEM files (e.g. self-signed or global CA certificates) #2622
Conversation
| * @throws CertificateException | ||
| */ | ||
| public PEMTrustManager(String pemCert) throws CertificateException { | ||
| if (!pemCert.isBlank() && pemCert.startsWith(BEGIN_CERT)) { |
There was a problem hiding this comment.
One of the use-cases (e.g. with the http binding) could be that there is a configuration field in the thing config that allows pinning the certificate permanently (the getInstanceFromServer does only pin until restarting the system).
Using line.separator would require the configuration to be different depending on the operating system that openHAB runs on and could lead to problems if the user configures on a different OS. Maybe the cert string should be normalized to the line.separator line endings before the checks are done.
There was a problem hiding this comment.
One of the use-cases (e.g. with the http binding) could be that there is a configuration field in the thing config
Yes, exactly. I already though about such an implementation for another binding.
A refresh feature for the downloaded cert would be nice (e.g. on expiration). A future enhancement.
There was a problem hiding this comment.
Can you check if the new version handles the line endings even better?
bundles/org.openhab.core.io.net/src/main/java/org/openhab/core/io/net/http/PEMTrustManager.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.core.io.net/src/main/java/org/openhab/core/io/net/http/PEMTrustManager.java
Outdated
Show resolved
Hide resolved
|
This is IMO one of the most useful extensions in the last years, it allows the remove the |
bundles/org.openhab.core.io.net/src/main/java/org/openhab/core/io/net/http/PEMTrustManager.java
Outdated
Show resolved
Hide resolved
| * @throws CertificateException | ||
| */ | ||
| public PEMTrustManager(String pemCert) throws CertificateException { | ||
| if (!pemCert.isBlank() && pemCert.startsWith(BEGIN_CERT)) { |
There was a problem hiding this comment.
One of the use-cases (e.g. with the http binding) could be that there is a configuration field in the thing config
Yes, exactly. I already though about such an implementation for another binding.
A refresh feature for the downloaded cert would be nice (e.g. on expiration). A future enhancement.
| * @throws FileNotFoundException | ||
| * @throws CertificateInstantiationException | ||
| */ | ||
| public static PEMTrustManager getInstanceFromFile(String path) throws FileNotFoundException, CertificateException { |
There was a problem hiding this comment.
I am currently not sure if we really need this. There is a mir elegant way to provide a certificate by placing it into the resources folder (see icloud binding).
bundles/org.openhab.core.io.net/src/main/java/org/openhab/core/io/net/http/PEMTrustManager.java
Outdated
Show resolved
Hide resolved
cweitkamp
force-pushed
the
feature-pem-tls-trust-manager
branch
2 times, most recently
from
e46fcf4
to
997b8cd
Compare
self-signed or global CA certificates) Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
cweitkamp
force-pushed
the
feature-pem-tls-trust-manager
branch
from
997b8cd
to
74acea3
Compare
|
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/java-connection-to-https-with-self-signed-cert/130593/5 |
kaikreuzer
changed the title
There was a problem hiding this comment.
lgtm, thank you!
Let's wait for @J-N-K final comments before merging.
|
I'll check today and report back this evening. |
…lf-signed or global CA certificates) (openhab#2622) * Added a PEMTrustManager to deal with different PEM files (e.g. self-signed or global CA certificates) Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de> GitOrigin-RevId: 9609ffb
Signed-off-by: Christoph Weitkamp github@christophweitkamp.de