search

novastone-media / MQTT-Client-Framework

iOS, macOS, tvOS native ObjectiveC MQTT Client Framework
Other
1.84k stars 463 forks source link

[bugfix] remove SecCertificateRef that will cause tls handshake failed #547

Open yacosdad opened 4 years ago

yacosdad commented 4 years ago

Before deleting the 'cert' code, the package info of client for exchange had two certificates which were same with each other. The screenshot is below: before-1 before-2 After deleting that, everything was OK! after-1 after-2

The SecIdentityRef contains a SecCertificateRef, so we do not add another SecCertificateRef again!

Please fix it and release a new pod version, thanks!

ckrey commented 4 years ago

@yacosdad Thanks for spotting and fixing this

yacosdad commented 4 years ago

@yacosdad Thanks for spotting and fixing this

@ckrey Em...but why the CI executed failed？Does this PR will be merged as soon as possible？

jcavar commented 4 years ago

Don't worry about CI - that is just flakiness. But should we return only cert instead of identityRef?

yacosdad commented 4 years ago

Don't worry about CI - that is just flakiness. But should we return only cert instead of identityRef?

@jcavar No，we can not. The kCFStreamSSLCertificates Discussion shows that the SecIdentityRef must be first item in the array.

WechatIMG1267

jcavar commented 4 years ago

Hmm, I am still not sure I understand. Could you please share how you use certificates returned by this method? Where do you set them?

yacosdad commented 4 years ago

Hmm, I am still not sure I understand. Could you please share how you use certificates returned by this method? Where do you set them?

Finally, the certificates was used in "open" method of MQTTSSLSecurityPolicyTransport class. That was set into the key kCFStreamSSLCertificates of ssloptions to set kCFStreamPropertySSLSettings property of CFReadStreamRef/CFWriteStreamRef.

This is the demo: WechatIMG1269

jcavar commented 4 years ago

Ok, I see. But the docs state that it should be an array of SecCertificateRefs except first SecIdentityRef. With this change, we only leave SecIdentityRef?

Also, implementation of this other library does the same thing as we are doing now: https://github.com/daltoniam/Starscream/blob/master/Sources/Starscream/SSLClientCertificate.swift

yacosdad commented 4 years ago

Ok, I see. But the docs state that it should be an array of SecCertificateRefs except first SecIdentityRef. With this change, we only leave SecIdentityRef?

Also, implementation of this other library does the same thing as we are doing now: https://github.com/daltoniam/Starscream/blob/master/Sources/Starscream/SSLClientCertificate.swift

YES, my code works fine and the package screenshot in the issue topic shows that the communication process is right. You can grab the network package and take a test.

jcavar commented 4 years ago

I understand what you are saying, but I don't understand why that is the case. Documentation confirms what our current code is doing. (and other library is doing the same thing).

Maybe there is an issue somewhere else?