Closed gbrooker closed 5 years ago
The decryption is failing, because the message length is longer than the read buffer. In Server.swift
a NIO AdaptiveRecvByteBufferAllocator
is created which defaults to a minimum size of 64 bytes, an initial size of 1024 bytes and a large maximum, however without digging yet into the details of how the buffer resizes itself, I noticed that after a few messages it adapts to a 512 byte buffer size, which is not long enough for many messages, and doesn't seem to grow when larger messages are received.
I increased the minimum size of the buffer until I stopped getting decryption errors by modifying the buffer creation in Server.swift:71
.childChannelOption(ChannelOptions.recvAllocator, value:
AdaptiveRecvByteBufferAllocator(minimum: 2056, initial: 4098, maximum: 16392))
With a short interaction using my configuration, the longest message size was 1155 bytes. Even with a initial buffer size of 2046, I was only able to avoid a decryption failure by making the minimum buffer size larger than the longest message.
Here is an extract:
[hap.encryption|INFO] Decrypt message #0, length: 95
[hap.encryption|INFO] Decrypt message #1, length: 236
[hap.encryption|INFO] Decrypt message #2, length: 387
[hap.encryption|INFO] Decrypt message #1, length: 205
[hap.encryption|INFO] Decrypt message #2, length: 537
[hap.encryption|INFO] Decrypt message #1, length: 205
[hap.encryption|INFO] Decrypt message #2, length: 205
[hap.encryption|INFO] Decrypt message #3, length: 1155
[hap.encryption|INFO] Decrypt message #3, length: 537
[hap.encryption|INFO] Decrypt message #4, length: 477
[hap.encryption|INFO] Decrypt message #2, length: 537
[hap.encryption|INFO] Decrypt message #5, length: 1155
[hap.encryption|INFO] Decrypt message #7, length: 927
[hap.encryption|INFO] Decrypt message #3, length: 1042
[hap.encryption|INFO] Decrypt message #4, length: 113
[hap.encryption|INFO] Decrypt message #2, length: 537
[hap.encryption|INFO] Decrypt message #3, length: 1042
[hap.encryption|INFO] Decrypt message #4, length: 113
[hap.encryption|INFO] Decrypt message #3, length: 1042
[hap.encryption|INFO] Decrypt message #4, length: 113
[hap.encryption|INFO] Decrypt message #0, length: 95
[hap.encryption|INFO] Decrypt message #1, length: 236
[hap.encryption|INFO] Decrypt message #2, length: 387
[hap.encryption|INFO] Decrypt message #8, length: 107
[hap.encryption|INFO] Decrypt message #9, length: 205
This workaround seems to temporarily cure the decryption issue, but I expect the long term solution requires working out why the buffer is not growing in size as required.
Fixed by PR #79 which implements the workaround mentioned above.
Great investigation so far. The HAP specification notes the following on the transport security:
Each HTTP message is split into frames no larger than 1024 bytes. Each frame has the following format:
<2:AAD for little endian length of encrypted data (n) in bytes> <n:encrypted data according to AEAD algorithm, up to 1024 bytes> <16:authTag according to AEAD algorithm>
So my guess is that if you make the buffer 1042, it should be able to receive frames just fine. However while handling reads from the channel, we're not looking at the frame's header, but just forward whatever we get to the Cryptographer
. This means that when a frame doesn't fit a buffer, thus is split over multiple reads, we forward incomplete frames to the Cryptographer
.
So while I'm happy to take the workaround suggested in the PR, a proper fix would be to read the frame headers. Either by merge the Cryptographer
and the CryptographerHandler
, or at least by not sending incomplete frames to the Cryptographer
.
Yes, I noted the mention in the public available spec that frames would be no larger than 1024 bytes, however a minimum buffer size of 1024 still results in decryption failures, due to the occasional frame of length 1155 (as shown in the above log extracts). Either the spec changed or there are some devices on my network (HomePods?) which are not respecting the requirement. Setting the minimum buffer size to 1200 has worked successfully so far. I've added that workaround in PR #79
I agree that a proper fix would be to ensure that a complete frame is read before sending for decryption.
I've started working on a proper fix for this in #80. A fixed read buffer of 1042 (not 1024) would probably workaround the issue, as then each frame would at most fill the entire buffer. However I don't think the current allocator works that way, so that's why it's probably failing for you. Setting a bigger size would only defer the issue until bigger messages are send by the client.
Great, let me know when you would like me to test it out on my local network.
I've experimented with simple setups in hap-server
to try to reproduce the large messages. I only get small messages with the default configuration, and not a great deal bigger when I add all the test accessories that are currently commented out in main.swift
. However if I add a TV accessory, which has many more Services and Characteristics, I see much larger messages, and so it may be possible for you to reproduce the issue too.
To see the packet lengths, change the log level of hap.encryption
in hap-server/main.swift
:
getLogger("hap.encryption").logLevel = .info
You can search the log for lines beginning with [hap.encryption|INFO] Decrypt message
Then add a TV accessory to main.swift
:
let tv = Accessory.Television(info: Service.Info(name: "Main TV", serialNumber: "00008"),
inputs: [("Channel 1", Enums.InputSourceType.hdmi),
("Channel 2", Enums.InputSourceType.hdmi),
("Channel 3", Enums.InputSourceType.hdmi),
("Channel 4", Enums.InputSourceType.hdmi),
("Channel 5", Enums.InputSourceType.hdmi),
("Channel 6", Enums.InputSourceType.hdmi),
("Channel 7", Enums.InputSourceType.hdmi),
("Channel 8", Enums.InputSourceType.hdmi)
])
and add the accessory to the bridge
let device = Device(
bridgeInfo: Service.Info(name: "Bridge", serialNumber: "00001"),
setupCode: "123-44-321",
storage: storage,
accessories: [
tv,
livingRoomLightbulb,
bedroomNightStand
...
You will need to use the PR #78 changes to be able to sucessfully add the Television
device to HomeKit. You don't need the beta builds, you can add this test device under iOS 12.1. Although it just cannot be controlled from the iOS 12.1 UI, you can still see the large messages. Note that without PR #78 HomeKit is unable to add the accessory.
With the setup above, I am seeing packets as long as 1338 bytes. I checked which device was sending the longer messages, and discovered they were coming from both the iOS 12.1 iPhone I used to add the bridge, and from one (of two) HomePod's on the network which is the primary HomeKit Hub for the test Home.
With these changes, are you able to reproduce the long packets ?
Thanks, this helps a lot. I've got a fix ready in my PR. With the steps to reproduce that you provided, the problem does no longer appear. I also reduced the amount of memory allocations required when encrypting / decrypting. As a result the code should be a little more performant on constrained environments, like the Raspberry Pi.
I've updated my home installation to that branch, to further test it. Can you let me know how it works for you?
I'm moving my project over to the new master, but am seeing issues with the NIO stack. I am able to pair with a controller, but HAP gets into an infinite loop reverifying connections over and over, preceded by the following message:
Here is a log extract after pairing. The bridge worked fine in the same HomeKit network and config with the previous network stack. Any pointers on where to look in the NIO code for what is causing this problem ?