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

Add more sophisticated internet connectivity checker #520

Open
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

ConnorRigby
Copy link
Member

This adds the ability to detect sneaky firewalls or anything else that may give false positives on internet connectivity by checking that a valid SSL connection can be made to a list of hosts.

@ConnorRigby ConnorRigby requested a review from fhunleth May 2, 2024 14:13
@ConnorRigby ConnorRigby force-pushed the ssl-connectivity-checker branch 4 times, most recently from 5a859f4 to e34b05c Compare May 2, 2024 15:49
mix.exs Outdated Show resolved Hide resolved
@ConnorRigby ConnorRigby force-pushed the ssl-connectivity-checker branch 3 times, most recently from 8167006 to 6be5110 Compare May 2, 2024 21:22
@ConnorRigby ConnorRigby force-pushed the ssl-connectivity-checker branch 2 times, most recently from bccd098 to 7a83e52 Compare May 7, 2024 13:54
@fhunleth
Copy link
Member

@ConnorRigby I made some changes in the ssl-connectivity-checker-fh branch to pull more of the ping-related code to the behavior. I think there shouldn't be any case statements in the core now. Could you see if you like this.

Other things that I'm thinking about:

  1. The VintageNet.Connectivity.SSLPing.ConnectOptions behaviour only had one function. It seems like making a behaviour isn't needed and we just pass a function around for this. However, since so many cases don't require runtime options, how about passing the ssl options with :host and :port too?
  2. The example was with AWS IoT, but AWS IoT has a one-connection policy. That means that if the device is already connected, the SSL ping can cause AWS to break the current connection after it authenticates. (I actually don't know when AWS IoT breaks old connections, but it seems that after a mutual auth completes, it could do this.) I think we should pick a different example server.

Overall, I think this is looking really good.

@ConnorRigby
Copy link
Member Author

ConnorRigby commented May 13, 2024

The VintageNet.Connectivity.SSLPing.ConnectOptions behaviour only had one function. It seems like making a behaviour isn't needed and we just pass a function around for this. However, since so many cases don't require runtime options, how about passing the ssl options with :host and :port too?

I think passing a function around is fine. I just added the behaviour for easy typing. I think a typespec will also suffice. Should i change it back to the {module, function, args} version instead of just a module?

The example was with AWS IoT, but AWS IoT has a one-connection policy. That means that if the device is already connected, the SSL ping can cause AWS to break the current connection after it authenticates. (I actually don't know when AWS IoT breaks old connections, but it seems that after a mutual auth completes, it could do this.) I think we should pick a different example server.

how about github.com?

@ConnorRigby
Copy link
Member Author

The changes in your branch look great to me.

@ConnorRigby ConnorRigby requested a review from fhunleth June 7, 2024 17:31
@ConnorRigby ConnorRigby force-pushed the ssl-connectivity-checker branch 2 times, most recently from dcea8ee to 371f122 Compare June 7, 2024 18:45
@fhunleth fhunleth changed the title Add SSL based internet connectivity checker Add more sophisticated internet connectivity checker Jun 7, 2024
mix.exs Outdated Show resolved Hide resolved
mix.exs Outdated Show resolved Hide resolved
lib/vintage_net/connectivity/when_where.ex Show resolved Hide resolved
lib/vintage_net/connectivity/when_where.ex Show resolved Hide resolved
lib/vintage_net/connectivity/when_where.ex Outdated Show resolved Hide resolved
lib/vintage_net/connectivity/when_where.ex Outdated Show resolved Hide resolved
mix.exs Outdated Show resolved Hide resolved
test/vintage_net/connectivity/when_where_test.exs Outdated Show resolved Hide resolved
@ConnorRigby ConnorRigby force-pushed the ssl-connectivity-checker branch 6 times, most recently from 4027e42 to 5964f92 Compare June 14, 2024 14:04
ConnorRigby and others added 27 commits June 17, 2024 16:42
Signed-off-by: Connor Rigby <[email protected]>
Signed-off-by: Connor Rigby <[email protected]>
Signed-off-by: Connor Rigby <[email protected]>
Signed-off-by: Connor Rigby <[email protected]>
Signed-off-by: Connor Rigby <[email protected]>
Signed-off-by: Connor Rigby <[email protected]>
Signed-off-by: Connor Rigby <[email protected]>
Signed-off-by: Connor Rigby <[email protected]>
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.

2 participants