Skip to content

Improve and extend the security checklist #2565

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 29 commits into from
Dec 5, 2024
Merged

Improve and extend the security checklist #2565

merged 29 commits into from
Dec 5, 2024

Conversation

@glye
Copy link
Contributor

@glye glye commented Dec 2, 2024 •
edited
Loading

Question Answer
JIRA Ticket N/A
Versions All
Edition All

Reorganised the checklist to make it more intuitive, and added more advice:

  • Moved Ibexa DXP to the top, above Symfony and PHP.
  • Added Web server and Database sections.
  • Moved some items from the Ibexa DXP section to the new sections.
  • Added Domain section.
  • Added advice on TLS, HSTS, DNSSEC, CAA, and domain update protection.

Preview: https://ez-systems-developer-documentation--2565.com.readthedocs.build/en/2565/infrastructure_and_maintenance/security/security_checklist/

Checklist

  • Text renders correctly
  • Text has been checked with vale
  • Description metadata is up to date
  • Redirects cover removed/moved pages
  • Code samples are working (no changes to code samples)
  • PHP code samples have been fixed with PHP CS fixer (no changes to code samples)
  • Added link to this PR in relevant JIRA ticket or code PR

@glye
Reorganised the security checklist
600931b 
Reorganised the checklist to make it more intuitive.
glye
glye commented Dec 2, 2024
View reviewed changes
Copy link
Contributor Author

@glye glye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review.

@reithor
Copy link
Contributor

reithor commented Dec 2, 2024

regarding 'powered by' we should probably give explicit instructions, like:

ibexa_system_info:
  system_info:
    powered_by:
      # major => v4 || minor => v4.6 || none
      release: major
      # true || false
      enabled: true

@adriendupuis adriendupuis mentioned this pull request Dec 3, 2024
Closed
7 tasks
@glye glye requested a review from a team December 3, 2024 09:45
bdunogier
bdunogier reviewed Dec 3, 2024
View reviewed changes
reithor
reithor reviewed Dec 3, 2024
View reviewed changes
Copy link
Contributor

@reithor reithor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me (as far my technical knowledge goes here).
One thing regarding "Security headers":
Probably we should review our cloud vcl for varnish and fastly, if we are following our own guidelines?

@glye
Copy link
Contributor Author

glye commented Dec 3, 2024

@reithor I and others have held back on the security headers due to the need to adapt them to the site in question. Generally there isn't a "safe setting" that can be applied for everyone. But I have been reconsidering. We could change vcl and vhost templates, they won't affect existing sites. We could set at least Strict-Transport-Security, and X-Content-Type-Options to nosniff. For the admin ui we could set all of them.

adriendupuis
adriendupuis reviewed Dec 3, 2024
View reviewed changes
dabrt
dabrt requested changes Dec 3, 2024
View reviewed changes
mnocon
mnocon reviewed Dec 3, 2024
View reviewed changes
Copy link
Contributor

@mnocon mnocon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some smaller nitpicks - the HSTS header with Varnish being the most important one

glye and others added 5 commits December 3, 2024 13:46
@glye
Secrets storage and .gitignore
fe92994
@glye
Carefully select admin users
317edf9
@glye @dabrt
Symfony docs
382d18e 
Co-authored-by: Tomasz Dąbrowski <[email protected]>
@glye @dabrt
Wording
e5a00d6 
Co-authored-by: Tomasz Dąbrowski <[email protected]>
@glye @dabrt
Wording
37d4126 
Co-authored-by: Tomasz Dąbrowski <[email protected]>
glye and others added 14 commits December 3, 2024 13:58
@glye @dabrt
Wording
e0a70fa 
Co-authored-by: Tomasz Dąbrowski <[email protected]>
@glye
TLS 1.3 usage
6ed60e8
@glye @dabrt
Monospace filename
12940d6 
Co-authored-by: Tomasz Dąbrowski <[email protected]>
@glye @dabrt
includeSubDomains
7ef336f 
Co-authored-by: Tomasz Dąbrowski <[email protected]>
@glye @dabrt
Monospace filename
5b4032d 
Co-authored-by: Tomasz Dąbrowski <[email protected]>
@glye @dabrt
DNSSEC & ICANN
42a59ef 
Co-authored-by: Tomasz Dąbrowski <[email protected]>
@glye @dabrt
Wording
8fd2c58 
Co-authored-by: Tomasz Dąbrowski <[email protected]>
@glye @adriendupuis
Make random_bytes command copyable
c414e93 
Co-authored-by: Adrien Dupuis <[email protected]>
@glye @mnocon
OWASP are more up to date on PHP now 💪
48da5c3 
Co-authored-by: Marek Nocoń <[email protected]>
@glye @mnocon
Wording
88f8590 
Co-authored-by: Marek Nocoń <[email protected]>
@glye
Keep users up to date
377d9c8
@glye
Merged Secure secrets into APP_SECRET
6941f05
@glye
HSTS and Varnish
7eb8d61
@glye
Removed database rights, added monitor logs
26d5a78
@glye glye requested a review from dabrt December 3, 2024 15:50
@glye
Copy link
Contributor Author

glye commented Dec 3, 2024

Thanks for all the reviews, very useful. I believe I have answered all comments and applied all suggestions now, and it's looking good. Please have another look, if you had concerns earlier.

reithor
reithor approved these changes Dec 5, 2024
View reviewed changes
Copy link
Contributor

@reithor reithor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me now!

@adriendupuis adriendupuis merged commit 270e2e3 into master Dec 5, 2024
6 checks passed
@adriendupuis adriendupuis deleted the improve_security_checklist branch December 5, 2024 13:49
adriendupuis added a commit that referenced this pull request Dec 5, 2024
@glye @dabrt
@adriendupuis @mnocon
Improve and extend the security checklist (#2565)
052cb71 
---------

Co-authored-by: Tomasz Dąbrowski <[email protected]>
Co-authored-by: Adrien Dupuis <[email protected]>
Co-authored-by: Marek Nocoń <[email protected]>
(cherry picked from commit 270e2e3)
adriendupuis pushed a commit that referenced this pull request Dec 5, 2024
@glye @adriendupuis
Improve and extend the security checklist (#2565)
1fd6457 
---------

Co-authored-by: Tomasz Dąbrowski <[email protected]>
Co-authored-by: Adrien Dupuis <[email protected]>
Co-authored-by: Marek Nocoń <[email protected]>

(cherry picked from commit 270e2e3)
@adriendupuis adriendupuis mentioned this pull request Dec 5, 2024
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vidarl vidarl vidarl approved these changes

@mnocon mnocon mnocon approved these changes

@reithor reithor reithor approved these changes

@adriendupuis adriendupuis adriendupuis approved these changes

@dabrt dabrt dabrt approved these changes

@julitafalcondusza julitafalcondusza Awaiting requested review from julitafalcondusza

+1 more reviewer

@bdunogier bdunogier bdunogier approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Needs DOC review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@glye @reithor @bdunogier @vidarl @mnocon @adriendupuis @dabrt