Opened 13 years ago
Closed 12 years ago
#19037 closed enhancement (fixed)
Patch for is_ssl, ssl_redirect and general http/https logic / bug
Reported by: | MarcusPope | Owned by: | |
---|---|---|---|
Milestone: | 3.5 | Priority: | normal |
Severity: | normal | Version: | 3.2.1 |
Component: | Security | Keywords: | has-patch |
Focuses: | Cc: |
Description
WP core uses several different methods for determining if the site is using SSL, and whether to redirect to an SSL scheme. Additionally some of those cases use complete logic checks and some only use partial checks.
This patch refactors the copy-pasted logic throughout several files and offers a unified approach to getting the correct scheme.
I've also removed parenthesis from is_ssl in code comments to reduce the number of false positive results when grepping for usage of is_ssl in the code.
site-info.php mistakenly reports the homepage url scheme as the scheme currently being used to browse the page and not the intended scheme of the site.
feed.php now operates on fallback assumption that if http is not "on" server_port could still properly catch a correct ssl session.
corrected inappropriate use of "schema" for scheme variable in functions.php
And it provides a central place for implementing future hooks related to http/https selection logic.
Not sure if component:security is the best choice, but http seemed to apply more to http.php than the ssl/non-ssl scheme selection.
Attachments (2)
Change History (13)
#1
@
13 years ago
btw, if this patch doesn't properly apply to 3.3 because of other changes for that branch I can probably swing some time from my employer to recreate the patch. As it stands we were having to do an audit of full-site ssl-mode security and this just came with the territory of auditing the code.
Thanks,
Marcus
#4
@
13 years ago
I really like where this patch is going.
Ultimately I'd rather see set_url_scheme() in terms of a function name. I'll leave this ticket open though, as it actually begins to make the changes in core, rather than just introducing the new function.
#5
@
13 years ago
Thanks for the ecouraging remarks Nacin. From a naming convention perspective I'm completely ignorant AND agnostic to wordpress standards on that front. So, please take this with a grain of salt if necessary :D
Since this function doesn't actually change the scheme in use, perhaps fix_url_scheme() would be more appropriate? I think set_url_scheme would apply more towards a hybrid of ssl_redirect(), force_ssl_admin() and force_ssl_login() where you could set the scheme to be used for future requests. As where this was/is designed to correct the url schemes of existing link generation functions based purely on the current request method.
I'll totally leave this up to you guys though, just giving my outsider's perspective.
patch for ssl checking logic