Skip to content

Commit 9c8ccc5

Browse files
committed
Merge pull request #178 from twitter/directive-cleanup
Directive cleanup
2 parents 11bb60b + e6e25a5 commit 9c8ccc5

File tree

6 files changed

+46
-62
lines changed

6 files changed

+46
-62
lines changed

README.md

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,23 @@ The following methods are going to be called, unless they are provided in a `ski
4242
config.x_download_options = 'noopen'
4343
config.x_permitted_cross_domain_policies = 'none'
4444
config.csp = {
45-
:default_src => "https: self",
45+
:default_src => "https: 'self'",
4646
:enforce => proc {|controller| controller.current_user.enforce_csp? },
4747
:frame_src => "https: http:.twimg.com http://itunes.apple.com",
4848
:img_src => "https:",
49+
:connect_src => "wws:"
50+
:font_src => "'self' data:",
51+
:frame_src => "'self'",
52+
:img_src => "mycdn.com data:",
53+
:media_src => "utoob.com",
54+
:object_src => "'self'",
55+
:script_src => "'self'",
56+
:style_src => "'unsafe-inline'",
57+
:base_uri => "'self'",
58+
:child_src => "'self'",
59+
:form_action => "'self' github.com",
60+
:frame_ancestors => "'none'",
61+
:plugin_types => 'application/x-shockwave-flash',
4962
:report_uri => '//example.com/uri-directive'
5063
}
5164
config.hpkp = {
@@ -152,7 +165,7 @@ This configuration will likely work for most applications without modification.
152165

153166
# Auction site wants to allow images from anywhere, plugin content from a list of trusted media providers (including a content distribution network), and scripts only from its server hosting sanitized JavaScript
154167
:csp => {
155-
:default_src => 'self',
168+
:default_src => "'self'",
156169
:img_src => '*',
157170
:object_src => ['media1.com', 'media2.com', '*.cdn.com'],
158171
# alternatively (NOT csv) :object_src => 'media1.com media2.com *.cdn.com'
@@ -191,8 +204,8 @@ Setting a nonce will also set 'unsafe-inline' for browsers that don't support no
191204

192205
```ruby
193206
:csp => {
194-
:default_src => 'self',
195-
:script_src => 'self nonce'
207+
:default_src => "'self'",
208+
:script_src => "'self' nonce"
196209
}
197210
```
198211

@@ -238,7 +251,7 @@ If you only have a few hashes, you can hardcode them for the entire app:
238251
```ruby
239252
config.csp = {
240253
:default_src => "https:",
241-
:script_src => 'self'
254+
:script_src => "'self'"
242255
:script_hashes => ['sha1-abc', 'sha1-qwe']
243256
}
244257
```
@@ -248,7 +261,7 @@ The following will work as well, but may not be as clear:
248261
```ruby
249262
config.csp = {
250263
:default_src => "https:",
251-
:script_src => "self 'sha1-qwe'"
264+
:script_src => "'self' 'sha1-qwe'"
252265
}
253266
```
254267

@@ -263,7 +276,7 @@ use ::SecureHeaders::ContentSecurityPolicy::ScriptHashMiddleware
263276
```ruby
264277
config.csp = {
265278
:default_src => "https:",
266-
:script_src => 'self',
279+
:script_src => "'self'",
267280
:script_hash_middleware => true
268281
}
269282
```

fixtures/rails_3_2_22/config/initializers/secure_headers.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
config.x_xss_protection = {:value => 1, :mode => 'block'}
66
config.x_permitted_cross_domain_policies = 'none'
77
csp = {
8-
:default_src => "self",
9-
:script_src => "self nonce",
8+
:default_src => "'self'",
9+
:script_src => "'self' nonce",
1010
:report_uri => 'somewhere',
1111
:script_hash_middleware => true,
1212
:enforce => false # false means warnings only

fixtures/rails_3_2_22_no_init/app/controllers/other_things_controller.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
class OtherThingsController < ApplicationController
2-
ensure_security_headers :csp => {:default_src => 'self'}
2+
ensure_security_headers :csp => {:default_src => "'self'"}
33
def index
44

55
end
@@ -11,7 +11,7 @@ def other_action
1111
def secure_header_options_for(header, options)
1212
if params[:action] == "other_action"
1313
if header == :csp
14-
options.merge(:style_src => 'self')
14+
options.merge(:style_src => "'self'")
1515
end
1616
else
1717
options

fixtures/rails_4_1_8/config/initializers/secure_headers.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
config.x_xss_protection = {:value => 0}
66
config.x_permitted_cross_domain_policies = 'none'
77
csp = {
8-
:default_src => "self",
9-
:script_src => "self nonce",
8+
:default_src => "'self'",
9+
:script_src => "'self' nonce",
1010
:report_uri => 'somewhere',
1111
:script_hash_middleware => true,
1212
:enforce => false # false means warnings only

lib/secure_headers/headers/content_security_policy.rb

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,26 +20,19 @@ module Constants
2020
:media_src,
2121
:object_src,
2222
:script_src,
23-
:style_src
24-
]
25-
26-
NON_DEFAULT_SOURCES = [
23+
:style_src,
2724
:base_uri,
2825
:child_src,
2926
:form_action,
3027
:frame_ancestors,
31-
:plugin_types,
32-
:referrer,
33-
:reflected_xss
28+
:plugin_types
3429
]
3530

3631
OTHER = [
3732
:report_uri
3833
]
3934

40-
SOURCE_DIRECTIVES = DIRECTIVES + NON_DEFAULT_SOURCES
41-
42-
ALL_DIRECTIVES = DIRECTIVES + NON_DEFAULT_SOURCES + OTHER
35+
ALL_DIRECTIVES = DIRECTIVES + OTHER
4336
CONFIG_KEY = :csp
4437
end
4538

@@ -111,7 +104,7 @@ def initialize(config=nil, options={})
111104
@config = config.inject({}) do |hash, (key, value)|
112105
config_val = value.respond_to?(:call) ? value.call(@controller) : value
113106

114-
if SOURCE_DIRECTIVES.include?(key) # directives need to be normalized to arrays of strings
107+
if DIRECTIVES.include?(key) # directives need to be normalized to arrays of strings
115108
config_val = config_val.split if config_val.is_a? String
116109
if config_val.is_a?(Array)
117110
config_val = config_val.map do |val|
@@ -191,7 +184,6 @@ def build_value
191184
append_http_additions unless ssl_request?
192185
header_value = [
193186
generic_directives,
194-
non_default_directives,
195187
report_uri_directive
196188
].join.strip
197189
end
@@ -258,15 +250,6 @@ def generic_directives
258250
header_value
259251
end
260252

261-
def non_default_directives
262-
header_value = ''
263-
NON_DEFAULT_SOURCES.each do |directive_name|
264-
header_value += build_directive(directive_name) if @config[directive_name]
265-
end
266-
267-
header_value
268-
end
269-
270253
def build_directive(key)
271254
"#{self.class.symbol_to_hyphen_case(key)} #{@config[key].join(" ")}; "
272255
end

spec/lib/secure_headers/headers/content_security_policy_spec.rb

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ def request_for user_agent, request_uri=nil, options={:ssl => false}
8282

8383
describe "#normalize_csp_options" do
8484
before(:each) do
85-
default_opts[:script_src] << ' self none'
85+
default_opts[:script_src] << " 'self' 'none'"
8686
@opts = default_opts
8787
end
8888

@@ -106,7 +106,7 @@ def request_for user_agent, request_uri=nil, options={:ssl => false}
106106

107107
it "accepts procs for report-uris" do
108108
opts = {
109-
:default_src => 'self',
109+
:default_src => "'self'",
110110
:report_uri => proc { "http://lambda/result" }
111111
}
112112

@@ -131,7 +131,7 @@ def request_for user_agent, request_uri=nil, options={:ssl => false}
131131

132132
allow(controller).to receive(:current_user).and_return(user)
133133
opts = {
134-
:default_src => "self",
134+
:default_src => "'self'",
135135
:enforce => lambda { |c| c.current_user.beta_testing? }
136136
}
137137
csp = ContentSecurityPolicy.new(opts, :controller => controller)
@@ -148,36 +148,24 @@ def request_for user_agent, request_uri=nil, options={:ssl => false}
148148
}.to raise_error(RuntimeError)
149149
end
150150

151-
context "CSP level 2 directives" do
152-
let(:config) { {:default_src => 'self'} }
153-
::SecureHeaders::ContentSecurityPolicy::Constants::NON_DEFAULT_SOURCES.each do |non_default_source|
154-
it "supports all level 2 directives" do
155-
directive_name = ::SecureHeaders::ContentSecurityPolicy.send(:symbol_to_hyphen_case, non_default_source)
156-
config.merge!({ non_default_source => "value" })
157-
csp = ContentSecurityPolicy.new(config, :request => request_for(CHROME))
158-
expect(csp.value).to match(/#{directive_name} value;/)
159-
end
160-
end
161-
end
162-
163151
context "auto-whitelists data: uris for img-src" do
164152
it "sets the value if no img-src specified" do
165-
csp = ContentSecurityPolicy.new({:default_src => 'self'}, :request => request_for(CHROME))
153+
csp = ContentSecurityPolicy.new({:default_src => "'self'"}, :request => request_for(CHROME))
166154
expect(csp.value).to eq("default-src 'self'; img-src 'self' data:;")
167155
end
168156

169157
it "appends the value if img-src is specified" do
170-
csp = ContentSecurityPolicy.new({:default_src => 'self', :img_src => 'self'}, :request => request_for(CHROME))
158+
csp = ContentSecurityPolicy.new({:default_src => "'self'", :img_src => "'self'"}, :request => request_for(CHROME))
171159
expect(csp.value).to eq("default-src 'self'; img-src 'self' data:;")
172160
end
173161

174162
it "doesn't add a duplicate data uri if img-src specifies it already" do
175-
csp = ContentSecurityPolicy.new({:default_src => 'self', :img_src => 'self data:'}, :request => request_for(CHROME))
163+
csp = ContentSecurityPolicy.new({:default_src => "'self'", :img_src => "'self' data:"}, :request => request_for(CHROME))
176164
expect(csp.value).to eq("default-src 'self'; img-src 'self' data:;")
177165
end
178166

179167
it "allows the user to disable img-src data: uris auto-whitelisting" do
180-
csp = ContentSecurityPolicy.new({:default_src => 'self', :img_src => 'self', :disable_img_src_data_uri => true}, :request => request_for(CHROME))
168+
csp = ContentSecurityPolicy.new({:default_src => "'self'", :img_src => "'self'", :disable_img_src_data_uri => true}, :request => request_for(CHROME))
181169
expect(csp.value).to eq("default-src 'self'; img-src 'self';")
182170
end
183171
end
@@ -203,47 +191,47 @@ def request_for user_agent, request_uri=nil, options={:ssl => false}
203191

204192
context "when using a nonce" do
205193
it "adds a nonce and unsafe-inline to the script-src value when using chrome" do
206-
header = ContentSecurityPolicy.new(default_opts.merge(:script_src => "self nonce"), :request => request_for(CHROME), :controller => controller)
194+
header = ContentSecurityPolicy.new(default_opts.merge(:script_src => "'self' nonce"), :request => request_for(CHROME), :controller => controller)
207195
expect(header.value).to include("script-src 'self' 'nonce-#{header.nonce}' 'unsafe-inline'")
208196
end
209197

210198
it "adds a nonce and unsafe-inline to the script-src value when using firefox" do
211-
header = ContentSecurityPolicy.new(default_opts.merge(:script_src => "self nonce"), :request => request_for(FIREFOX), :controller => controller)
199+
header = ContentSecurityPolicy.new(default_opts.merge(:script_src => "'self' nonce"), :request => request_for(FIREFOX), :controller => controller)
212200
expect(header.value).to include("script-src 'self' 'nonce-#{header.nonce}' 'unsafe-inline'")
213201
end
214202

215203
it "adds a nonce and unsafe-inline to the script-src value when using opera" do
216-
header = ContentSecurityPolicy.new(default_opts.merge(:script_src => "self nonce"), :request => request_for(OPERA), :controller => controller)
204+
header = ContentSecurityPolicy.new(default_opts.merge(:script_src => "'self' nonce"), :request => request_for(OPERA), :controller => controller)
217205
expect(header.value).to include("script-src 'self' 'nonce-#{header.nonce}' 'unsafe-inline'")
218206
end
219207

220208
it "does not add a nonce and unsafe-inline to the script-src value when using Safari" do
221-
header = ContentSecurityPolicy.new(default_opts.merge(:script_src => "self nonce"), :request => request_for(SAFARI), :controller => controller)
209+
header = ContentSecurityPolicy.new(default_opts.merge(:script_src => "'self' nonce"), :request => request_for(SAFARI), :controller => controller)
222210
expect(header.value).to include("script-src 'self' 'unsafe-inline'")
223211
expect(header.value).not_to include("nonce")
224212
end
225213

226214
it "does not add a nonce and unsafe-inline to the script-src value when using IE" do
227-
header = ContentSecurityPolicy.new(default_opts.merge(:script_src => "self nonce"), :request => request_for(IE), :controller => controller)
215+
header = ContentSecurityPolicy.new(default_opts.merge(:script_src => "'self' nonce"), :request => request_for(IE), :controller => controller)
228216
expect(header.value).to include("script-src 'self' 'unsafe-inline'")
229217
expect(header.value).not_to include("nonce")
230218
end
231219

232220
it "adds a nonce and unsafe-inline to the style-src value" do
233-
header = ContentSecurityPolicy.new(default_opts.merge(:style_src => "self nonce"), :request => request_for(CHROME), :controller => controller)
221+
header = ContentSecurityPolicy.new(default_opts.merge(:style_src => "'self' nonce"), :request => request_for(CHROME), :controller => controller)
234222
expect(header.value).to include("style-src 'self' 'nonce-#{header.nonce}' 'unsafe-inline'")
235223
end
236224

237225
it "adds an identical nonce to the style and script-src directives" do
238-
header = ContentSecurityPolicy.new(default_opts.merge(:style_src => "self nonce", :script_src => "self nonce"), :request => request_for(CHROME), :controller => controller)
226+
header = ContentSecurityPolicy.new(default_opts.merge(:style_src => "'self' nonce", :script_src => "'self' nonce"), :request => request_for(CHROME), :controller => controller)
239227
nonce = header.nonce
240228
value = header.value
241229
expect(value).to include("style-src 'self' 'nonce-#{nonce}' 'unsafe-inline'")
242230
expect(value).to include("script-src 'self' 'nonce-#{nonce}' 'unsafe-inline'")
243231
end
244232

245233
it "does not add 'unsafe-inline' twice" do
246-
header = ContentSecurityPolicy.new(default_opts.merge(:script_src => "self nonce 'unsafe-inline'"), :request => request_for(CHROME), :controller => controller)
234+
header = ContentSecurityPolicy.new(default_opts.merge(:script_src => "'self' nonce 'unsafe-inline'"), :request => request_for(CHROME), :controller => controller)
247235
expect(header.value).to include("script-src 'self' 'nonce-#{header.nonce}' 'unsafe-inline';")
248236
end
249237
end
@@ -291,7 +279,7 @@ def request_for user_agent, request_uri=nil, options={:ssl => false}
291279

292280
describe ".add_to_env" do
293281
let(:controller) { double }
294-
let(:config) { {:default_src => 'self'} }
282+
let(:config) { {:default_src => "'self'"} }
295283
let(:options) { {:controller => controller} }
296284

297285
it "adds metadata to env" do

0 commit comments

Comments
 (0)