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

Split validation #235

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Split validation #235

wants to merge 6 commits into from

Conversation

ohnorobo
Copy link
Collaborator

@ohnorobo ohnorobo commented May 10, 2023

This splits answer:not_validated into answer:unvalidated_http_connection and answer:no_tcp_connection (when we were not able to connect to the answer ip at all.

I'd like suggestions about:

  • the clearest (short) names to use for these outcome
  • thoughts on this approach overall. I think trying to break out not_validated further is useful, since it's 60% of our unexpected outcomes. But I don't have a full theory of how this new information is actionable.
  • Do we want to break out answer:no_tcp_connection further, by splitting along different types of connection errors? (timeout, no route, etc)

dashboard with this data: here

Here's the new outcome breakdown
image

@@ -129,7 +129,15 @@ CREATE TEMP FUNCTION OutcomeString(domain_name STRING,
WHEN (SELECT LOGICAL_OR(answer.matches_control.asn)
FROM UNNEST(answers) answer)
THEN "✅answer:matches_asn"
ELSE CONCAT("❓answer:not_validated:", AnswersSignature(answers))
# TODO: delete, this case is covered by all the cert cases above
WHEN (SELECT LOGICAL_OR(NOT (a.https_response_status IS NULL))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we tell whether we are able to establish a TCP connection to port 443?

I would suggest to create a "{error}:{AS}", and move it to right after matches:ip. Possible errors: connection_timeout, address_unreachable, network_unreachable, connection_refused.
Example: connection_timeout:TWITTER or maybe answer:TWITTER:connection_refused

We would still keep the not_validated as an unexpected catch all in that case.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, you can find the syscall error strings from Go at https://cs.opensource.google/go/go/+/master:src/syscall/zerrors_linux_amd64.go

WHEN (SELECT LOGICAL_OR(NOT (a.https_response_status IS NULL))
FROM UNNEST(answers) a)
THEN CONCAT("❗️answer:unvalidated_https_connection:", AnswersSignature(answers))
WHEN (SELECT LOGICAL_OR(NOT (a.http_response_status IS NULL))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also suggest we drop the HTTP test. It makes it harder to reason about the methodology by involving another port and another fetch.

@ohnorobo
Copy link
Collaborator Author

ohnorobo commented May 12, 2023

So @fortuna the overall thought here is:

  • drop HTTP page fetches. We know the domain is capable of HTTPS, so what happens with port 80 on a random dubiously-received IP is not helpful.
  • this will also drop HTTP blockpages, which by now are very small fraction of outcomes. (0.14% of unexpected)
  • Any IP where we received a response on port 443 is split out into one of the cert in/valid outcomes.
  • Any IP where we did not get a response on 443 (currently marked as not_validated) gets split out by error type (timeout, unreachable, refused, etc)

I also looked some into the remaining cases of http blockpages here which makes me think they still have some value

@ohnorobo ohnorobo changed the base branch from cert-bump to master May 12, 2023 13:38
@ohnorobo
Copy link
Collaborator Author

I made a sample dashboard that does something like this here

I'm not yet convinced that having the errors allows you to make any better analysis decisions, but it's interesting to be able to see.

image

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.

None yet

2 participants