Skip to content
This repository has been archived by the owner on Feb 28, 2018. It is now read-only.

Upgrade SSL grade on Mozilla Observatory #207

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

davinirjr
Copy link

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling ecb6998 on davinirjr:davinirjr-nginx into ** on datasciencebr:master**.

@jtemporal
Copy link
Collaborator

Hi @davinirjr thank you for your PR! But I'm having a little trouble evaluating it, would you mind providing a better description of what was your idea here? :)

@davinirjr
Copy link
Author

No problem at all, @jtemporal.

Today, we have D+ on Mozilla's Observatory:

image

@cuducos and I talked about it, and I just helped on providing a more secure SSL config for Jarbas site.

He pointed that we could verify the changes on test environment that you're working on (since we need real certificates, and for Let's Encrypt, we need a fully functional public-DNS host).

@davinirjr
Copy link
Author

Better?

@cuducos
Copy link
Collaborator

cuducos commented Jul 3, 2017

Hi @davinirjr thank you for your PR! But I'm having a little trouble evaluating it

Hi @jtemporal, I'm sorry I missed that: I had a ~1h call with @davinirjr in which he explained line by line this nginx confs. Merging it here doesn't change anything in production (it only properly document the settings we're using).

So what's next? You or I should log into the production server, add the confs line by line testing its effect. If we can push all of them to production, then I consider this ready to merge. Does that make sense?

@jtemporal
Copy link
Collaborator

jtemporal commented Jul 3, 2017

Better?

Yes!

he explained line by line this nginx confs

For future reference: That would be nice to have reflected on the commit messages

Does that make sense?

It does.

I still have concerns on testing this in production though.

Another question (I'm really green on nginx and all of that): is there a reason why there are many commented cipher lines in there?

@davinirjr
Copy link
Author

davinirjr commented Jul 4, 2017

I still have concerns on testing this in production though.

Huge +1 here.

But to verify this PR we'll need a "live" server, aka, public DNS with certificates, as Observatory will not reach internal IP's and Let's Encrypt will not generate certificates either.

Do we have an live QA server with certificate? We can measure it before and after to make sure this will not break anything.

Another question (I'm really green on nginx and all of that): is there a reason why there are many commented cipher lines in there?

If we use better ciphers, we choose to drop out "older" browser support.

I don't think we could push and test line by line, as @cuducos says (there are lines that have some dependencies), but I can help on it too.

And nginx can test the conf file before it loads into running instance, using -t in cli:

https://www.nginx.com/resources/wiki/start/topics/tutorials/commandline/

EDIT:

Will be good pratice to include nginx -t on CI/CD.

@jtemporal
Copy link
Collaborator

Just to link it: This is supposed to fix #205

@jtemporal
Copy link
Collaborator

@filipecifali showed me this other SSL scanner the other day that showed a not so low score for Jarbas. Thought you guys might find it interesting =)

@davinirjr
Copy link
Author

Yep, Mozilla Observatory uses SSLLabs on "third-party" tests tab.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants