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

Fix ssl issues on middleware urls #1759

Closed
wants to merge 1 commit into from
Closed

Conversation

sainak
Copy link
Member

@sainak sainak commented Dec 8, 2023

Proposed Changes

  • revert to urllib3 1.x

Associated Issue

  • urllib3 changed the verification procedure for ssl certs
  • this causes domains like https://dev_middleware.coronasafe.live/ to fail ssl verification
  • Weirdly this url works fine even though both have same CA, just the CN and sans have changed
  • so until we identify the issue with our certs we are downgrading urllib3 to 1.x
image image

Architecture changes

  • Remove this section if not used

Merge Checklist

  • Tests added/fixed
  • Update docs in /docs
  • Linting Complete
  • Any other necessary step

Only PR's with test cases included and passing lint and test pipelines will be reviewed

@coronasafe/care-backend-maintainers @coronasafe/care-backend-admins

Copy link

codecov bot commented Dec 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0869c46) 61.02% compared to head (beb847c) 61.02%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1759   +/-   ##
=======================================
  Coverage   61.02%   61.02%           
=======================================
  Files         211      211           
  Lines       11589    11589           
  Branches     1659     1659           
=======================================
  Hits         7072     7072           
  Misses       4264     4264           
  Partials      253      253           

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

Copy link

sentry-io bot commented Dec 8, 2023

Sentry issue: CARE-YC

@sainak
Copy link
Member Author

sainak commented Dec 10, 2023

As we checked more middleware instances, we discovered that this only happening with https://dev_middleware.coronasafe.live/

@Ashesh3
Copy link
Member

Ashesh3 commented Dec 10, 2023

It appears that the root of our SSL verification issue is indeed related to the use of underscores in domain names.

I conducted some tests and found the following:

  • test_abc.domain.com - fails SSL verification
  • abc.domain.com - passes SSL verification

These tests were carried out on my Cloudflare domain.

Further, I referred to the DigiCert knowledge base, which confirms that underscores are not allowed in Fully Qualified Domain Names (FQDNs). Here's the link for reference: DigiCert Knowledge Base.

Additionally, I checked the CA/Browser Forum's Ballot SC12 regarding the sunset of underscores in DNS names. The ballot has passed, clearly indicating a shift in standards concerning domain naming practices. You can find more details here: CAB Forum - Ballot SC12.

Based on these findings, it's evident that the issue lies in the use of underscores in the domain name, which is not compliant with SSL hostname matching standards. Therefore, this pull request will be closed as the problem does not stem from our code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked discussion P1 High priority; urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants