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

need a little help understanding and solving signal errors #34

Closed
Connoropolous opened this issue May 15, 2023 · 27 comments · Fixed by #35
Closed

need a little help understanding and solving signal errors #34

Connoropolous opened this issue May 15, 2023 · 27 comments · Fixed by #35

Comments

@Connoropolous
Copy link

Connoropolous commented May 15, 2023

we have the goal to compile and run holochain natively on ios, and it's almost working. Zome calls are working, but gossip isn't. here's what we're now seeing in the logs. We are now using wss://signal.holotest.net, which corrected the initial error that we were previously having.

try connect addr=167.71.76.147:443
err=Custom { kind: InvalidData, error: InvalidCertificateData("invalid peer certificate: UnknownIssuer") }
failed all sig dns addr connects

is the cause of this obvious to you @neonphog ?

@neonphog
Copy link
Collaborator

@Connoropolous - can you go to https://signal.holotest.net/metrics on the device you're testing on and see what it says about the cert?

In the mean time, I'll look into the go lib to see if it's using some alternate TLS validation other than the native system one.

@neonphog
Copy link
Collaborator

neonphog commented May 15, 2023

There's very little information, possibly because it looks like it's just built-in that it uses the native device certs:

https://pkg.go.dev/crypto/tls#Config

// RootCAs defines the set of root certificate authorities
// that clients use when verifying server certificates.
// If RootCAs is nil, TLS uses the host's root CA set.
RootCAs *x509.CertPool

Is it possible the device you are testing on hasn't had an update in long enough that Let's Encrypt root CA is not up-to-date on it?

EDIT: The go handling of this is irrelevant, since rust code is communicating with the signal server, and the rust code is for sure using the native system CAs.

@neonphog
Copy link
Collaborator

Also looks like you need iOS >= 9 (~2015) - https://www.ssllabs.com/ssltest/analyze.html?d=signal.holotest.net

@Connoropolous
Copy link
Author

Connoropolous commented May 15, 2023

I visited, and it says 'connection is secure' from an iPhone XR, with iOS 16.4.1. That is the device I was testing on.

@Connoropolous
Copy link
Author

can you give a bit more background.
what server is this? 167.71.76.147:443 (from try connect addr=167.71.76.147:443)

@Connoropolous
Copy link
Author

that is the signal server ip I guess, based on using ping signal.holotest.net

@neonphog
Copy link
Collaborator

@Connoropolous - Double check my logic - it looks like rustls-native-certs doesn't support ios, and the recommendation is to instead use https://github.com/rustls/rustls-platform-verifier... but that crate seems to have been deleted from crates.io.

I will look into it further tomorrow.

@neonphog
Copy link
Collaborator

If nothing else, I guess we could use webpki-roots on android and ios.

@Connoropolous
Copy link
Author

Your logic seems right. It seems to me that this particular comment has a simple solution that could work. rustls/rustls-native-certs#3 (comment)

@Connoropolous
Copy link
Author

The same solution that is being used for macOS should work with iOS, so just including it through the targets
https://github.com/rustls/rustls-native-certs/blob/88066f5eebeabf034a5a1deda83c6214b3d024c9/src/lib.rs#L36

@Connoropolous
Copy link
Author

Connoropolous commented May 16, 2023

Based on the logic in src/lib.rs iOS is likely compiling with the Unix platform code

@neonphog
Copy link
Collaborator

@Connoropolous - as I don't have a good way to directly test such a fix, I wouldn't be comfortable submitting a PR to their repo.

I'll work on the webpki-roots fallback in our code, since it looks like we'll need that for android anyways. If you'd like to submit a PR for rustls-native-certs, then I can remove ios from the fallback set if/when it gets merged.

@Connoropolous
Copy link
Author

Ok thankyou. We will await your solution and try it before doing anything else

@Connoropolous
Copy link
Author

Will you give me a heads up here?

@neonphog
Copy link
Collaborator

@Connoropolous - can you give this a try? #35 Thanks!

@Connoropolous
Copy link
Author

Yes absolutely, asap

@Connoropolous
Copy link
Author

Thanks!

@Connoropolous
Copy link
Author

Updating the stack to use this branch you posted worked to resolve the error!
Now attempting hands on testing

@abe-njama
Copy link

abe-njama commented May 22, 2023

@Connoropolous @zippy wondering which release we should be getting this fix into. It didn't make it into the 0.2.1-beta-dev.0.

  • Should we backport this into Holochain 0.2.2 or work forward to Holochain 0.3.0?
  • @Connoropolous was the hands on testing a success?

@neonphog
Copy link
Collaborator

PR to potentially backport to 0.2: holochain/holochain#2415

@Connoropolous
Copy link
Author

k. as I've done hands on testing, I've had inconclusive results. @neonphog I am wondering if its mandatory that tx2 still be enabled. Why is it?
Here's my writeup on how things are going, and where we're stuck.
https://hackmd.io/_CFnGaWvQg6Ys3DxZblmug

@neonphog
Copy link
Collaborator

@Connoropolous - tx2 is tech debt at this point. We have a bunch of test mocks built around it that need to be migrated, plus the final work of deleting things. It just hasn't been prioritized.

@neonphog
Copy link
Collaborator

neonphog commented May 23, 2023

@Connoropolous - To get some additional hints into the tx5 state machine: RUST_LOG=info,tx5::state=debug

EDIT: even better: RUST_LOG=info,tx5::state=debug,tx5::endpoint=debug

@Connoropolous
Copy link
Author

Connoropolous commented May 23, 2023

thanks, I will try

@Connoropolous
Copy link
Author

@neonphog I've just tried updating to 0.3.0-beta-dev.0

Here's my update, can you take a peek, does the error make any sense to you?
https://hackmd.io/_CFnGaWvQg6Ys3DxZblmug?view#May-23

@neonphog
Copy link
Collaborator

@Connoropolous - I just created this issue: holochain/holochain#2422

@Connoropolous
Copy link
Author

Fantastic thanks

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 a pull request may close this issue.

3 participants