Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Disable mandatory TLS verification with --insecure #1401

Closed

Conversation

d4xfe
Copy link
Contributor

@d4xfe d4xfe commented Apr 29, 2024

getpeercert() only returns the certificate with enabled certificate validation. Because of this, I updated it to getpeercert(True), which will return a binary blob of the servers certificate regardless of the verification mode.

I added the util method cert_der_to_dict using ssl internals to convert the blob to a python dict like getpeercert() would.
The flag --insecure was introduced to disable certificate validation of ssl sockets.
This allows TLS interception when the server is using a self-signed certificate.

This is my first pull request on GitHub and I'm still a bit confused. So if I should change anything let me know.

@abhinavsingh
Copy link
Owner

@d4xfe Thank you, looks excellent considering this is your first PR. Congratulations.

Can you please:

  1. Resolve conflicts
  2. Sync your branch with develop. For some reasons, I see diffs in your PR which are not part of your changes

@abhinavsingh abhinavsingh self-requested a review April 30, 2024 03:00
@abhinavsingh abhinavsingh added the bot:chronographer:skip PR using this label is exempted from CHANGELOG management label Apr 30, 2024
@d4xfe
Copy link
Contributor Author

d4xfe commented Apr 30, 2024

@abhinavsingh Thanks. I've synced the branches and I don't conflicts anymore.

Copy link

codecov bot commented Aug 10, 2024

Codecov Report

Attention: Patch coverage is 87.09677% with 4 lines in your changes missing coverage. Please review.

Project coverage is 84.37%. Comparing base (39854e1) to head (ec87033).
Report is 21 commits behind head on develop.

Files with missing lines Patch % Lines
proxy/common/utils.py 81.81% 1 Missing and 1 partial ⚠️
proxy/core/connection/server.py 33.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1401      +/-   ##
===========================================
- Coverage    84.57%   84.37%   -0.20%     
===========================================
  Files          177      178       +1     
  Lines         8103     8130      +27     
  Branches      1239     1242       +3     
===========================================
+ Hits          6853     6860       +7     
- Misses        1052     1059       +7     
- Partials       198      211      +13     
Flag Coverage Δ
GHA 84.36% <87.09%> (-0.05%) ⬇️
Linux 83.99% <87.09%> (-0.04%) ⬇️
Python 3.10 85.13% <82.60%> (-0.02%) ⬇️
Python 3.11 84.31% <87.09%> (+<0.01%) ⬆️
Python 3.12 79.36% <87.09%> (-4.96%) ⬇️
Python 3.6 ?
Python 3.7 ?
Python 3.8 ?
Python 3.9 85.21% <82.60%> (-0.02%) ⬇️
Windows ?
macOS 84.36% <87.09%> (-0.05%) ⬇️
pytest 84.36% <87.09%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@abhinavsingh
Copy link
Owner

@d4xfe Thank you for this work and the PR. There are some workflow issues. I am going create a new PR based on top of your contribution and try to this ship.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bot:chronographer:skip PR using this label is exempted from CHANGELOG management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants