-
Notifications
You must be signed in to change notification settings - Fork 873
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 HTTPS upgrade checks #23029
Add HTTPS upgrade checks #23029
Conversation
a683903
to
57f5d6e
Compare
@@ -599,6 +599,26 @@ extension BrowserViewController: WKNavigationDelegate { | |||
let response = navigationResponse.response | |||
let responseURL = response.url | |||
let tab = tab(for: webView) | |||
let isInvalid: Bool | |||
if let httpResponse = response as? HTTPURLResponse { | |||
isInvalid = httpResponse.statusCode >= 400 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure about this >= 400 check? What happens if the response is a 000?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so:
if (headers && headers->response_code() >= 400) { \ |
Needs a manual sec review for secure icon state when upgrading from http to https: brave/brave-browser#36951 (comment). |
57f5d6e
to
74a3491
Compare
ios/browser/api/https_upgrade_exceptions/https_upgrade_exceptions_service.mm
Show resolved
Hide resolved
Add http upgrade strict mode and interstitial
[puLL-Merge] - brave/brave-core@23029 DescriptionThis PR adds support for the HTTPS Upgrades feature in Brave iOS. It allows upgrading HTTP connections to HTTPS and blocking HTTP connections that can't be upgraded, with a user-facing setting to control the behavior. ChangesChanges
Security Hotspots
Overall this is a security-enhancing feature, but the implementation should be audited to ensure it does not introduce any new vulnerabilities around SSL/HTTPS. The core logic around upgrading and allowing exceptions is the key area to focus on. |
Resolves brave/brave-browser#36408
Security review: https://github.com/brave/reviews/issues/1593
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
Note: I don't know of any sites that do exist in http but not in https. If you are able to come across one please test it.