starlightprivate / flash2

nodejs app
0 stars 0 forks source link

Make application send proper security related headers #220

Open vodolaz095 opened 7 years ago

vodolaz095 commented 7 years ago

What do i propose: https://starlightads.slack.com/messages/C4DNQFD7V/

@shahmeer @vladislavt i'd like to discuss security headers being send by application.

Currently, i setup many headers being send by nodejs app.

https://github.com/starlightgroup/flash2/blob/master/app.js#L81-L120

That's why:

  1. when developers start application locally, they already has all headers like on production app

  2. i have added unit tests to verify that headers are actually send - https://github.com/starlightgroup /flash2/blob/master/test/api.spec.js#L50-L116i have recieved requests from @vladislavt to add cors header.

As far as i understand it, https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS our site has both API and static files being served from same origin (!!!!)

and it turns out that @vladislavt nginx config sends vairous headers not intended to be send by nodejs application, i think this approach (part headers in nginx, part headeds in nodejs application) is quite bug prone.

I ask @shahmeer to clarify the situation.

I think

  1. headers HAVE TO BE SEND BY NODEJS APPLICTION
  2. if any headers are missing being send by nodejs app, i'll add them to nodejs app with tests, not to nginx

What @shahmeer-star explained me


Anatolij Ostroumov
6:19 PM
for headers thread?
https://starlightads.slack.com/archives/C4DNQFD7V/p1490620618522534What to do?
Summary
1. i think headers have to be send by nodejs app (so developers has more sane local environment, and with unit tests)
2. headers do not have to be send by nginx - it make development and other environments quite different - hard to find errors
2. i can add headers required to nodejs app with unit tests, just say what to add
Shahmeer Amir
6:21 PM
Understood
6:21
Add XFO, X Content options
Anatolij Ostroumov
6:21 PM
as far as i understand, XFO - is cross site requests forgery protection? yes?
Shahmeer Amir
6:22 PM
Clickjacking
Anatolij Ostroumov
6:22 PM
so, it is cross site requests forgery protection
6:22
what value of the header have it be?
Shahmeer Amir
6:23 PM
No, it is for clickjscking protection set to same origin
Anatolij Ostroumov
6:23 PM
https://github.com/starlightgroup/flash2/blob/b4f1a8b7389c66df01649a1d2bc3081abdc363af/test/api.spec.js#L51-L51
6:23

  it('have X-Frame-Options set to "DENY"', (done) => {
    supertest(app)
      .get('/tacticalsales/')
      .expect('X-Powered-By', 'TacticalMastery')
      .expect('X-Frame-Options', 'DENY')
      .end(done);
  });

6:23
it was set to be like this
6:23
so it have to be same origin?
6:24
'X-Frame-Options':'DENY'
6:24
yes?
6:25
and, as far as i understand you with X Content options, it means this:

  it('have X-Content-Type-Options set to "nosniff"', (done) => {
    supertest(app)
      .get('/tacticalsales/')
      .expect('X-Powered-By', 'TacticalMastery')
      .expect('X-Content-Type-Options', 'nosniff')
      .end(done);
  });

6:25
if yes, it means all this headers are already set by nodejs application, it is even included in unit tests called test/api/spec.js
6:25
probably they can be tampered by nginx settings, i'll verify it
6:28
it is worth notice, that nginx also set headers
https://github.com/starlightgroup/flash2-nginx/blob/production/nginx/nginx.conf
https://github.com/starlightgroup/flash2-nginx/blob/develop/nginx/nginx.conf
6:29
if both nginx and application sends headers, the values from nginx config are used
6:29
i think it is not correct, the ones from nodejs app have to be used
6:29
i explained it in threaded post
Shahmeer Amir
6:29 PM
I know
6:29
Gimme a minite for dinner

What have i understood?

@shahmeer-star had dinner.

What headers to send, alongside the ones already send by nodejs application - no idea.