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

(Tentative) Move SSL capture to ScoopProxy #140

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

matteocargnelutti
Copy link
Collaborator

@matteocargnelutti matteocargnelutti commented Apr 11, 2023

Implements #138


  • Removes crip dependency, dedicated certificates capture step, and associated options.
  • Intercepts certificate chain at ScoopProxy level using socket.getPeerCertificate() to assemble a PEM on the fly. Runs once per origin.
  • Removes duplicate processing of noarchive checks

Still working through:

  • Blocking: Certs sometimes don't get intercepted (Tentative) Move SSL capture to ScoopProxy #140 (comment)
  • Opportunity: Since there is little to no cost in pulling the certs with this setup, we can account for the (rare) cases in which certs change from one url to the other by indexing them by hash.
  • Optional: The certificates interception currently happens at ScoopProxy.onResponse() level.
    It should be in ScoopProxy.onConnected(), but in some cases it appears to be "too early".
    TBD, but this version works.

Implements #138

---

- Removes `crip` dependency, dedicated certificates capture step and associated options.
- Intercepts certificate chain at `ScoopProxy` level using `socket.getPeerCertificate()` to assemble a PEM on the fly. Runs once per origin.
- Removes duplicate processing of `noarchive` checks

---

**Still working through:** The certificates interception currently happens at `ScoopProxy.onResponse()` level. It should be in `ScoopProxy.onConnected()`, but in some cases it appears to be _"too early"_. TBD, but this version works.
Copy link
Contributor

@bensteinberg bensteinberg left a comment

Choose a reason for hiding this comment

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

LGTM!

@matteocargnelutti
Copy link
Collaborator Author

matteocargnelutti commented Apr 11, 2023

Update: This works in most cases, but during my tests I've observed inconsistent behavior.
In some cases, certain certs will never be intercepted. For example when capturing an instagram.com URL with that method, only *.facebook.com certs are intercepted.

Looking into it, but setting this PR as draft in the meantime, as I don't know if this method of capture will end up being suitable.

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