Wedge

Public area => The Pub => Features => Topic started by: Arantor on March 21st, 2013, 04:39 AM

Title: X-Content-Security-Policy
Post by: Arantor on March 21st, 2013, 04:39 AM
https://developer.mozilla.org/en-US/docs/Security/CSP/Using_Content_Security_Policy

I already implemented X-Frame-Options for security, but CSP is much more hardcore.

The idea is that you restrict the acceptable sources for content that can be included. You can limit where scripts can be included from (e.g. forcing the scripts to run only from the main site/subdomains, or accepted third parties), plus you can limit where images can be included from.

I love the idea, but I'm wary of implementing this in core because we have acceptable third party sources. In our case, we typically want to allow main includes from the main domain, images from anywhere, movies and so on from acceptable domains (e.g. auto embedding), scripts from the main domain and specific CDNs.

This is all doable, but it makes the potential header very large (though that's possible to mitigate by being careful about what's included where, e.g. not worrying about auto-embed in places where we don't handle bbc parsing where movies would be meaningful, e.g. news) which is a per-page thing :/

But it is a useful method for mitigating injections from unexpected sources and can limit clickjacking and so on.
Title: Re: X-Content-Security-Policy
Post by: Nao on March 24th, 2013, 05:34 PM
Yup, I'm not tempted by this either... I was already frightened when you added a new header :lol: (But I figured, I've saved enough bytes in every page to allow for some increased security...!)

Perhaps as a plugin, if doable.
Title: Re: X-Content-Security-Policy
Post by: Arantor on March 24th, 2013, 05:48 PM
The only downside about headers is that they don't get gzipped >_<
Quote
Perhaps as a plugin, if doable.
Oh hell yes.
Title: Re: X-Content-Security-Policy
Post by: Nao on March 25th, 2013, 08:36 AM
I don't think gzipping would be a game changer for headers, though ;)

I'm more interested in saving a kilobyte per page, than a few bytes, as opposed to what everyone may think. I just can't help myself :lol:

@Everyone> Please @ me! I can't bother to manually modify the database to add more notifications to myself :P :edit: Oh well, that should be okay.
Title: Re: X-Content-Security-Policy
Post by: MultiformeIngegno on March 25th, 2013, 11:10 AM
Ok @Nao :D
Posted: March 25th, 2013, 11:08 AM

Ops, the first fine I tried to post the message above I got this:
Please try again. If you come back to this error screen, report the error to an administrator.

Trying again worked.
Title: Re: X-Content-Security-Policy
Post by: Arantor on March 25th, 2013, 04:19 PM
@Nao, probably not, for the simple reason that they're not duplicated on each page.

Though I'm cringing because what I'm looking at for the permissions area is a 22KB JS library (unminified), specifically the one from http://fixedheadertable.com/ (there, that's got you thinking)

And yes, I'm convinced it's possible to do this header as a plugin, it's just going to be messy.
Posted: March 25th, 2013, 04:18 PM

Also:
Quote
Unknown column 'disabled_notifiers' in 'field list'
File: Sources/Class-Notification.php
Line: 181
Title: Re: X-Content-Security-Policy
Post by: Dragooon on March 25th, 2013, 05:41 PM
Oh crap, my mistake

Code: (Find) [Select]
$request = wesql::query('
SELECT disabled_notifiers, email_notifiers, email_address, id_member
FROM {db_prefix}members
WHERE id_member IN ({array_int:member})
LIMIT {int:limit}',
array(
'member' => $members,
'limit' => count($members),
)
);
$members = array();
while ($row = wesql::fetch_assoc($request))
{
$members[$row['id_member']] = array(
'id' => $row['id_member'],
'disabled_notifiers' => explode(',', $row['disabled_notifiers']),
'email_notifiers' => json_decode($row['email_notifiers'], true),
'email' => $row['email_address'],
);
}
Code: (Replace) [Select]
$request = wesql::query('
SELECT data, email_address, id_member
FROM {db_prefix}members
WHERE id_member IN ({array_int:member})
LIMIT {int:limit}',
array(
'member' => $members,
'limit' => count($members),
)
);
$members = array();
while ($row = wesql::fetch_assoc($request))
{
$data = unserialize($row['data']);

$members[$row['id_member']] = array(
'id' => $row['id_member'],
'disabled_notifiers' => !empty($data['disabled_notifiers']) ? $data['disabled_notifiers'] : array(),
'email_notifiers' => !empty($data['email_notifiers']) ? $data['email_notifiers'] : array(),
'email' => $row['email_address'],
);
}
Title: Re: X-Content-Security-Policy
Post by: Arantor on March 25th, 2013, 05:47 PM
Can't really assume $data will be populated - assume it will likely be empty and not even unserialize properly as a result.
Title: Re: X-Content-Security-Policy
Post by: Dragooon on March 25th, 2013, 06:00 PM
Quote from Arantor on March 25th, 2013, 05:47 PM
Can't really assume $data will be populated - assume it will likely be empty and not even unserialize properly as a result.
Yeah, sorry about that. I added checks for missing fields, but what about $data itself? Something like
Code: [Select]
$data = unserialize($row['data']);
$data = is_array($data) ? $data : array();
? Because either way, a crappy string will issue an E_NOTICE.
Title: Re: X-Content-Security-Policy
Post by: Arantor on March 25th, 2013, 06:02 PM
Code: [Select]
$data = !empty($data) ? @unserialize($row['data']) : array();

After that it's empty() or isset() all the way ;)
Title: Re: X-Content-Security-Policy
Post by: Nao on March 25th, 2013, 07:54 PM
Hmm... How about we set the data field to be filled by default with an empty array, i.e. "a:0:{}"..?
It's only six bytes. I've seen worse... I wouldn't recommend it, but I wouldn't be upset by this either.

Or, alternatively, we could force loading data into one (or all) of the three member profiles, and unserialize it, and defaulting it to an empty array. Problem solved, and no need to run another query. But you have to use member contexts, of course.

Also, keep in mind that we::$user['data'] contains the data array for the current member, *and* it's always defaulted to an empty array() if empty.

Re: JavaScript in admin, go crazy if you must. As I said before, non-admin areas are the ones I'm being careful with. In admin areas, comfort comes over performance or bandwidth concerns. By FAR! ;)

PS: my @Nao above didn't work, I guess it's because of the bug... I've uploaded a fixed version. Please @ me in return, @Arantor/@Dragooon/@whoever.
Title: Re: X-Content-Security-Policy
Post by: Arantor on March 25th, 2013, 08:11 PM
@Nao, 6 bytes every row? I'd rather not. When loading the row, check for empty and unserialize or initialising an empty array.

On loading this page:
Quote
Notice: Undefined index: member in Plugins/mentions/plugin.php on line 229

Notice: Undefined index: subject in Plugins/mentions/plugin.php on line 229

Notice: Undefined index: topic in Plugins/mentions/plugin.php on line 202
Title: Re: X-Content-Security-Policy
Post by: Nao on March 25th, 2013, 08:18 PM
I know, I'm getting tons of errors, not sure why... I've disabled these errors for now, but they probably break the mentions system entirely.

(I'm busy IRL, damn RL really!!! No time to fix any of these things for now... -_-)

IMPORTANT: doing a mention will break your post! It won't be sent. Be careful!! I'll try to fix it tonight. I don't know why the post above worked, though ;)
Title: Re: X-Content-Security-Policy
Post by: Nao on March 25th, 2013, 11:30 PM
Should be good now...
This was due, ahem, to a bug in Dragooon's fix of today(http://wedge.org/pub/feats/8003/x-content-security-policy/msg287323/#msg287323)... :lol:
He was using $data to store member data, when it was already used as a function parameter.
I renamed it to $member_data and all should be fine.

Also enabled (temporarily?) self-made mentions. @Nao!

:edit: Works!
Title: Re: X-Content-Security-Policy
Post by: Nao on March 26th, 2013, 12:47 AM
Well, all in all, that revision 2021 is a bad dream that @Dragooon should forget ASAP... Although it would help me save time if he could check his code for bugs before submitting patches..................... :whistle:



:niark:

Okay, time for bed........ Uh, actually it's nearly time to wake up for me ^^
Title: Re: X-Content-Security-Policy
Post by: live627 on March 26th, 2013, 01:32 AM
Nao doesn't like inferior code :niark: :P :whistle:
Title: Re: X-Content-Security-Policy
Post by: Arantor on March 26th, 2013, 01:37 AM
Who does?
Title: Re: X-Content-Security-Policy
Post by: Dragooon on March 26th, 2013, 05:37 AM
Sorry about that, I'll try to be more careful.