From 4e6d9ddff61ad6447e42ba12a3e1226bd0e1380e Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Thu, 8 Oct 2026 14:36:25 +0200 Subject: [PATCH 1/2] Fix the SpotBugs findings and gate the build on SpotBugs (#116) SpotBugs 4.10.4 reported 196 bugs on main (DecoderRunner and a few others were already gone); it now reports 0 and spotbugs:check runs at verify, pinned to 4.10.4.1 like the site report. Real fixes: - ProtocolDecoder.decodePayload: an empty payload threw a NullPointerException in the payload constructors; it now throws IllegalArgumentException("Empty payload") (NP_GUARANTEED_DEREF) - Credentials (part of #90): the user name and password are encoded in UTF-8 whatever the platform charset, and the BMC key (Kg) is passed to the HMAC as raw bytes instead of going through new String(key) and getBytes(), which corrupted any byte of 80h or above into a wrong SIK. The user name length and its 16-byte limit are counted in encoded bytes. AuthenticationAlgorithm takes the key and password as byte[] (DM_DEFAULT_ENCODING). Rakp1Test checks the SIK against IPMI 2.0 section 13.31 and fails on the old code. - volatile on the fields shared by the caller, UDP and timer threads in Connection, MessageQueue and UdpMessenger; MessageListener writes its tag under its lock (AT_STALE_THREAD_WRITE_OF_PRIMITIVE, IS2, part of #96) - ConnectionManager locks a dedicated object instead of an AtomicInteger, SessionManager uses an AtomicInteger instead of a static synchronized method (JLM, USO) - UdpMessenger: remove the unused static getSentPackets() debug counter (ST, SSD); setBufferSize() now sizes the receive buffer, which was hard-coded to 512 bytes - SerialOverLan: the overloads documented as using the platform charset say so with Charset.defaultCharset(); ManagementAccessInfo decodes ISO-8859-1 like the other FRU 8-bit ASCII fields - PropertiesManager closes its resource stream (OBL, part of #98) - Remove a vacuous instanceof (IpmiCommandCoder) and a useless condition (ChassisInfo); CONST1/CONST2 are private (MS_PKGPROTECT) Justified suppressions (spotbugs-annotations, provided scope): - core/package-info.java: CT_CONSTRUCTOR_THROW and EI_EXPOSE_REP (which also matches EI_EXPOSE_REP2) for the whole protocol core: constructors validate arguments and guard no security-sensitive state, and response data and records are mutable holders with public setters, so defensive copies would protect no invariant - IpmiClientConfiguration (credentials shared so the caller can wipe them), Fru and Sensor (result holders), PropertiesManager singleton Live-verified on the GIGABYTE and Lenovo IMM BMCs: login with cipher suites 3 and 17 and an encrypted command; a wrong password is still rejected. Closes #116 Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- README.md | 2 + pom.xml | 25 ++++- .../ipmi/client/IpmiClientConfiguration.java | 5 + .../org/metricshub/ipmi/client/model/Fru.java | 5 + .../metricshub/ipmi/client/model/Sensor.java | 5 + .../ipmi/core/api/sol/SerialOverLan.java | 4 +- .../ipmi/core/api/sync/MessageListener.java | 16 ++-- .../coding/commands/IpmiCommandCoder.java | 12 +-- .../commands/fru/record/ChassisInfo.java | 3 - .../fru/record/ManagementAccessInfo.java | 4 +- .../core/coding/commands/session/Rakp1.java | 95 +++++++------------ .../core/coding/commands/session/Rakp3.java | 25 +---- .../protocol/decoder/ProtocolDecoder.java | 14 +-- .../security/AuthenticationAlgorithm.java | 11 +-- .../security/AuthenticationRakpNone.java | 4 +- .../security/ConfidentialityAesCbc128.java | 2 +- .../coding/security/IntegrityAlgorithm.java | 2 +- .../ipmi/core/common/PropertiesManager.java | 8 +- .../ipmi/core/connection/Connection.java | 10 +- .../core/connection/ConnectionManager.java | 17 ++-- .../ipmi/core/connection/SessionManager.java | 8 +- .../core/connection/queue/MessageQueue.java | 2 +- .../metricshub/ipmi/core/package-info.java | 9 ++ .../ipmi/core/transport/UdpMessenger.java | 19 +--- src/site/markdown/configuration.md | 7 +- src/site/markdown/upgrading.md | 19 +++- .../coding/commands/session/Rakp1Test.java | 63 ++++++++++++ 28 files changed, 235 insertions(+), 163 deletions(-) create mode 100644 src/test/java/org/metricshub/ipmi/core/coding/commands/session/Rakp1Test.java diff --git a/AGENTS.md b/AGENTS.md index ba8e193..2dd3b66 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -26,7 +26,7 @@ Unit tests must not depend on a real BMC. Exercising the RMCP+ session code agai ## Code quality reports -Code quality reports (checkstyle, pmd/cpd, spotbugs) are generated by `mvn verify site` into ./target/checkstyle-result.xml, ./target/pmd.xml, ./target/cpd.xml and ./target/spotbugsXml.xml. Checkstyle and PMD are gated (the build fails on any error); CPD is gated on duplications of 100 tokens or more (`cpd-check` at `verify`), while the site report lists those of 50 tokens or more; SpotBugs is not yet gated (issue #116 tracks the clean-up). Extract shared code instead of copying it: `AbstractSensorRecord` holds what the Full, Compact and Event-Only sensor records share, and `IpmiCommandCoder.validateResponse()` the response checks of every command. Do not add new violations: check the reports for the files you changed before committing and submitting your code. The SpotBugs site report runs spotbugs-maven-plugin 4.10.4.1 (pinned in ``, as the 4.9.3.0 of the parent POM cannot read the class files of JDK 21+); to run SpotBugs alone, use `mvn com.github.spotbugs:spotbugs-maven-plugin:4.10.4.1:spotbugs`. +Code quality reports (checkstyle, pmd/cpd, spotbugs) are generated by `mvn verify site` into ./target/checkstyle-result.xml, ./target/pmd.xml, ./target/cpd.xml and ./target/spotbugsXml.xml. Checkstyle and PMD are gated (the build fails on any error); CPD is gated on duplications of 100 tokens or more (`cpd-check` at `verify`), while the site report lists those of 50 tokens or more; SpotBugs is gated on any bug at the default threshold (`spotbugs:check` at `verify`). Extract shared code instead of copying it: `AbstractSensorRecord` holds what the Full, Compact and Event-Only sensor records share, and `IpmiCommandCoder.validateResponse()` the response checks of every command. Do not add new violations: check the reports for the files you changed before committing and submitting your code. The SpotBugs gate and site report run spotbugs-maven-plugin 4.10.4.1 (pinned in `` and ``, as the 4.9.3.0 of the parent POM cannot read the class files of JDK 21+); to run SpotBugs alone, use `mvn com.github.spotbugs:spotbugs-maven-plugin:4.10.4.1:check`. Fix SpotBugs findings; when one is intentional, suppress it with `@SuppressFBWarnings` (`spotbugs-annotations`, provided scope) and a `justification`. `CT_CONSTRUCTOR_THROW` and `EI_EXPOSE_REP` (which also matches `EI_EXPOSE_REP2`) are suppressed for the whole protocol core in `org/metricshub/ipmi/core/package-info.java`; SpotBugs reports a suppression that matches nothing, so remove it with the code it covered. ## Documentation diff --git a/README.md b/README.md index 303cbe3..29ba63a 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,8 @@ mvn formatter:format The build also fails on [Checkstyle](checkstyle.xml) violations. A justified violation can be suppressed with `// CHECKSTYLE.OFF: ` and `// CHECKSTYLE.ON: ` comments. +The build fails on any SpotBugs bug as well. An intentional one is suppressed with `@SuppressFBWarnings` and a `justification`. + To ignore the whole-tree reformat commit in `git blame`, run once: ```bash diff --git a/pom.xml b/pom.xml index 43bef44..089d002 100644 --- a/pom.xml +++ b/pom.xml @@ -92,6 +92,13 @@ + + + com.github.spotbugs + spotbugs-annotations + 4.10.4 + provided + org.slf4j slf4j-api @@ -177,6 +184,22 @@ + + + com.github.spotbugs + spotbugs-maven-plugin + 4.10.4.1 + + + verify + + check + + + + + + com.github.spotbugs spotbugs-maven-plugin diff --git a/src/main/java/org/metricshub/ipmi/client/IpmiClientConfiguration.java b/src/main/java/org/metricshub/ipmi/client/IpmiClientConfiguration.java index 57241da..232d114 100644 --- a/src/main/java/org/metricshub/ipmi/client/IpmiClientConfiguration.java +++ b/src/main/java/org/metricshub/ipmi/client/IpmiClientConfiguration.java @@ -24,10 +24,15 @@ import org.metricshub.ipmi.core.common.Constants; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + /** * IPMI configuration including the required credentials that need to be used to establish the * communication with the IPMI interface. */ +// EI_EXPOSE_REP also matches EI_EXPOSE_REP2 +@SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "The password char[] and BMC key byte[] are " + + "deliberately shared, not copied, so that the caller can wipe the only copy of the credentials") public class IpmiClientConfiguration { private String hostname; diff --git a/src/main/java/org/metricshub/ipmi/client/model/Fru.java b/src/main/java/org/metricshub/ipmi/client/model/Fru.java index f542382..cf204fc 100644 --- a/src/main/java/org/metricshub/ipmi/client/model/Fru.java +++ b/src/main/java/org/metricshub/ipmi/client/model/Fru.java @@ -24,6 +24,8 @@ import java.util.List; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import org.metricshub.ipmi.core.coding.commands.fru.record.BoardInfo; import org.metricshub.ipmi.core.coding.commands.fru.record.ChassisInfo; import org.metricshub.ipmi.core.coding.commands.fru.record.FruRecord; @@ -37,6 +39,9 @@ *
  • The FRU records containing {@link BoardInfo}, {@link ChassisInfo} and/or {@link ProductInfo}.
    • * */ +// EI_EXPOSE_REP also matches EI_EXPOSE_REP2 +@SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "Result holder: hands out the decoded records, " + + "which are mutable holders themselves, by reference") public class Fru { private FruDeviceLocatorRecord fruLocator; diff --git a/src/main/java/org/metricshub/ipmi/client/model/Sensor.java b/src/main/java/org/metricshub/ipmi/client/model/Sensor.java index 8ee79c4..83bdfb5 100644 --- a/src/main/java/org/metricshub/ipmi/client/model/Sensor.java +++ b/src/main/java/org/metricshub/ipmi/client/model/Sensor.java @@ -22,6 +22,8 @@ * ╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱ */ +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import org.metricshub.ipmi.core.coding.commands.sdr.GetSensorReadingResponseData; import org.metricshub.ipmi.core.coding.commands.sdr.record.AbstractSensorRecord; import org.metricshub.ipmi.core.coding.commands.sdr.record.CompactSensorRecord; @@ -42,6 +44,9 @@ * $sensorName=$state|$sensorName=$state|...|$sensorName=$state * */ +// EI_EXPOSE_REP also matches EI_EXPOSE_REP2 +@SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "Result holder: hands out the decoded record and " + + "reading, which are mutable holders themselves, by reference") public class Sensor { private SensorRecord sensorRecord; diff --git a/src/main/java/org/metricshub/ipmi/core/api/sol/SerialOverLan.java b/src/main/java/org/metricshub/ipmi/core/api/sol/SerialOverLan.java index 5344be6..337d2ba 100644 --- a/src/main/java/org/metricshub/ipmi/core/api/sol/SerialOverLan.java +++ b/src/main/java/org/metricshub/ipmi/core/api/sol/SerialOverLan.java @@ -413,7 +413,7 @@ public boolean writeIntArray(int[] buffer) { * @return true if whole string was successfully sent and acknowledged by remote server, false otherwise. */ public boolean writeString(String string) { - return writeBytes(string.getBytes()); + return writeString(string, Charset.defaultCharset()); } /** @@ -541,7 +541,7 @@ public String readString() { * @return all bytes that could be read as {@link String}, but no more than given byteCount. */ public String readString(int byteCount) { - return new String(readBytes(byteCount)); + return readString(Charset.defaultCharset(), byteCount); } /** diff --git a/src/main/java/org/metricshub/ipmi/core/api/sync/MessageListener.java b/src/main/java/org/metricshub/ipmi/core/api/sync/MessageListener.java index 7e29ef9..6e1bb31 100644 --- a/src/main/java/org/metricshub/ipmi/core/api/sync/MessageListener.java +++ b/src/main/java/org/metricshub/ipmi/core/api/sync/MessageListener.java @@ -46,7 +46,7 @@ public class MessageListener implements IpmiResponseListener { private int tag; - private IpmiResponse response; + private volatile IpmiResponse response; /** * Messages that have proper connection handle but arrived before tag was @@ -86,17 +86,21 @@ public ResponseData waitForAnswer(int messageTag) throws Exception { if (messageTag < 0 || messageTag > 63) { throw new IllegalArgumentException("Corrupted message tag"); } - this.tag = messageTag; - for (IpmiResponse quickResponse : quickMessages) { - this.notify(quickResponse); + synchronized (this) { + this.tag = messageTag; + for (IpmiResponse quickResponse : quickMessages) { + this.notify(quickResponse); + } } while (response == null) { Thread.sleep(1); } if (response instanceof IpmiResponseData) { - this.tag = -1; - quickMessages.clear(); + synchronized (this) { + this.tag = -1; + quickMessages.clear(); + } return ((IpmiResponseData) response).getResponseData(); } else /* response instanceof IpmiError */ { throw ((IpmiError) response).getException(); diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/IpmiCommandCoder.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/IpmiCommandCoder.java index ac7f5ac..c70498f 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/IpmiCommandCoder.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/IpmiCommandCoder.java @@ -24,7 +24,6 @@ import org.metricshub.ipmi.core.coding.PayloadCoder; import org.metricshub.ipmi.core.coding.payload.CompletionCode; -import org.metricshub.ipmi.core.coding.payload.IpmiPayload; import org.metricshub.ipmi.core.coding.payload.PlainMessage; import org.metricshub.ipmi.core.coding.payload.lan.IPMIException; import org.metricshub.ipmi.core.coding.payload.lan.IpmiLanResponse; @@ -66,15 +65,10 @@ public PayloadType getSupportedPayloadType() { * class, false otherwise. */ public boolean isCommandResponse(IpmiMessage message) { - if (message.getPayload() instanceof IpmiPayload) { - if (message.getPayload() instanceof IpmiLanResponse) { - return ((IpmiLanResponse) message.getPayload()).getCommand() == getCommandCode(); - } else { - return message.getPayload() instanceof PlainMessage; - } - } else { - return false; + if (message.getPayload() instanceof IpmiLanResponse) { + return ((IpmiLanResponse) message.getPayload()).getCommand() == getCommandCode(); } + return message.getPayload() instanceof PlainMessage; } /** diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ChassisInfo.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ChassisInfo.java index 97017c3..e062ecf 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ChassisInfo.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ChassisInfo.java @@ -105,9 +105,6 @@ public ChassisInfo(final byte[] fruData, final int offset) { true)); break; default: - if (partDataLength == 0) { - continue; - } customInfo .add( decodeString( diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ManagementAccessInfo.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ManagementAccessInfo.java index a1620b1..0355314 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ManagementAccessInfo.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/fru/record/ManagementAccessInfo.java @@ -24,6 +24,8 @@ import org.metricshub.ipmi.core.common.TypeConverter; +import java.nio.charset.StandardCharsets; + /** * Management Access Information record from FRU Multi Record Area */ @@ -56,7 +58,7 @@ public ManagementAccessInfo(byte[] fruData, int offset, int length) { System.arraycopy(fruData, offset + 1, buffer, 0, length - 1); - accessInfo = new String(buffer); + accessInfo = new String(buffer, StandardCharsets.ISO_8859_1); } public ManagementAccessRecordType getRecordType() { diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/session/Rakp1.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/session/Rakp1.java index 7726c1d..0edeaa0 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/session/Rakp1.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/session/Rakp1.java @@ -40,6 +40,7 @@ import org.metricshub.ipmi.core.common.Randomizer; import org.metricshub.ipmi.core.common.TypeConverter; +import java.nio.charset.StandardCharsets; import java.security.InvalidKeyException; import java.security.NoSuchAlgorithmException; @@ -108,9 +109,9 @@ public PrivilegeLevel getRequestedMaximumPrivilegeLevel() { } public void setUsername(String username) { - if (username.length() > 16) { + if (username.getBytes(StandardCharsets.UTF_8).length > 16) { throw new IllegalArgumentException( - "Username is too long. It's length cannot exceed 16"); + "Username is too long. It's length cannot exceed 16 bytes"); } this.username = username; } @@ -119,6 +120,13 @@ public String getUsername() { return username; } + /** + * @return the user name as sent to the BMC, encoded in UTF-8 + */ + byte[] getUsernameBytes() { + return username.getBytes(StandardCharsets.UTF_8); + } + private void setPassword(String password) { this.password = password; } @@ -127,6 +135,13 @@ public String getPassword() { return password; } + /** + * @return the password as the key of the authentication algorithm, encoded in UTF-8 (empty when null) + */ + byte[] getPasswordBytes() { + return password == null ? new byte[0] : password.getBytes(StandardCharsets.UTF_8); + } + private void setConsoleRandomNumber(byte[] randomNumber) { this.consoleRandomNumber = randomNumber; } @@ -216,13 +231,8 @@ public IpmiMessage encodePayload(int messageSequenceNumber, int sessionSequenceN @Override protected IpmiPayload preparePayload(int sequenceNumber) { - byte[] payload = null; - - if (getUsername() == null) { - setUsername(""); - } - - payload = new byte[28 + getUsername().length()]; + byte[] usernameBytes = getUsernameBytes(); + byte[] payload = new byte[28 + usernameBytes.length]; // message tag payload[0] = TypeConverter.intToByte(sequenceNumber); @@ -247,18 +257,8 @@ protected IpmiPayload preparePayload(int sequenceNumber) { payload[25] = 0; // reserved payload[26] = 0; // reserved - payload[27] = TypeConverter.intToByte(getUsername().length()); // username - // length - - if (getUsername().length() > 0) { - System - .arraycopy( - getUsername().getBytes(), - 0, - payload, - 28, - getUsername().length()); // username - } + payload[27] = TypeConverter.intToByte(usernameBytes.length); // username length + System.arraycopy(usernameBytes, 0, payload, 28, usernameBytes.length); // username return new PlainMessage(payload); } @@ -349,7 +349,7 @@ public ResponseData getResponseData(IpmiMessage message) .checkKeyExchangeAuthenticationCode( prepareKeyExchangeAuthenticationCodeBase(data), key, - getPassword())) { + getPasswordBytes())) { throw new IllegalArgumentException("Authentication check failed"); } @@ -362,11 +362,8 @@ public ResponseData getResponseData(IpmiMessage message) */ private byte[] prepareKeyExchangeAuthenticationCodeBase( Rakp1ResponseData responseData) { - int length = 58; - if (getUsername() != null) { - length += getUsername().length(); - } - byte[] keac = new byte[length]; + byte[] usernameBytes = getUsernameBytes(); + byte[] keac = new byte[58 + usernameBytes.length]; byte[] rSID = TypeConverter .intToLittleEndianByteArray( @@ -395,20 +392,8 @@ private byte[] prepareKeyExchangeAuthenticationCodeBase( keac[56] = TypeConverter .intToByte(encodePrivilegeLevel(requestedMaximumPrivilegeLevel) | 0x10); - if (getUsername() != null) { - keac[57] = TypeConverter.intToByte(getUsername().length()); - if (getUsername().length() > 0) { - System - .arraycopy( - getUsername().getBytes(), - 0, - keac, - 58, - getUsername().length()); - } - } else { - keac[57] = 0; - } + keac[57] = TypeConverter.intToByte(usernameBytes.length); + System.arraycopy(usernameBytes, 0, keac, 58, usernameBytes.length); return keac; } @@ -430,7 +415,7 @@ public byte[] calculateSik(Rakp1ResponseData responseData) NoSuchAlgorithmException { byte[] key = null; if (getBmcKey() == null || getBmcKey().length <= 0) { - key = getPassword().getBytes(); + key = getPasswordBytes(); } else { key = getBmcKey(); } @@ -439,7 +424,7 @@ public byte[] calculateSik(Rakp1ResponseData responseData) .getAuthenticationAlgorithm() .getKeyExchangeAuthenticationCode( prepareSikBase(responseData), - new String(key)); + key); } /** @@ -447,12 +432,8 @@ public byte[] calculateSik(Rakp1ResponseData responseData) * Integrity Key */ private byte[] prepareSikBase(Rakp1ResponseData responseData) { - int length = 34; - if (getUsername() != null) { - length += getUsername().length(); - } - - byte[] sikBase = new byte[length]; + byte[] usernameBytes = getUsernameBytes(); + byte[] sikBase = new byte[34 + usernameBytes.length]; System.arraycopy(getConsoleRandomNumber(), 0, sikBase, 0, 16); @@ -467,20 +448,8 @@ private byte[] prepareSikBase(Rakp1ResponseData responseData) { sikBase[32] = TypeConverter .intToByte(encodePrivilegeLevel(requestedMaximumPrivilegeLevel) | 0x10); - if (getUsername() != null) { - sikBase[33] = TypeConverter.intToByte(getUsername().length()); - if (getUsername().length() > 0) { - System - .arraycopy( - getUsername().getBytes(), - 0, - sikBase, - 34, - getUsername().length()); - } - } else { - sikBase[33] = 0; - } + sikBase[33] = TypeConverter.intToByte(usernameBytes.length); + System.arraycopy(usernameBytes, 0, sikBase, 34, usernameBytes.length); return sikBase; } diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/session/Rakp3.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/session/Rakp3.java index 36a6c75..7f63161 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/session/Rakp3.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/session/Rakp3.java @@ -210,7 +210,7 @@ protected IpmiPayload preparePayload(int sequenceNumber) prepareKeyExchangeAuthenticationCodeBase( rakp1, rakp1ResponseData), - rakp1.getPassword()); + rakp1.getPasswordBytes()); byte[] result = null; @@ -239,11 +239,8 @@ protected IpmiPayload preparePayload(int sequenceNumber) private byte[] prepareKeyExchangeAuthenticationCodeBase( Rakp1 rakp1Message, Rakp1ResponseData responseData) { - int length = 22; - if (rakp1Message.getUsername() != null) { - length += rakp1Message.getUsername().length(); - } - byte[] keac = new byte[length]; + byte[] username = rakp1Message.getUsernameBytes(); + byte[] keac = new byte[22 + username.length]; System .arraycopy( @@ -271,20 +268,8 @@ private byte[] prepareKeyExchangeAuthenticationCodeBase( .getRequestedMaximumPrivilegeLevel()) | 0x10); - if (rakp1Message.getUsername() != null) { - keac[21] = TypeConverter.intToByte(rakp1Message.getUsername().length()); - if (rakp1Message.getUsername().length() > 0) { - System - .arraycopy( - rakp1Message.getUsername().getBytes(), - 0, - keac, - 22, - rakp1Message.getUsername().length()); - } - } else { - keac[21] = 0; - } + keac[21] = TypeConverter.intToByte(username.length); + System.arraycopy(username, 0, keac, 22, username.length); return keac; } diff --git a/src/main/java/org/metricshub/ipmi/core/coding/protocol/decoder/ProtocolDecoder.java b/src/main/java/org/metricshub/ipmi/core/coding/protocol/decoder/ProtocolDecoder.java index 4aff7fe..c79df37 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/protocol/decoder/ProtocolDecoder.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/protocol/decoder/ProtocolDecoder.java @@ -192,6 +192,8 @@ protected static int decodeSessionID(byte[] rawMessage, int offset) { * - {@link ConfidentialityAlgorithm} required to decrypt * payload. * @return Payload decoded into {@link IpmiLanResponse}. + * @throws IllegalArgumentException + * when the payload is empty */ protected IpmiPayload decodePayload( byte[] rawData, @@ -199,14 +201,14 @@ protected IpmiPayload decodePayload( int length, ConfidentialityAlgorithm confidentialityAlgorithm, PayloadType payloadType) { - byte[] payload = null; - if (length > 0) { - payload = new byte[length]; + if (length <= 0) { + throw new IllegalArgumentException("Empty payload"); + } + byte[] payload = new byte[length]; - System.arraycopy(rawData, offset, payload, 0, length); + System.arraycopy(rawData, offset, payload, 0, length); - payload = confidentialityAlgorithm.decrypt(payload); - } + payload = confidentialityAlgorithm.decrypt(payload); if (payloadType == PayloadType.Sol) { return new SolInboundMessage(payload); diff --git a/src/main/java/org/metricshub/ipmi/core/coding/security/AuthenticationAlgorithm.java b/src/main/java/org/metricshub/ipmi/core/coding/security/AuthenticationAlgorithm.java index 81a0ad6..3ceb68d 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/security/AuthenticationAlgorithm.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/security/AuthenticationAlgorithm.java @@ -76,12 +76,12 @@ private AuthenticationAlgorithm(Mac mac) { * @param data - The base for authentication algorithm. Depends on RAKP * Message. * @param key - the Key Exchange Authentication Code to check. - * @param password - password of the user establishing a session + * @param password - password of the user establishing a session, as bytes * @return True if authentication check was successful, false otherwise. * @throws NoSuchAlgorithmException when initiation of the algorithm fails * @throws InvalidKeyException when creating of the algorithm key fails */ - public boolean checkKeyExchangeAuthenticationCode(byte[] data, byte[] key, String password) + public boolean checkKeyExchangeAuthenticationCode(byte[] data, byte[] key, byte[] password) throws NoSuchAlgorithmException, InvalidKeyException { byte[] check = getKeyExchangeAuthenticationCode(data, password); @@ -93,16 +93,15 @@ public boolean checkKeyExchangeAuthenticationCode(byte[] data, byte[] key, Strin * * @param data - The base for authentication algorithm. Depends on RAKP * Message. - * @param password - password of the user establishing a session + * @param key - the password of the user establishing a session, or the BMC key (Kg), as bytes + * @return the Key Exchange Authentication Code * @throws NoSuchAlgorithmException when initiation of the algorithm fails * @throws InvalidKeyException when creating of the algorithm key fails */ - public byte[] getKeyExchangeAuthenticationCode(byte[] data, String password) + public byte[] getKeyExchangeAuthenticationCode(byte[] data, byte[] key) throws NoSuchAlgorithmException, InvalidKeyException { - final byte[] key = password.getBytes(); - SecretKeySpec sKey = new SecretKeySpec(key, getAlgorithmName()); mac.init(sKey); diff --git a/src/main/java/org/metricshub/ipmi/core/coding/security/AuthenticationRakpNone.java b/src/main/java/org/metricshub/ipmi/core/coding/security/AuthenticationRakpNone.java index cfd330d..7b7d2ca 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/security/AuthenticationRakpNone.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/security/AuthenticationRakpNone.java @@ -44,7 +44,7 @@ public byte getCode() { * the RAKP-None algorithm. */ @Override - public boolean checkKeyExchangeAuthenticationCode(byte[] data, byte[] key, String password) { + public boolean checkKeyExchangeAuthenticationCode(byte[] data, byte[] key, byte[] password) { return true; } @@ -53,7 +53,7 @@ public boolean checkKeyExchangeAuthenticationCode(byte[] data, byte[] key, Strin * using the RAKP-None algorithm. */ @Override - public byte[] getKeyExchangeAuthenticationCode(byte[] data, String password) { + public byte[] getKeyExchangeAuthenticationCode(byte[] data, byte[] key) { return new byte[0]; } diff --git a/src/main/java/org/metricshub/ipmi/core/coding/security/ConfidentialityAesCbc128.java b/src/main/java/org/metricshub/ipmi/core/coding/security/ConfidentialityAesCbc128.java index 11798a3..57cbf77 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/security/ConfidentialityAesCbc128.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/security/ConfidentialityAesCbc128.java @@ -39,7 +39,7 @@ */ public class ConfidentialityAesCbc128 extends ConfidentialityAlgorithm { - protected static final byte[] CONST2 = new byte[20]; + private static final byte[] CONST2 = new byte[20]; static { Arrays.fill(CONST2, (byte) 2); } diff --git a/src/main/java/org/metricshub/ipmi/core/coding/security/IntegrityAlgorithm.java b/src/main/java/org/metricshub/ipmi/core/coding/security/IntegrityAlgorithm.java index 949e15b..b739adf 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/security/IntegrityAlgorithm.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/security/IntegrityAlgorithm.java @@ -37,7 +37,7 @@ */ public abstract class IntegrityAlgorithm { - protected static final byte[] CONST1 = new byte[20]; + private static final byte[] CONST1 = new byte[20]; static { Arrays.fill(CONST1, (byte) 1); } diff --git a/src/main/java/org/metricshub/ipmi/core/common/PropertiesManager.java b/src/main/java/org/metricshub/ipmi/core/common/PropertiesManager.java index 84384a4..2962707 100644 --- a/src/main/java/org/metricshub/ipmi/core/common/PropertiesManager.java +++ b/src/main/java/org/metricshub/ipmi/core/common/PropertiesManager.java @@ -22,7 +22,10 @@ * ╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱ */ +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import java.io.IOException; +import java.io.InputStream; import java.util.HashMap; import java.util.Map; import java.util.Properties; @@ -45,6 +48,7 @@ private PropertiesManager() { loadProperties("/vxipmi.properties"); } + @SuppressFBWarnings(value = "MS_EXPOSE_REP", justification = "Singleton: handing out the shared instance is the point") public static PropertiesManager getInstance() { if (instance == null) { instance = new PropertiesManager(); @@ -53,9 +57,9 @@ public static PropertiesManager getInstance() { } private void loadProperties(String name) { - try { + try (InputStream stream = getClass().getResourceAsStream(name)) { Properties props = new Properties(); - props.load(getClass().getResourceAsStream(name)); + props.load(stream); for (Object key : props.keySet()) { this.properties.put(key.toString(), props.getProperty(key.toString())); diff --git a/src/main/java/org/metricshub/ipmi/core/connection/Connection.java b/src/main/java/org/metricshub/ipmi/core/connection/Connection.java index 4f3e81f..6eb1906 100644 --- a/src/main/java/org/metricshub/ipmi/core/connection/Connection.java +++ b/src/main/java/org/metricshub/ipmi/core/connection/Connection.java @@ -94,11 +94,11 @@ public class Connection extends TimerTask implements MachineObserver { /** * Time in ms after which a message times out. */ - private int timeout = -1; - private StateMachineAction lastAction; - private int sessionId; - private int managedSystemSessionId; - private byte[] sik; + private volatile int timeout = -1; + private volatile StateMachineAction lastAction; + private volatile int sessionId; + private volatile int managedSystemSessionId; + private volatile byte[] sik; private int handle; diff --git a/src/main/java/org/metricshub/ipmi/core/connection/ConnectionManager.java b/src/main/java/org/metricshub/ipmi/core/connection/ConnectionManager.java index 63ce1e6..c3b8198 100644 --- a/src/main/java/org/metricshub/ipmi/core/connection/ConnectionManager.java +++ b/src/main/java/org/metricshub/ipmi/core/connection/ConnectionManager.java @@ -34,7 +34,6 @@ import java.net.InetAddress; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; /** * Manages multiple {@link Connection}s @@ -43,7 +42,8 @@ public class ConnectionManager { private Messenger messenger; private List connections; - private static final AtomicInteger SESSIONLESS_TAG = new AtomicInteger(0); + private static final Object SESSIONLESS_TAG_LOCK = new Object(); + private static int sessionlessTag; private static List reservedTags = new ArrayList(); /** @@ -128,34 +128,33 @@ public void close() { * {@link ConnectionManager}. Auto-incremented. */ public static int generateSessionlessTag() { - synchronized (SESSIONLESS_TAG) { + synchronized (SESSIONLESS_TAG_LOCK) { boolean wait = true; // wait(1) clears the interrupt flag when it throws; restore it only once a tag is found, // otherwise every following wait(1) would throw immediately and the loop would hot-spin boolean interrupted = false; while (wait) { - SESSIONLESS_TAG.incrementAndGet(); - SESSIONLESS_TAG.set(SESSIONLESS_TAG.get() % 60); + sessionlessTag = (sessionlessTag + 1) % 60; synchronized (reservedTags) { - if (!reservedTags.contains(SESSIONLESS_TAG.get())) { + if (!reservedTags.contains(sessionlessTag)) { wait = false; } } if (wait) { try { - SESSIONLESS_TAG.wait(1); + SESSIONLESS_TAG_LOCK.wait(1); } catch (InterruptedException e) { interrupted = true; } } } synchronized (reservedTags) { - reservedTags.add(SESSIONLESS_TAG.get()); + reservedTags.add(sessionlessTag); } if (interrupted) { Thread.currentThread().interrupt(); } - return SESSIONLESS_TAG.get(); + return sessionlessTag; } } diff --git a/src/main/java/org/metricshub/ipmi/core/connection/SessionManager.java b/src/main/java/org/metricshub/ipmi/core/connection/SessionManager.java index d076eb2..d74e5d1 100644 --- a/src/main/java/org/metricshub/ipmi/core/connection/SessionManager.java +++ b/src/main/java/org/metricshub/ipmi/core/connection/SessionManager.java @@ -34,6 +34,7 @@ import java.net.InetAddress; import java.util.List; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; /** * Manages multiple {@link Session}s. @@ -42,15 +43,14 @@ public class SessionManager { private static final Logger LOGGER = LoggerFactory.getLogger(SessionManager.class); - private static Integer sessionId = 100; + private static final AtomicInteger SESSION_ID = new AtomicInteger(100); /** * The session ID generated by the {@link SessionManager}. * Auto-incremented. */ - public static synchronized int generateSessionId() { - sessionId %= (Integer.MAX_VALUE / 4); - return sessionId++; + public static int generateSessionId() { + return SESSION_ID.getAndUpdate(id -> (id + 1) % (Integer.MAX_VALUE / 4)); } public static Session establishSession( diff --git a/src/main/java/org/metricshub/ipmi/core/connection/queue/MessageQueue.java b/src/main/java/org/metricshub/ipmi/core/connection/queue/MessageQueue.java index 45566b3..1a27f8c 100644 --- a/src/main/java/org/metricshub/ipmi/core/connection/queue/MessageQueue.java +++ b/src/main/java/org/metricshub/ipmi/core/connection/queue/MessageQueue.java @@ -41,7 +41,7 @@ public class MessageQueue extends TimerTask { private List queue; - private int timeout; + private volatile int timeout; private Timer timer; private Connection connection; private int lastSequenceNumber; diff --git a/src/main/java/org/metricshub/ipmi/core/package-info.java b/src/main/java/org/metricshub/ipmi/core/package-info.java index b985ed2..5010027 100644 --- a/src/main/java/org/metricshub/ipmi/core/package-info.java +++ b/src/main/java/org/metricshub/ipmi/core/package-info.java @@ -4,7 +4,16 @@ * * @see org.metricshub.ipmi.core.api */ +// Applies to all the subpackages. EI_EXPOSE_REP also matches EI_EXPOSE_REP2 +@SuppressFBWarnings(value = { "CT_CONSTRUCTOR_THROW", "EI_EXPOSE_REP" }, justification = "CT_CONSTRUCTOR_THROW: " + + "constructors reject invalid arguments and packets, and no class guards security-sensitive state that a " + + "finalizer attack could exploit. EI_EXPOSE_REP: the protocol core shares buffers, records and " + + "collaborators by reference by design; messages are built and decoded in place, and response data and " + + "records are mutable holders with public setters, so a defensive copy would add allocations without " + + "protecting any invariant") package org.metricshub.ipmi.core; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; /*- * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * IPMI Java Client diff --git a/src/main/java/org/metricshub/ipmi/core/transport/UdpMessenger.java b/src/main/java/org/metricshub/ipmi/core/transport/UdpMessenger.java index 4c08990..59f7221 100644 --- a/src/main/java/org/metricshub/ipmi/core/transport/UdpMessenger.java +++ b/src/main/java/org/metricshub/ipmi/core/transport/UdpMessenger.java @@ -45,13 +45,13 @@ public class UdpMessenger extends Thread implements Messenger { private List listeners; - private boolean closing = false; + private volatile boolean closing = false; /** * Size of the message data buffer. Default * {@link UdpMessenger#DEFAULTBUFFERSIZE}. */ - private int bufferSize; + private volatile int bufferSize; private static final int DEFAULTBUFFERSIZE = 512; @@ -89,7 +89,6 @@ public UdpMessenger(int port) throws SocketException, UnknownHostException { * bind to the specified local port. */ public UdpMessenger(int port, InetAddress address) throws SocketException { - sentPackets = 0; this.port = port; listeners = new ArrayList(); bufferSize = DEFAULTBUFFERSIZE; @@ -117,7 +116,8 @@ public void run() { boolean run = true; while (run) { - DatagramPacket response = new DatagramPacket(new byte[512], 512); + int size = bufferSize; + DatagramPacket response = new DatagramPacket(new byte[size], size); try { socket.receive(response); @@ -195,16 +195,6 @@ public void unregister(UdpListener listener) { } } - private static int sentPackets = 0; - - /** - * Returns number of packets sent since last creation of the instance of - * {@link UdpMessenger}. For debug/testing purposes only. - */ - public static int getSentPackets() { - return sentPackets; - } - /** * Sends {@link UdpMessage}. * @@ -225,6 +215,5 @@ public synchronized void send(UdpMessage message) throws IOException { } catch (InterruptedException e) { currentThread().interrupt(); } - ++sentPackets; } } diff --git a/src/site/markdown/configuration.md b/src/site/markdown/configuration.md index 9787595..f3f35ad 100644 --- a/src/site/markdown/configuration.md +++ b/src/site/markdown/configuration.md @@ -50,9 +50,10 @@ chosen by the operating system for each session. ### Credentials `username` and `password` are the IPMI account of the BMC: see -[Preparing the BMC](preparing-the-bmc.html#creating-the-account). The password is a `char[]`; -the library converts it to a `String` internally to open the session and does not clear the -array, so clear it yourself once you no longer need the configuration. +[Preparing the BMC](preparing-the-bmc.html#creating-the-account). Both are sent to the BMC +encoded in UTF-8, whatever the platform charset. The password is a `char[]`; the library converts +it to a `String` internally to open the session and does not clear the array, so clear it +yourself once you no longer need the configuration. The client opens every session with the **User** privilege level, which is enough for every `IpmiClient` method. diff --git a/src/site/markdown/upgrading.md b/src/site/markdown/upgrading.md index 6fc0071..d601c57 100644 --- a/src/site/markdown/upgrading.md +++ b/src/site/markdown/upgrading.md @@ -1,4 +1,4 @@ -keywords: upgrade, migration, release notes, breaking changes, protected fields, accessors, abstractsensorrecord, validateresponse, org.sentrysoftware +keywords: upgrade, migration, release notes, breaking changes, protected fields, accessors, abstractsensorrecord, validateresponse, utf-8, bmc key, org.sentrysoftware description: What changes when upgrading the IPMI Java Client — from 1.2.02, from 1.2.01, and from the org.sentrysoftware:ipmi artifact of 1.2.00 and earlier. # Upgrading @@ -13,7 +13,13 @@ The `IpmiClient` API is unchanged, and the client is more tolerant of real-world records (`C0h` – `FFh`) are decoded as `OemRecord`, and any record that cannot be decoded is logged and skipped, where 1.2.02 returned no sensors and no FRUs at all ([OEM and unknown records](supported-commands.html#oem-and-unknown-records)); -* a BMC that answers a whole-record Get SDR with a truncated record is read again in chunks. +* a BMC that answers a whole-record Get SDR with a truncated record is read again in chunks; +* the user name and password are encoded in UTF-8 whatever the platform charset (1.2.02 used the + platform charset, UTF-8 by default only since Java 18), and the + [BMC key](configuration.html#bmc-key) is used as raw bytes: a key with bytes `80h` or above no + longer gets corrupted into a wrong session key; +* the user name is limited to 16 bytes once encoded, as the IPMI specification requires, instead of + 16 characters. Code that **extends** the library's protocol classes needs the changes below. @@ -39,6 +45,15 @@ with the new `protected` accessors: `MessageComposer` are now `final` (they only had private constructors, so they could not be subclassed anyway). +### Credentials and transport + +| Class | Change | +| --- | --- | +| `AuthenticationAlgorithm` | `getKeyExchangeAuthenticationCode(byte[] data, byte[] key)` and `checkKeyExchangeAuthenticationCode(byte[] data, byte[] key, byte[] password)` take the key and the password as bytes instead of a `String` | +| `IntegrityAlgorithm`, `ConfidentialityAesCbc128` | The `protected` constants `CONST1` and `CONST2` are now `private` | +| `UdpMessenger` | `getSentPackets()`, a debug counter, is removed; `setBufferSize(int)` now sets the size of the receive buffer, which was always 512 bytes | +| `ProtocolDecoder` | `decodePayload(...)` throws `IllegalArgumentException` on an empty payload instead of a `NullPointerException` | + ### Sensor records `FullSensorRecord`, `CompactSensorRecord` and `EventOnlyRecord` now extend the new diff --git a/src/test/java/org/metricshub/ipmi/core/coding/commands/session/Rakp1Test.java b/src/test/java/org/metricshub/ipmi/core/coding/commands/session/Rakp1Test.java new file mode 100644 index 0000000..16342bc --- /dev/null +++ b/src/test/java/org/metricshub/ipmi/core/coding/commands/session/Rakp1Test.java @@ -0,0 +1,63 @@ +package org.metricshub.ipmi.core.coding.commands.session; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; + +import java.io.ByteArrayOutputStream; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; + +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; + +import org.junit.jupiter.api.Test; +import org.metricshub.ipmi.core.coding.commands.PrivilegeLevel; +import org.metricshub.ipmi.core.coding.security.CipherSuite; +import org.metricshub.ipmi.core.coding.security.SecurityConstants; + +class Rakp1Test { + + private static final CipherSuite SUITE_3 = new CipherSuite( + (byte) 3, + SecurityConstants.AA_RAKP_HMAC_SHA1, + SecurityConstants.CA_AES_CBC128, + SecurityConstants.IA_HMAC_SHA1_96); + + /** + * SIK = HMAC-SHA1 keyed with Kg (or the password) over Rc | Rm | RoleM | ULengthM | UNameM (IPMI 2.0 section + * 13.31), computed independently of the library. + */ + private static byte[] expectedSik(Rakp1 rakp1, byte[] managedSystemRandom, String username, byte[] key) + throws Exception { + ByteArrayOutputStream base = new ByteArrayOutputStream(); + base.write(rakp1.getConsoleRandomNumber()); + base.write(managedSystemRandom); + base.write(0x12); // User privilege level (2h), name-only lookup (10h) + byte[] name = username.getBytes(StandardCharsets.UTF_8); + base.write(name.length); + base.write(name); + Mac mac = Mac.getInstance("HmacSHA1"); + mac.init(new SecretKeySpec(key, "HmacSHA1")); + return mac.doFinal(base.toByteArray()); + } + + @Test + void calculateSikUsesRawBmcKeyAndUtf8Credentials() throws Exception { + Rakp1ResponseData rakp2 = new Rakp1ResponseData(); + byte[] managedSystemRandom = new byte[16]; + Arrays.fill(managedSystemRandom, (byte) 0xa5); + rakp2.setManagedSystemRandomNumber(managedSystemRandom); + String username = "usér"; + + // Kg bytes of 80h and above used to go through new String(key).getBytes() and come out mangled + byte[] bmcKey = { (byte) 0x80, (byte) 0xff, 0x01, (byte) 0xc3 }; + Rakp1 twoKey = new Rakp1(0x1234, PrivilegeLevel.User, username, "password", bmcKey, SUITE_3); + assertArrayEquals(expectedSik(twoKey, managedSystemRandom, username, bmcKey), twoKey.calculateSik(rakp2)); + + // without Kg, the key is the password, in UTF-8 whatever the platform charset + String password = "pässwörd"; + Rakp1 oneKey = new Rakp1(0x1234, PrivilegeLevel.User, username, password, null, SUITE_3); + assertArrayEquals( + expectedSik(oneKey, managedSystemRandom, username, password.getBytes(StandardCharsets.UTF_8)), + oneKey.calculateSik(rakp2)); + } +} From d0e7e6d45ad7f41da76821494ca4797c2e72e98a Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Thu, 8 Oct 2026 14:46:33 +0200 Subject: [PATCH 2/2] Keep the 512-byte receive buffer, list the API breaks in the README (#116) - UdpMessenger.run(): back to the fixed 512-byte receive buffer of main. Sizing it from setBufferSize() raced with the receive thread started by the constructor (the first datagram used the old size); making the setter effective is out of the scope of the SpotBugs clean-up, and the volatile field still fixes the stale write SpotBugs reported - README: the 1.2.03 upgrade summary mentions the UTF-8 credentials, the byte[] AuthenticationAlgorithm methods, the removed UdpMessenger.getSentPackets() and the private CONST1/CONST2 Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- .../java/org/metricshub/ipmi/core/transport/UdpMessenger.java | 3 +-- src/site/markdown/upgrading.md | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 29ba63a..6b86d0d 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ The BMC must have IPMI over LAN enabled and an account with the User privilege: ## Upgrading -Version 1.2.03 makes the `protected` fields of the protocol classes (`AbstractIpmiRunner`, `MessageHandler`, `IpmiLanMessage`, `ConfidentialityAlgorithm`, `IntegrityAlgorithm`) `private`. Subclasses must use the new `protected` accessors instead; see [Upgrading from 1.2.02](https://metricshub.org/ipmi-java/upgrading.html#upgrading-from-1-2-02) for the list. The `IpmiClient` API is unchanged. The Full, Compact and Event-Only sensor records now share the `AbstractSensorRecord` superclass, and commands can check responses with `IpmiCommandCoder.validateResponse()`; both are described on the same page. +Version 1.2.03 makes the `protected` fields of the protocol classes (`AbstractIpmiRunner`, `MessageHandler`, `IpmiLanMessage`, `ConfidentialityAlgorithm`, `IntegrityAlgorithm`) `private`. Subclasses must use the new `protected` accessors instead; see [Upgrading from 1.2.02](https://metricshub.org/ipmi-java/upgrading.html#upgrading-from-1-2-02) for the list. The `IpmiClient` API is unchanged. The Full, Compact and Event-Only sensor records now share the `AbstractSensorRecord` superclass, and commands can check responses with `IpmiCommandCoder.validateResponse()`; both are described on the same page. The user name and password are now encoded in UTF-8 whatever the platform charset, and the BMC key is used as raw bytes; as a result, `AuthenticationAlgorithm.getKeyExchangeAuthenticationCode()` and `checkKeyExchangeAuthenticationCode()` take the key and password as `byte[]` instead of `String`. `UdpMessenger.getSentPackets()` is removed, and the `CONST1`/`CONST2` constants of `IntegrityAlgorithm` and `ConfidentialityAesCbc128` are now `private`. ## Build instructions diff --git a/src/main/java/org/metricshub/ipmi/core/transport/UdpMessenger.java b/src/main/java/org/metricshub/ipmi/core/transport/UdpMessenger.java index 59f7221..b59b93c 100644 --- a/src/main/java/org/metricshub/ipmi/core/transport/UdpMessenger.java +++ b/src/main/java/org/metricshub/ipmi/core/transport/UdpMessenger.java @@ -116,8 +116,7 @@ public void run() { boolean run = true; while (run) { - int size = bufferSize; - DatagramPacket response = new DatagramPacket(new byte[size], size); + DatagramPacket response = new DatagramPacket(new byte[512], 512); try { socket.receive(response); diff --git a/src/site/markdown/upgrading.md b/src/site/markdown/upgrading.md index d601c57..a5a40b0 100644 --- a/src/site/markdown/upgrading.md +++ b/src/site/markdown/upgrading.md @@ -51,7 +51,7 @@ subclassed anyway). | --- | --- | | `AuthenticationAlgorithm` | `getKeyExchangeAuthenticationCode(byte[] data, byte[] key)` and `checkKeyExchangeAuthenticationCode(byte[] data, byte[] key, byte[] password)` take the key and the password as bytes instead of a `String` | | `IntegrityAlgorithm`, `ConfidentialityAesCbc128` | The `protected` constants `CONST1` and `CONST2` are now `private` | -| `UdpMessenger` | `getSentPackets()`, a debug counter, is removed; `setBufferSize(int)` now sets the size of the receive buffer, which was always 512 bytes | +| `UdpMessenger` | `getSentPackets()`, a debug counter, is removed | | `ProtocolDecoder` | `decodePayload(...)` throws `IllegalArgumentException` on an empty payload instead of a `NullPointerException` | ### Sensor records