Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update crypto libs and json #130

Closed
wants to merge 24 commits into from
Closed

update crypto libs and json #130

wants to merge 24 commits into from

Conversation

gjvanderheiden
Copy link
Contributor

Pull Request Checklist

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

  • [x ] For fixes and other improvements, please reference the GitHub issue that your change addresses.
  • [ x] For fixes, optimizations and new features, please add an entry to the CHANGES.md file.
  • [x ] Run mvn compile before committing, so that the auto-code formatter will format your changes consistently with the rest of the project.

@gjvanderheiden
Copy link
Contributor Author

issue:
#129 (#129)
and
#80 #80

Has overlap with pr #116 (#116) but I was getting impatient.

@gjvanderheiden gjvanderheiden changed the title [#130](https://github.com/hap-java/HAP-Java/pull/130) update crypto libs and json Dec 23, 2020
@gjvanderheiden
Copy link
Contributor Author

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
Copy link
Contributor Author

Can this PR pushed further?

@yfre
Copy link
Contributor

yfre commented Jan 25, 2021

Can this PR pushed further?

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

@yfre
Copy link
Contributor

yfre commented Jan 25, 2021

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@yfre
Copy link
Contributor

yfre commented Feb 8, 2021

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
Copy link
Contributor Author

Awesome. Thank you for testing this.

@gjvanderheiden
Copy link
Contributor Author

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
Copy link
Contributor

yfre commented Feb 17, 2021

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
Copy link

J-N-K commented Feb 17, 2021

@yfreyou are running into bndtools/bnd#2227

@yfre
Copy link
Contributor

yfre commented Feb 17, 2021

@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
openhab/openhab-core#1855 (comment)

but not sure how.

@gjvanderheiden
Copy link
Contributor Author

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
Copy link
Contributor

yfre commented Feb 18, 2021

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.

Perform maven build
Added release automation via Github workflow and upload to Maven Central for SNAPSHOTs and releases.
We don’t really need to build an assembly - most people will just consume this via maven, and if they don’t, they can do their own packaging.
Turns out wagon-scm doesn’t pick up the github properties the way the release plugin does. This fixes the site deploy.

Fixes #141
When we change the properties that we’re advertising over mDNS we unregister and re-register the service. However, we were calling unregister after changing the properties. This happened to work with our version of jmdns, which only compared the qualified name. jmdns has quite a few forks however, and it’s best to adhere to the implied contract when calling unregisterService.

This is adapted from a fix proposed by J-N-K
Without this, the snapshot won’t auto-close.
Fix for unregistering service when re-advertising
@bentech
Copy link

bentech commented Apr 8, 2021

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
Copy link
Collaborator

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 : bcgit/bc-java@9d0edfb#diff-9954afe44e634c0de04829e35b8a8903e2397865524bf3020794ed0aade8ce04

@andylintner andylintner mentioned this pull request Apr 10, 2021
andylintner and others added 4 commits April 10, 2021 12:46
extend support for custom min/max/step to further characteristics
… update_crypto_and_json_lib

# Conflicts:
#	CHANGES.md
@gjvanderheiden
Copy link
Contributor Author

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

@bentech
Copy link

bentech commented Nov 11, 2021

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

@gjvanderheiden gjvanderheiden deleted the update_crypto_and_json_lib branch January 2, 2022 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants