Skip to content

Commit 9c5bc6f

Browse files
authored
Merge pull request #204 from hap-java/claude/beautiful-ritchie-89e405
Stop logging pairing secrets; fix frame splitting in LengthPrefixedByteArrayProcessor
2 parents 3f5f58a + bf59554 commit 9c5bc6f

4 files changed

Lines changed: 130 additions & 9 deletions

File tree

‎CHANGES.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# HAP-Java 2.0.9
22
* Fix Set Duration and Remaining Duration characteristics to advertise format `uint32` instead of `int` [#202](https://github.com/hap-java/HAP-Java/issues/202)
3+
* Stop logging the SRP shared secret, derived key and M5 message data during pairing at DEBUG level
4+
* 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
35

46
# HAP-Java 2.0.8
57
* Updated bouncy castle to 1.83

‎src/main/java/io/github/hapjava/server/impl/connections/LengthPrefixedByteArrayProcessor.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ public synchronized Collection<byte[]> handle(byte[] data) {
2121
int pos = 0;
2222
LOGGER.trace(
2323
"Received message of length {}. Existing buffer is {}", data.length, buffer.size());
24-
if (buffer.size() == 0) {
24+
if (buffer.size() == 0 && firstLengthByteBuffer == null) {
2525
while (data.length - pos > 18) {
26-
int targetLength = (data[0] & 0xFF) + (data[1] & 0xFF) * 256 + 16 + 2;
26+
int targetLength = (data[pos] & 0xFF) + (data[pos + 1] & 0xFF) * 256 + 16 + 2;
2727
LOGGER.trace("Attempting to read message of length {}", targetLength);
2828
if (data.length >= pos + targetLength) {
2929
byte[] b = new byte[targetLength - 2];
@@ -56,6 +56,7 @@ private void step(byte[] data, int pos, Collection<byte[]> results) {
5656
if (targetLength == 0) {
5757
if (firstLengthByteBuffer != null) {
5858
targetLength = (firstLengthByteBuffer & 0xFF) + (data[pos] & 0xFF) * 256 + 16;
59+
firstLengthByteBuffer = null;
5960
pos += 1;
6061
LOGGER.trace(
6162
"Received the second byte after storing the first byte. New length is {}",

‎src/main/java/io/github/hapjava/server/impl/pairing/ExchangeHandler.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@ public ExchangeHandler(byte[] k, HomekitAuthInfo authInfo) {
3131
}
3232

3333
public HttpResponse handle(PairSetupRequest req) throws Exception {
34-
LOGGER.debug("ExchangeHandler: Starting M5 exchange with shared secret K: {}", bytesToHex(k));
35-
3634
HKDFBytesGenerator hkdf = new HKDFBytesGenerator(new SHA512Digest());
3735
hkdf.init(
3836
new HKDFParameters(
@@ -42,15 +40,10 @@ public HttpResponse handle(PairSetupRequest req) throws Exception {
4240
byte[] okm = hkdf_enc_key = new byte[32];
4341
hkdf.generateBytes(okm, 0, 32);
4442

45-
LOGGER.debug("ExchangeHandler: HKDF encryption key: {}", bytesToHex(okm));
46-
4743
return decrypt((ExchangeRequest) req, okm);
4844
}
4945

5046
private HttpResponse decrypt(ExchangeRequest req, byte[] key) throws Exception {
51-
LOGGER.debug("ExchangeHandler: Received AuthTag: {}", bytesToHex(req.getAuthTagData()));
52-
LOGGER.debug("ExchangeHandler: Received MessageData: {}", bytesToHex(req.getMessageData()));
53-
5447
try {
5548
ChachaDecoder chacha = new ChachaDecoder(key, "PS-Msg05".getBytes(StandardCharsets.UTF_8));
5649
byte[] plaintext = chacha.decodeCiphertext(req.getAuthTagData(), req.getMessageData());
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package io.github.hapjava.server.impl.connections;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import java.io.ByteArrayOutputStream;
6+
import java.util.ArrayList;
7+
import java.util.Arrays;
8+
import java.util.Collection;
9+
import java.util.List;
10+
import org.junit.Test;
11+
12+
public class LengthPrefixedByteArrayProcessorTest {
13+
14+
private static final int TAG_LENGTH = 16;
15+
16+
/**
17+
* Builds a frame as it arrives on the wire: 2-byte little-endian payload length, payload, 16-byte
18+
* tag. The payload and tag bytes start at 0x80 so that they never look like a short length.
19+
*/
20+
private static byte[] frame(int payloadLength, int seed) {
21+
byte[] frame = new byte[2 + payloadLength + TAG_LENGTH];
22+
frame[0] = (byte) (payloadLength & 0xFF);
23+
frame[1] = (byte) ((payloadLength >> 8) & 0xFF);
24+
for (int i = 2; i < frame.length; i++) {
25+
frame[i] = (byte) (0x80 + seed + i);
26+
}
27+
return frame;
28+
}
29+
30+
/** What the processor is expected to emit for a frame: payload + tag, without the length. */
31+
private static byte[] body(byte[] frame) {
32+
return Arrays.copyOfRange(frame, 2, frame.length);
33+
}
34+
35+
private static byte[] concat(byte[]... parts) {
36+
ByteArrayOutputStream out = new ByteArrayOutputStream();
37+
for (byte[] part : parts) {
38+
out.write(part, 0, part.length);
39+
}
40+
return out.toByteArray();
41+
}
42+
43+
private static List<byte[]> feed(LengthPrefixedByteArrayProcessor processor, byte[]... reads) {
44+
List<byte[]> results = new ArrayList<>();
45+
for (byte[] read : reads) {
46+
results.addAll(processor.handle(read));
47+
}
48+
return results;
49+
}
50+
51+
private static List<Integer> sizes(Collection<byte[]> results) {
52+
List<Integer> sizes = new ArrayList<>();
53+
for (byte[] result : results) {
54+
sizes.add(result.length);
55+
}
56+
return sizes;
57+
}
58+
59+
private static void assertBodies(List<byte[]> results, byte[]... frames) {
60+
List<Integer> expectedSizes = new ArrayList<>();
61+
for (byte[] frame : frames) {
62+
expectedSizes.add(frame.length - 2);
63+
}
64+
assertThat(sizes(results)).isEqualTo(expectedSizes);
65+
for (int i = 0; i < frames.length; i++) {
66+
assertThat(results.get(i)).as("frame %d", i).isEqualTo(body(frames[i]));
67+
}
68+
}
69+
70+
@Test
71+
public void singleFrameInOneRead() {
72+
byte[] a = frame(100, 1);
73+
assertBodies(feed(new LengthPrefixedByteArrayProcessor(), a), a);
74+
}
75+
76+
@Test
77+
public void threeFramesInOneRead() {
78+
byte[] a = frame(1024, 1);
79+
byte[] b = frame(1024, 2);
80+
byte[] c = frame(200, 3);
81+
assertBodies(feed(new LengthPrefixedByteArrayProcessor(), concat(a, b, c)), a, b, c);
82+
}
83+
84+
@Test
85+
public void frameSplitAcrossTwoReads() {
86+
byte[] a = frame(1024, 1);
87+
int[] offsets = {1, 2, 3, 10, 18, 19, 20, 500, a.length - 1};
88+
for (int offset : offsets) {
89+
byte[] first = Arrays.copyOfRange(a, 0, offset);
90+
byte[] second = Arrays.copyOfRange(a, offset, a.length);
91+
List<byte[]> results = feed(new LengthPrefixedByteArrayProcessor(), first, second);
92+
assertThat(sizes(results)).as("split at %d", offset).containsExactly(a.length - 2);
93+
assertThat(results.get(0)).as("split at %d", offset).isEqualTo(body(a));
94+
}
95+
}
96+
97+
@Test
98+
public void frameSplitWithNextFrameInSecondRead() {
99+
byte[] a = frame(1024, 1);
100+
byte[] b = frame(200, 2);
101+
byte[] first = Arrays.copyOfRange(a, 0, 500);
102+
byte[] second = concat(Arrays.copyOfRange(a, 500, a.length), b);
103+
assertBodies(feed(new LengthPrefixedByteArrayProcessor(), first, second), a, b);
104+
}
105+
106+
@Test
107+
public void shortFrameFollowedByLongerFrameInOneRead() {
108+
byte[] a = frame(5, 1);
109+
byte[] b = frame(100, 2);
110+
List<byte[]> results = feed(new LengthPrefixedByteArrayProcessor(), concat(a, b));
111+
assertThat(sizes(results)).containsExactly(21, 116);
112+
assertBodies(results, a, b);
113+
}
114+
115+
@Test
116+
public void singleLengthByteThenRestThenAnotherFrame() {
117+
byte[] a = frame(5, 1);
118+
byte[] b = frame(40, 2);
119+
byte[] first = Arrays.copyOfRange(a, 0, 1);
120+
byte[] second = concat(Arrays.copyOfRange(a, 1, a.length), b);
121+
List<byte[]> results = feed(new LengthPrefixedByteArrayProcessor(), first, second);
122+
assertThat(sizes(results)).containsExactly(21, 56);
123+
assertBodies(results, a, b);
124+
}
125+
}

0 commit comments

Comments
 (0)