search

hap-java / HAP-Java

Java implementation of the HomeKit Accessory Protocol
MIT License
153 stars 82 forks source link

update crypto libs and json #130

Closed gjvanderheiden closed 2 years ago

gjvanderheiden commented 3 years ago

Pull Request Checklist

Please confirm that you've done the following when opening a new pull request:

gjvanderheiden commented 3 years ago

issue:

129 (https://github.com/hap-java/HAP-Java/issues/129)

and

80 https://github.com/hap-java/HAP-Java/issues/80

Has overlap with pr #116 (https://github.com/hap-java/HAP-Java/pull/116) but I was getting impatient.

gjvanderheiden commented 3 years ago

Note for a review:

  1. Removed duplicate code in SrpHandler, this was allready present in ByteUtils. Changed the name of the routine a bit, hopefully the name of the method makes it tiny bit easier to read
  2. Removed duplicate code in SrpHandler, handleRequest was doing the same as PairingManager.handle.
  3. The srp6 lib now has got the 3072-bit prime 'N'(RFC 5054, appendix A) build in. No need for that constant anymore.
  4. throw new TlsFatalAlert(AlertDescription.bad_record_mac); in ChachaDecoder is no longer present in the lib, but the caller doesn't seem to do anything special with this Exception specialization of IOException.
gjvanderheiden commented 3 years ago

Can this PR pushed further?

yfre commented 3 years ago

Can this PR pushed further?

let me do some testing to verify that new libs are working fine with apple devices.

yfre commented 3 years ago

@gjvanderheiden just tested. existing and new pairings are both failing here are the logs (hex dump shortened): existing pairing (in home it says bridge not available)

2021-01-25 09:36:44.465 [TRACE] [impl.pairing.PairVerificationManager] - Starting pair verification for openHAB
2021-01-25 09:36:44.487 [TRACE] [er.impl.http.HomekitClientConnection] - 200 /pair-verify
2021-01-25 09:36:44.487 [TRACE] [er.impl.http.HomekitClientConnection] - 200 /pair-verify
2021-01-25 09:36:44.487 [TRACE] [er.impl.http.HomekitClientConnection] - 200 /pair-verify
2021-01-25 09:36:44.491 [TRACE] [server.impl.http.impl.LoggingHandler] - WRITE PooledUnsafeDirectByteBuf(ridx: 0, widx: 243, cap: 256) [/192.168.1.54:49265]:
485454502F312E3120323030204F4B0D0A436F6E74656E742D747970653A206170706C69636174696F6E2F70616972696E672B746C76380D0A436F6E74656E742D4C656E6774683A203133390D0A436F6E6E65637469

2021-01-25 09:36:44.491 [TRACE] [server.impl.http.impl.LoggingHandler] - WRITE PooledUnsafeDirectByteBuf(ridx: 0, widx: 243, cap: 256) [/192.168.1.79:61280]:
485454502F312E3120323030204F4B0D0A436F6E74656E742D747970653A206170706C69636174696F6E2F70616972696E672B746C76380D0A436F6E74656E742D4C656E6774683A203133390D0A436F6E6E656374696

2021-01-25 09:36:44.491 [TRACE] [server.impl.http.impl.LoggingHandler] - WRITE SimpleLeakAwareByteBuf(PooledUnsafeDirectByteBuf(ridx: 0, widx: 243, cap: 256)) [/192.168.1.71:52564]:
485454502F312E3120323030204F4B0D0A436F6E74656E742D747970653A206170706C69636174696F6E2F7

2021-01-25 09:36:44.498 [TRACE] [rver.impl.http.impl.AccessoryHandler] - Terminated HomeKit connection from /192.168.1.71:52564
2021-01-25 09:36:44.498 [TRACE] [rver.impl.http.impl.AccessoryHandler] - Terminated HomeKit connection from /192.168.1.79:61280
2021-01-25 09:36:44.501 [TRACE] [rver.impl.http.impl.AccessoryHandler] - Terminated HomeKit connection from /192.168.1.54:49265
2021-01-25 09:37:31.925 [TRACE] [rver.impl.http.impl.AccessoryHandler] - New HomeKit connection from /192.168.1.71:52567
2021-01-25 09:37:31.935 [TRACE] [server.impl.http.impl.LoggingHandler] - READ PooledUnsafeDirectByteBuf(ridx: 0, widx: 165, cap: 1024) [/192.168.1.71:52567]:
504F5354202F706169722D76657269667920485454502F312E310D0A486F73743A206F70656E4841425C303

2021-01-25 09:37:31.936 [TRACE] [impl.pairing.PairVerificationManager] - Starting pair verification for openHAB
2021-01-25 09:37:31.939 [TRACE] [er.impl.http.HomekitClientConnection] - 200 /pair-verify
2021-01-25 09:37:31.940 [TRACE] [server.impl.http.impl.LoggingHandler] - WRITE PooledUnsafeDirectByteBuf(ridx: 0, widx: 243, cap: 256) [/192.168.1.71:52567]:
485454502F312E3120323030204F4B0D0A436F6E74656E742D747970653A206170706C69636174696F6E2F7

2021-01-25 09:37:31.970 [TRACE] [rver.impl.http.impl.AccessoryHandler] - Terminated HomeKit connection from /192.168.1.71:52567

new pairing but old key

2021-01-25 09:39:26.789 [TRACE] [rver.impl.http.impl.AccessoryHandler] - New HomeKit connection from /192.168.1.71:52571
2021-01-25 09:39:26.805 [TRACE] [server.impl.http.impl.LoggingHandler] - READ PooledUnsafeDirectByteBuf(ridx: 0, widx: 132, cap: 1024) [/192.168.1.71:52571]:
504F5354202F706169722D736574757020485454502F312E310D0A486F73743A206F70656E4841425C3033322833292E5F6861702E5F7463702E6C6F63616C0D0A436F6E74656E742D4C656E6774683A20360D0A436F6E74656E742D547970653A206170706C69636174696F6E2F70616972696E672B746C76380D0A0D0A000100060101

2021-01-25 09:39:26.810 [TRACE] [a.server.impl.pairing.PairingManager] - Starting pair for openHAB
2021-01-25 09:39:26.813 [WARN ] [.server.impl.connections.HttpSession] - Exception encountered during pairing
java.lang.IllegalArgumentException: The SRP-6a crypto parameters must not be null
    at io.github.hapjava.server.impl.pairing.HomekitSRP6ServerSession.<init>(HomekitSRP6ServerSession.java:94) ~[bundleFile:?]
    at io.github.hapjava.server.impl.pairing.HomekitSRP6ServerSession.<init>(HomekitSRP6ServerSession.java:116) ~[bundleFile:?]
    at io.github.hapjava.server.impl.pairing.SrpHandler.<init>(SrpHandler.java:31) ~[bundleFile:?]
    at io.github.hapjava.server.impl.pairing.PairingManager.handle(PairingManager.java:31) ~[bundleFile:?]
    at io.github.hapjava.server.impl.connections.HttpSession.handlePairSetup(HttpSession.java:111) [bundleFile:?]
    at io.github.hapjava.server.impl.connections.HttpSession.handleRequest(HttpSession.java:53) [bundleFile:?]
    at io.github.hapjava.server.impl.connections.ConnectionImpl.doHandleRequest(ConnectionImpl.java:56) [bundleFile:?]
    at io.github.hapjava.server.impl.connections.ConnectionImpl.handleRequest(ConnectionImpl.java:49) [bundleFile:?]
    at io.github.hapjava.server.impl.http.impl.AccessoryHandler.channelRead0(AccessoryHandler.java:52) [bundleFile:?]
    at io.github.hapjava.server.impl.http.impl.AccessoryHandler.channelRead0(AccessoryHandler.java:17) [bundleFile:?]
    at io.netty.channel.SimpleChannelInboundHandler.channelRead(SimpleChannelInboundHandler.java:105) [bundleFile:4.1.42.Final]
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:374) [bundleFile:4.1.42.Final]
    at io.netty.channel.AbstractChannelHandlerContext.access$600(AbstractChannelHandlerContext.java:56) [bundleFile:4.1.42.Final]
    at io.netty.channel.AbstractChannelHandlerContext$7.run(AbstractChannelHandlerContext.java:365) [bundleFile:4.1.42.Final]
    at io.netty.util.concurrent.DefaultEventExecutor.run(DefaultEventExecutor.java:66) [bundleFile:4.1.42.Final]
    at io.netty.util.concurrent.SingleThreadEventExecutor$6.run(SingleThreadEventExecutor.java:1044) [bundleFile:4.1.42.Final]
    at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) [bundleFile:4.1.42.Final]
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) [bundleFile:4.1.42.Final]
    at java.lang.Thread.run(Thread.java:834) [?:?]
2021-01-25 09:39:26.815 [TRACE] [er.impl.http.HomekitClientConnection] - 500 /pair-setup
2021-01-25 09:39:26.816 [TRACE] [server.impl.http.impl.LoggingHandler] - WRITE PooledUnsafeDirectByteBuf(ridx: 0, widx: 116, cap: 256) [/192.168.1.71:52571]:
485454502F312E312035303020496E7465726E616C20536572766572204572726F720D0A436F6E74656E742D4C656E6774683A2033340D0A436F6E6E656374696F6E3A206B6565702D616C6976650D0A0D0A6A6176612E6C616E672E496C6C6567616C417267756D656E74457863657074696F6E

2021-01-25 09:39:26.975 [TRACE] [rver.impl.http.impl.AccessoryHandler] - Terminated HomeKit connection from /192.168.1.71:52571

new pairing and new key (complete fresh installation)

2021-01-25 09:48:22.195 [ERROR] [org.openhab.io.homekit              ] - bundle org.openhab.io.homekit:3.1.0.202101250833 (409)[org.openhab.io.homekit.internal.HomekitImpl(280)] :  Error during instantiation of the implementation object
java.lang.reflect.InvocationTargetException: null
    at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) ~[?:?]
    at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62) ~[?:?]
    at jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) ~[?:?]
    at java.lang.reflect.Constructor.newInstance(Constructor.java:490) ~[?:?]
    at org.apache.felix.scr.impl.inject.ComponentConstructor.newInstance(ComponentConstructor.java:309) ~[bundleFile:?]
    at org.apache.felix.scr.impl.manager.SingleComponentManager.createImplementationObject(SingleComponentManager.java:277) [bundleFile:?]
Caused by: java.lang.IllegalAccessError: class io.github.hapjava.server.impl.HomekitUtils tried to access private method com.nimbusds.srp6.SRP6Routines.<init>()V (io.github.hapjava.server.impl.HomekitUtils and com.nimbusds.srp6.SRP6Routines are in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @544ed7a4)
    at io.github.hapjava.server.impl.HomekitUtils.generateSalt(HomekitUtils.java:16) ~[?:?]
    at io.github.hapjava.server.impl.HomekitServer.generateSalt(HomekitServer.java:169) ~[?:?]
    at org.openhab.io.homekit.internal.HomekitAuthInfoImpl.initializeStorage(HomekitAuthInfoImpl.java:155) ~[?:?]
    at org.openhab.io.homekit.internal.HomekitAuthInfoImpl.<init>(HomekitAuthInfoImpl.java:55) ~[?:?]
    at org.openhab.io.homekit.internal.HomekitImpl.<init>(HomekitImpl.java:88) ~[?:?]
gjvanderheiden commented 3 years ago

Quite puzzled here. You're testing in openhab? I guess this is a version conflict. The linenumbers in the stacktrace don't make any sense to me in the source code.

gjvanderheiden commented 3 years ago

I always test with clean registry. Especially with object steaming.

yfre commented 3 years ago

im testing with openHAB as i have more tests cases there and would like to keep it also backward compatible. but you are completely right, the old libraries were also in classpath. i will fix and try again.

gjvanderheiden commented 3 years ago

Awesome. Thank you for testing this.

gjvanderheiden commented 3 years ago

I use hap-java and calimero-project/camino-core(https://github.com/calimero-project/calimero-core) in my small project at home, to control my KNX system. Didn't run in any issues with this branch, but my java project is simple in lib dependencies.

yfre commented 3 years ago

update on testing status: i was not able to get it openHAB binding built with this PR (or PR #137) of Java-HAP due some strange issues with dependencies. first complaining about "Classes found in the wrong directory", then about missing "net.i2p.crypto" then about missing "sun.security.x509"

so, im not able to test it with openHAB.

J-N-K commented 3 years ago

@yfreyou are running into https://github.com/bndtools/bnd/issues/2227

yfre commented 3 years ago

@J-N-K yes, it is that issue. but i have not found any solution. fixupmessages has helped to get rid of error message but then i had number of missing dependencies issues

probably i need to do something like that https://github.com/openhab/openhab-core/issues/1855#issuecomment-739573389

but not sure how.

gjvanderheiden commented 3 years ago

ok. but now what? This PR is open for a long time now. Close it or push it. I'm sorry but I'm losing my interest in this project, stuff takes too long.

yfre commented 3 years ago

ok. but now what? This PR is open for a long time now. Close it or push it. I'm sorry but I'm losing my interest in this project, stuff takes too long.

@gjvanderheiden yeah. this is not the fastest project.

i cannot decide on this PR as i could not tested. but it was tested by you and @bentech and the issue with openHAB seems to be issue of openHAB build setup. so, probably it is fine to merge.

bentech commented 3 years ago

ok. but now what? This PR is open for a long time now. Close it or push it. I'm sorry but I'm losing my interest in this project, stuff takes too long.

@gjvanderheiden yeah. this is not the fastest project.

i cannot decide on this PR as i could not tested. but it was tested by you and @bentech and the issue with openHAB seems to be issue of openHAB build setup. so, probably it is fine to merge.

I found that this pull request didn't work for me. That's why i created #137.

andylintner commented 3 years ago

This worked for me. @bentech - your branch didn't. Can you give more details on what didn't work here?

There's only two functional differences between your branches:

That appears to be correct, as BouncyCastle fixed their implementation to do the same in 1.55 : https://github.com/bcgit/bc-java/commit/9d0edfbede2bc877257fdf704b837315de2708ec#diff-9954afe44e634c0de04829e35b8a8903e2397865524bf3020794ed0aade8ce04

gjvanderheiden commented 2 years ago

Bump. More than a half year later... So what's the deal?

bentech commented 2 years ago

Ignore my branch. I did ineed have an upstream problem.