diff --git a/CHANGES.md b/CHANGES.md index 4997cae9d..a50fc7fae 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,7 @@ # 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 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/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); + } +}