diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 4b1e82997..3206d8a70 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -8,6 +8,7 @@ on: branches: [ master ] permissions: + contents: write packages: write jobs: diff --git a/CHANGES.md b/CHANGES.md index 8da353402..a50fc7fae 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,8 @@ +# HAP-Java 2.0.9 +* Fix Set Duration and Remaining Duration characteristics to advertise format `uint32` instead of `int` [#202](https://github.com/hap-java/HAP-Java/issues/202) +* Stop logging the SRP shared secret, derived key and M5 message data during pairing at DEBUG level +* Fix splitting of encrypted frames when several frames of different length arrive in one read, or when a read ends after the first length byte + # HAP-Java 2.0.8 * Updated bouncy castle to 1.83 * Updated netty to 4.1.126.Final diff --git a/pom.xml b/pom.xml index 549efbad5..d476d8e49 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ hap hap-java Homekit Accessory Protocol for Java - 2.0.8 + 2.0.9 jar https://github.com/hap-java/HAP-Java @@ -26,7 +26,7 @@ scm:git:https://github.com/hap-java/HAP-Java.git scm:git:https://github.com/hap-java/HAP-Java.git https://github.com/hap-java/HAP-Java.git - hap-2.0.8 + hap-2.0.9 @@ -162,7 +162,7 @@ org.assertj assertj-core - 3.19.0 + 3.27.7 test diff --git a/src/main/java/io/github/hapjava/characteristics/impl/base/IntegerCharacteristic.java b/src/main/java/io/github/hapjava/characteristics/impl/base/IntegerCharacteristic.java index 96448eea8..6b6cd2982 100644 --- a/src/main/java/io/github/hapjava/characteristics/impl/base/IntegerCharacteristic.java +++ b/src/main/java/io/github/hapjava/characteristics/impl/base/IntegerCharacteristic.java @@ -47,8 +47,55 @@ public IntegerCharacteristic( Optional> setter, Optional> subscriber, Optional unsubscriber) { + this( + type, + "int", + description, + minValue, + maxValue, + unit, + getter, + setter, + subscriber, + unsubscriber); + } + + /** + * Constructor for characteristics whose HAP format differs from the default "int", e.g. "uint8", + * "uint16" or "uint32". Values are still handled as Java {@link Integer}, so the supported range + * must fit into a signed 32-bit integer. + * + * @param type a string containing a UUID that indicates the type of characteristic. Apple defines + * a set of these, however implementors can create their own as well. + * @param format the HAP format to advertise, e.g. "int" or "uint32". + * @param description a description of the characteristic to be passed to the consuming device. + * @param minValue the minimum supported value. + * @param maxValue the maximum supported value + * @param unit a description of the unit this characteristic supports. + * @param getter getter for value + * @param setter setter for value + * @param subscriber subscribers to changes + * @param unsubscriber unsubscribers to changes + */ + protected IntegerCharacteristic( + String type, + String format, + String description, + int minValue, + int maxValue, + String unit, + Optional>> getter, + Optional> setter, + Optional> subscriber, + Optional unsubscriber) { super( - type, "int", description, getter.isPresent(), setter.isPresent(), subscriber, unsubscriber); + type, + format, + description, + getter.isPresent(), + setter.isPresent(), + subscriber, + unsubscriber); this.minValue = minValue; this.maxValue = maxValue; this.unit = unit; diff --git a/src/main/java/io/github/hapjava/characteristics/impl/valve/RemainingDurationCharacteristic.java b/src/main/java/io/github/hapjava/characteristics/impl/valve/RemainingDurationCharacteristic.java index 828fafab1..29409027a 100644 --- a/src/main/java/io/github/hapjava/characteristics/impl/valve/RemainingDurationCharacteristic.java +++ b/src/main/java/io/github/hapjava/characteristics/impl/valve/RemainingDurationCharacteristic.java @@ -26,6 +26,7 @@ public RemainingDurationCharacteristic( Runnable unsubscriber) { super( "000000D4-0000-1000-8000-0026BB765291", + "uint32", "remaining duration", minValue, maxValue, diff --git a/src/main/java/io/github/hapjava/characteristics/impl/valve/SetDurationCharacteristic.java b/src/main/java/io/github/hapjava/characteristics/impl/valve/SetDurationCharacteristic.java index 59cef5646..109a7bb6e 100644 --- a/src/main/java/io/github/hapjava/characteristics/impl/valve/SetDurationCharacteristic.java +++ b/src/main/java/io/github/hapjava/characteristics/impl/valve/SetDurationCharacteristic.java @@ -24,6 +24,7 @@ public SetDurationCharacteristic( Runnable unsubscriber) { super( "000000D3-0000-1000-8000-0026BB765291", + "uint32", "set duration", minValue, maxValue, diff --git a/src/main/java/io/github/hapjava/server/impl/connections/LengthPrefixedByteArrayProcessor.java b/src/main/java/io/github/hapjava/server/impl/connections/LengthPrefixedByteArrayProcessor.java index 124acbb51..34ec3a41c 100644 --- a/src/main/java/io/github/hapjava/server/impl/connections/LengthPrefixedByteArrayProcessor.java +++ b/src/main/java/io/github/hapjava/server/impl/connections/LengthPrefixedByteArrayProcessor.java @@ -21,9 +21,9 @@ public synchronized Collection handle(byte[] data) { int pos = 0; LOGGER.trace( "Received message of length {}. Existing buffer is {}", data.length, buffer.size()); - if (buffer.size() == 0) { + if (buffer.size() == 0 && firstLengthByteBuffer == null) { while (data.length - pos > 18) { - int targetLength = (data[0] & 0xFF) + (data[1] & 0xFF) * 256 + 16 + 2; + int targetLength = (data[pos] & 0xFF) + (data[pos + 1] & 0xFF) * 256 + 16 + 2; LOGGER.trace("Attempting to read message of length {}", targetLength); if (data.length >= pos + targetLength) { byte[] b = new byte[targetLength - 2]; @@ -56,6 +56,7 @@ private void step(byte[] data, int pos, Collection results) { if (targetLength == 0) { if (firstLengthByteBuffer != null) { targetLength = (firstLengthByteBuffer & 0xFF) + (data[pos] & 0xFF) * 256 + 16; + firstLengthByteBuffer = null; pos += 1; LOGGER.trace( "Received the second byte after storing the first byte. New length is {}", diff --git a/src/main/java/io/github/hapjava/server/impl/pairing/ExchangeHandler.java b/src/main/java/io/github/hapjava/server/impl/pairing/ExchangeHandler.java index 4b7c3eb11..6bae3ab84 100755 --- a/src/main/java/io/github/hapjava/server/impl/pairing/ExchangeHandler.java +++ b/src/main/java/io/github/hapjava/server/impl/pairing/ExchangeHandler.java @@ -31,8 +31,6 @@ public ExchangeHandler(byte[] k, HomekitAuthInfo authInfo) { } public HttpResponse handle(PairSetupRequest req) throws Exception { - LOGGER.debug("ExchangeHandler: Starting M5 exchange with shared secret K: {}", bytesToHex(k)); - HKDFBytesGenerator hkdf = new HKDFBytesGenerator(new SHA512Digest()); hkdf.init( new HKDFParameters( @@ -42,15 +40,10 @@ public HttpResponse handle(PairSetupRequest req) throws Exception { byte[] okm = hkdf_enc_key = new byte[32]; hkdf.generateBytes(okm, 0, 32); - LOGGER.debug("ExchangeHandler: HKDF encryption key: {}", bytesToHex(okm)); - return decrypt((ExchangeRequest) req, okm); } private HttpResponse decrypt(ExchangeRequest req, byte[] key) throws Exception { - LOGGER.debug("ExchangeHandler: Received AuthTag: {}", bytesToHex(req.getAuthTagData())); - LOGGER.debug("ExchangeHandler: Received MessageData: {}", bytesToHex(req.getMessageData())); - try { ChachaDecoder chacha = new ChachaDecoder(key, "PS-Msg05".getBytes(StandardCharsets.UTF_8)); byte[] plaintext = chacha.decodeCiphertext(req.getAuthTagData(), req.getMessageData()); diff --git a/src/test/java/io/github/hapjava/characteristics/impl/valve/DurationCharacteristicFormatTest.java b/src/test/java/io/github/hapjava/characteristics/impl/valve/DurationCharacteristicFormatTest.java new file mode 100644 index 000000000..2e12b2daa --- /dev/null +++ b/src/test/java/io/github/hapjava/characteristics/impl/valve/DurationCharacteristicFormatTest.java @@ -0,0 +1,56 @@ +package io.github.hapjava.characteristics.impl.valve; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.github.hapjava.characteristics.impl.windowcovering.CurrentPositionCharacteristic; +import java.util.concurrent.CompletableFuture; +import javax.json.JsonObject; +import org.junit.Test; + +/** Regression tests for https://github.com/hap-java/HAP-Java/issues/202 */ +public class DurationCharacteristicFormatTest { + + @Test + public void setDurationAdvertisesUint32() { + SetDurationCharacteristic characteristic = + new SetDurationCharacteristic( + () -> CompletableFuture.completedFuture(300), value -> {}, callback -> {}, () -> {}); + + JsonObject json = characteristic.toJson(12).join(); + + assertThat(json.getString("type")).isEqualTo("D3"); + assertThat(json.getString("format")).isEqualTo("uint32"); + assertThat(json.getInt("value")).isEqualTo(300); + assertThat(json.getInt("minValue")).isEqualTo(0); + assertThat(json.getInt("maxValue")).isEqualTo(3600); + assertThat(json.getInt("minStep")).isEqualTo(1); + assertThat(json.getString("unit")).isEqualTo("s"); + } + + @Test + public void remainingDurationAdvertisesUint32() { + RemainingDurationCharacteristic characteristic = + new RemainingDurationCharacteristic( + () -> CompletableFuture.completedFuture(180), callback -> {}, () -> {}); + + JsonObject json = characteristic.toJson(13).join(); + + assertThat(json.getString("type")).isEqualTo("D4"); + assertThat(json.getString("format")).isEqualTo("uint32"); + assertThat(json.getInt("value")).isEqualTo(180); + assertThat(json.getInt("minValue")).isEqualTo(0); + assertThat(json.getInt("maxValue")).isEqualTo(3600); + assertThat(json.getString("unit")).isEqualTo("s"); + } + + @Test + public void otherIntegerCharacteristicsKeepIntFormat() { + CurrentPositionCharacteristic characteristic = + new CurrentPositionCharacteristic( + () -> CompletableFuture.completedFuture(50), callback -> {}, () -> {}); + + JsonObject json = characteristic.toJson(14).join(); + + assertThat(json.getString("format")).isEqualTo("int"); + } +} diff --git a/src/test/java/io/github/hapjava/server/impl/connections/LengthPrefixedByteArrayProcessorTest.java b/src/test/java/io/github/hapjava/server/impl/connections/LengthPrefixedByteArrayProcessorTest.java new file mode 100644 index 000000000..9e839848d --- /dev/null +++ b/src/test/java/io/github/hapjava/server/impl/connections/LengthPrefixedByteArrayProcessorTest.java @@ -0,0 +1,125 @@ +package io.github.hapjava.server.impl.connections; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.ByteArrayOutputStream; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import org.junit.Test; + +public class LengthPrefixedByteArrayProcessorTest { + + private static final int TAG_LENGTH = 16; + + /** + * Builds a frame as it arrives on the wire: 2-byte little-endian payload length, payload, 16-byte + * tag. The payload and tag bytes start at 0x80 so that they never look like a short length. + */ + private static byte[] frame(int payloadLength, int seed) { + byte[] frame = new byte[2 + payloadLength + TAG_LENGTH]; + frame[0] = (byte) (payloadLength & 0xFF); + frame[1] = (byte) ((payloadLength >> 8) & 0xFF); + for (int i = 2; i < frame.length; i++) { + frame[i] = (byte) (0x80 + seed + i); + } + return frame; + } + + /** What the processor is expected to emit for a frame: payload + tag, without the length. */ + private static byte[] body(byte[] frame) { + return Arrays.copyOfRange(frame, 2, frame.length); + } + + private static byte[] concat(byte[]... parts) { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + for (byte[] part : parts) { + out.write(part, 0, part.length); + } + return out.toByteArray(); + } + + private static List feed(LengthPrefixedByteArrayProcessor processor, byte[]... reads) { + List results = new ArrayList<>(); + for (byte[] read : reads) { + results.addAll(processor.handle(read)); + } + return results; + } + + private static List sizes(Collection results) { + List sizes = new ArrayList<>(); + for (byte[] result : results) { + sizes.add(result.length); + } + return sizes; + } + + private static void assertBodies(List results, byte[]... frames) { + List expectedSizes = new ArrayList<>(); + for (byte[] frame : frames) { + expectedSizes.add(frame.length - 2); + } + assertThat(sizes(results)).isEqualTo(expectedSizes); + for (int i = 0; i < frames.length; i++) { + assertThat(results.get(i)).as("frame %d", i).isEqualTo(body(frames[i])); + } + } + + @Test + public void singleFrameInOneRead() { + byte[] a = frame(100, 1); + assertBodies(feed(new LengthPrefixedByteArrayProcessor(), a), a); + } + + @Test + public void threeFramesInOneRead() { + byte[] a = frame(1024, 1); + byte[] b = frame(1024, 2); + byte[] c = frame(200, 3); + assertBodies(feed(new LengthPrefixedByteArrayProcessor(), concat(a, b, c)), a, b, c); + } + + @Test + public void frameSplitAcrossTwoReads() { + byte[] a = frame(1024, 1); + int[] offsets = {1, 2, 3, 10, 18, 19, 20, 500, a.length - 1}; + for (int offset : offsets) { + byte[] first = Arrays.copyOfRange(a, 0, offset); + byte[] second = Arrays.copyOfRange(a, offset, a.length); + List results = feed(new LengthPrefixedByteArrayProcessor(), first, second); + assertThat(sizes(results)).as("split at %d", offset).containsExactly(a.length - 2); + assertThat(results.get(0)).as("split at %d", offset).isEqualTo(body(a)); + } + } + + @Test + public void frameSplitWithNextFrameInSecondRead() { + byte[] a = frame(1024, 1); + byte[] b = frame(200, 2); + byte[] first = Arrays.copyOfRange(a, 0, 500); + byte[] second = concat(Arrays.copyOfRange(a, 500, a.length), b); + assertBodies(feed(new LengthPrefixedByteArrayProcessor(), first, second), a, b); + } + + @Test + public void shortFrameFollowedByLongerFrameInOneRead() { + byte[] a = frame(5, 1); + byte[] b = frame(100, 2); + List results = feed(new LengthPrefixedByteArrayProcessor(), concat(a, b)); + assertThat(sizes(results)).containsExactly(21, 116); + assertBodies(results, a, b); + } + + @Test + public void singleLengthByteThenRestThenAnotherFrame() { + byte[] a = frame(5, 1); + byte[] b = frame(40, 2); + byte[] first = Arrays.copyOfRange(a, 0, 1); + byte[] second = concat(Arrays.copyOfRange(a, 1, a.length), b); + List results = feed(new LengthPrefixedByteArrayProcessor(), first, second); + assertThat(sizes(results)).containsExactly(21, 56); + assertBodies(results, a, b); + } +}