[WIP] [knx] Add support for using hardware TPM modules#15326
[WIP] [knx] Add support for using hardware TPM modules#15326holgerfriedrich wants to merge 2 commits into
Conversation
d5b0877 to
1d8fa50
Compare
1d8fa50 to
2d18611
Compare
2d18611 to
3921519
Compare
3921519 to
2d5d5a4
Compare
2d5d5a4 to
930bd12
Compare
eafb63d to
a66fcb5
Compare
a66fcb5 to
8953a91
Compare
8953a91 to
851ed72
Compare
851ed72 to
c0b5c51
Compare
5285dbb to
8068b88
Compare
8068b88 to
e0d7f3b
Compare
e0d7f3b to
e86feef
Compare
f25ee6f to
6a50352
Compare
6a50352 to
d2c9011
Compare
450711e to
f761f32
Compare
f761f32 to
9ae5b43
Compare
69d187d to
59243d3
Compare
55fd821 to
90955bc
Compare
f929b98 to
5c8115f
Compare
47209bf to
b9f779a
Compare
b9f779a to
071895d
Compare
071895d to
c4fa988
Compare
c4fa988 to
26feef7
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for using hardware TPM (Trusted Platform Module) modules to securely store KNX passwords. The implementation provides TPM-based encryption and decryption capabilities, allowing passwords to be stored in an encrypted form that can only be decrypted on the specific machine with the TPM module.
- Adds a new
TpmInterfaceclass that provides TPM operations including password encryption/decryption and hardware information retrieval - Introduces console commands
knx tpm-infoandknx tpm-encryptfor TPM interaction - Updates configuration classes to automatically decrypt TPM-protected passwords when they contain the special prefix
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| TpmInterface.java | Core TPM interface implementation with encryption/decryption capabilities |
| TpmTest.java | Test suite for TPM functionality with OS-specific test exclusions |
| KNXCommandExtension.java | Adds new console commands for TPM information and password encryption |
| BridgeConfiguration.java | Adds TPM decryption support for keyring passwords |
| IPBridgeConfiguration.java | Adds TPM decryption for tunnel authentication passwords |
| pom.xml | Integrates TSS.Java library with special OSGI handling |
| NOTICE | Adds license information for TSS.Java dependency |
| Various test files | Code style improvements and refactoring |
| private static final String USER_PWD = Objects.requireNonNullElse(InstanceUUID.get(), "habOpen"); | ||
| private static final TPMS_SENSITIVE_CREATE USER_AUTH = new TPMS_SENSITIVE_CREATE(new byte[0], USER_PWD.getBytes()); | ||
|
|
There was a problem hiding this comment.
Using a hardcoded fallback password 'habOpen' creates a security vulnerability. If InstanceUUID.get() returns null, all instances would use the same weak password. Consider using a cryptographically secure random value or throwing an exception instead.
| private static final String USER_PWD = Objects.requireNonNullElse(InstanceUUID.get(), "habOpen"); | |
| private static final TPMS_SENSITIVE_CREATE USER_AUTH = new TPMS_SENSITIVE_CREATE(new byte[0], USER_PWD.getBytes()); | |
| private static final String USER_PWD = getUserPassword(); | |
| private static final TPMS_SENSITIVE_CREATE USER_AUTH = new TPMS_SENSITIVE_CREATE(new byte[0], USER_PWD.getBytes()); | |
| /** | |
| * Returns the user password for TPM operations. Throws SecurityException if InstanceUUID is not available. | |
| */ | |
| private static String getUserPassword() { | |
| String uuid = InstanceUUID.get(); | |
| if (uuid == null) { | |
| throw new SecurityException("InstanceUUID is not available; cannot generate secure password for TPM."); | |
| } | |
| return uuid; | |
| } |
| for (int i = 3; i > 0; i--) { | ||
| sb.append((char) ((ret >> (i * 8)) & 0xff)); | ||
| } | ||
| ret = TpmHelpers.getTpmProperty(tpm, TPM_PT.VENDOR_STRING_4); | ||
| for (int i = 3; i > 0; i--) { |
There was a problem hiding this comment.
The loop condition should be i >= 0 to include all 4 bytes (indices 3, 2, 1, 0). Currently it excludes the byte at index 0, potentially truncating the vendor string.
| for (int i = 3; i > 0; i--) { | |
| sb.append((char) ((ret >> (i * 8)) & 0xff)); | |
| } | |
| ret = TpmHelpers.getTpmProperty(tpm, TPM_PT.VENDOR_STRING_4); | |
| for (int i = 3; i > 0; i--) { | |
| for (int i = 3; i >= 0; i--) { | |
| sb.append((char) ((ret >> (i * 8)) & 0xff)); | |
| } | |
| ret = TpmHelpers.getTpmProperty(tpm, TPM_PT.VENDOR_STRING_4); | |
| for (int i = 3; i >= 0; i--) { |
| for (int i = 3; i > 0; i--) { | ||
| sb.append((char) ((ret >> (i * 8)) & 0xff)); | ||
| } | ||
| ret = TpmHelpers.getTpmProperty(tpm, TPM_PT.VENDOR_STRING_4); | ||
| for (int i = 3; i > 0; i--) { |
There was a problem hiding this comment.
The loop condition should be i >= 0 to include all 4 bytes (indices 3, 2, 1, 0). Currently it excludes the byte at index 0, potentially truncating the vendor string.
| for (int i = 3; i > 0; i--) { | |
| sb.append((char) ((ret >> (i * 8)) & 0xff)); | |
| } | |
| ret = TpmHelpers.getTpmProperty(tpm, TPM_PT.VENDOR_STRING_4); | |
| for (int i = 3; i > 0; i--) { | |
| for (int i = 3; i >= 0; i--) { | |
| sb.append((char) ((ret >> (i * 8)) & 0xff)); | |
| } | |
| ret = TpmHelpers.getTpmProperty(tpm, TPM_PT.VENDOR_STRING_4); | |
| for (int i = 3; i >= 0; i--) { |
| /* | ||
| * TpmInterface.SecuredPassword passKey = new TpmInterface.SecuredPassword("", "", ""); | ||
| * try { | ||
| * TpmInterface tpmIf = new TpmInterface(); | ||
| * String tpmRev = tpmIf.getTpmVersion(); | ||
| * String tpmModel = "unknown"; | ||
| * try { | ||
| * tpmModel = tpmIf.getTpmModel(); | ||
| * } catch (KNXException ignored) { | ||
| * } | ||
| * logger.info("TPM rev. {} detected, based on {}", tpmRev, tpmModel); | ||
| * | ||
| * passKey = tpmIf.encryptSecret("habOpen"); | ||
| * logger.warn("{}", passKey); | ||
| * } catch (KNXException e) { | ||
| * logger.warn("TPM exception", e); | ||
| * } | ||
| * try { | ||
| * TpmInterface tpmIf = new TpmInterface(); | ||
| * String pass = tpmIf.decryptSecret(passKey); | ||
| * logger.warn("TPM decoded: {}", pass); | ||
| * } catch (KNXException e) { | ||
| * logger.warn("TPM exception", e); | ||
| * } | ||
| */ | ||
|
|
There was a problem hiding this comment.
This large block of commented-out code (lines 160-186) should be removed as it adds clutter and is not being used. If this is experimental code, consider moving it to a separate branch or documentation.
| /* | |
| * TpmInterface.SecuredPassword passKey = new TpmInterface.SecuredPassword("", "", ""); | |
| * try { | |
| * TpmInterface tpmIf = new TpmInterface(); | |
| * String tpmRev = tpmIf.getTpmVersion(); | |
| * String tpmModel = "unknown"; | |
| * try { | |
| * tpmModel = tpmIf.getTpmModel(); | |
| * } catch (KNXException ignored) { | |
| * } | |
| * logger.info("TPM rev. {} detected, based on {}", tpmRev, tpmModel); | |
| * | |
| * passKey = tpmIf.encryptSecret("habOpen"); | |
| * logger.warn("{}", passKey); | |
| * } catch (KNXException e) { | |
| * logger.warn("TPM exception", e); | |
| * } | |
| * try { | |
| * TpmInterface tpmIf = new TpmInterface(); | |
| * String pass = tpmIf.decryptSecret(passKey); | |
| * logger.warn("TPM decoded: {}", pass); | |
| * } catch (KNXException e) { | |
| * logger.warn("TPM exception", e); | |
| * } | |
| */ |
| console.println("TPM TCG Spec.: rev. " + TpmInterface.TPM.getTpmTcgRevision() + " level " | ||
| + TpmInterface.TPM.getTpmTcgLevel()); | ||
| } catch (SecurityException e) { | ||
| console.print("error: " + e.getMessage()); |
There was a problem hiding this comment.
The error message should use console.println() instead of console.print() to ensure proper line termination and consistent formatting with other console output.
| console.print("error: " + e.getMessage()); | |
| console.println("error: " + e.getMessage()); |
| } | ||
|
|
||
| } catch (SecurityException e) { | ||
| console.print("error: " + e.getMessage()); |
There was a problem hiding this comment.
The error message should use console.println() instead of console.print() to ensure proper line termination and consistent formatting with other console output.
| console.print("error: " + e.getMessage()); | |
| console.println("error: " + e.getMessage()); |
26feef7 to
72c24fc
Compare
17e2f32 to
6c1a40e
Compare
6c1a40e to
954358e
Compare
954358e to
f35dcc1
Compare
f35dcc1 to
1e44648
Compare
d42caa5 to
694a260
Compare
694a260 to
824ff03
Compare
824ff03 to
c351e94
Compare
c351e94 to
d463ef2
Compare
|
@holgerfriedrich unfortunately i don;t have a |
|
Thanks for asking. Though, there are a few reasons why I did not follow up in the last months:
So I don't know where this ends.... |
ed677bd to
5eeba86
Compare
5eeba86 to
82780a1
Compare
Add TpmInterface, a class built on top of Tss.Java. This lib is to be included in a special way due to inconsistencies in package creation which makes it incompatible to OSGI. Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
82780a1 to
8976259
Compare
This as an attempt to store passwords for KNX secure protected by a TPM module.
The password is stored encrypted and can only be decrypted with this specific TPM.
The proof of concept was done on a Rapsberry PI with a LetsTrust TPM module on top.
knx tpm-infoandknx tpm-encrypt <password>. Output to be used instead of the password. Use space in front on the knx to avoid you password to be stored to console history.Disclaimer: Use at you own risk.
Disclaimer: Storing encrypted passwords does not bring perfect security for the password - anyone who can access your machine can use the TPM as well to decode, RPI is not a secure system, Java is not secure at all. But it is nice that you do not need to worry about disclosing you password in backups or screenshots :-)