Skip to content
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.

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

T5796:backport-add/fixed OCSERV HTTP security headers #2572

Merged
merged 2 commits into from Dec 16, 2023
Merged

Conversation

fett0
Copy link
Contributor

@fett0 fett0 commented Dec 4, 2023

Change Summary

http security headers (cherry picked from commit db51546)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Related PR(s)

Component(s) name

openconnect

Proposed changes

How to test

vyos@dco1:~$    show openconnect-server sessions
Interface    Username    IP             Remote IP     RX       TX         State      Uptime
-----------  ----------  -------------  ------------  -------  ---------  ---------  --------
sslvpn0      tst         172.20.20.198  192.168.0.40  37.7 KB  152 bytes  connected  1m:43s

00:12:14 dco1 ocserv-worker[7112]: main: CN=oc-srv,O=VyOS,L=Mycity,ST=Delaware,C=US certificate key usage prevents key encipherment; unable to support the RSA ciphersuites; if that is not intent>
Dec 02 00:12:14 dco1 ocserv[7112]: note: setting 'file' as supplemental config option
Dec 02 00:12:18 dco1 ocserv[5989]: sec-mod: sec-mod instance 0 issue cookie
Dec 02 00:12:18 dco1 ocserv[5989]: sec-mod: using 'plain' authentication to authenticate user (session: sBJX91)
Dec 02 00:12:21 dco1 ocserv[5989]: sec-mod: initiating session for user 'tst' (session: sBJX91)
Dec 02 11:49:37 dco1 ocserv[7581]: Parsing plain auth method subconfig using legacy format
Dec 02 11:49:37 dco1 ocserv[7581]: note: vhost:default: setting 'plain' as primary authentication method
Dec 02 11:49:37 dco1 ocserv[7581]: note: setting 'file' as supplemental config option
Dec 02 11:49:37 dco1 ocserv-worker[7581]: main: CN=oc-srv,O=VyOS,L=Mycity,ST=Delaware,C=US certificate key usage prevents key encipherment; unable to support the RSA ciphersuites; if that is not intent>
Dec 02 11:49:40 dco1 ocserv[5989]: sec-mod: sec-mod instance 0 issue cookie
Dec 02 11:49:40 dco1 ocserv[5989]: sec-mod: using 'plain' authentication to authenticate user (session: v61ylj)
Dec 02 11:50:11 dco1 ocserv[5989]: sec-mod: initiating session for user 'tst' (session: v61ylj)

Smoketest result

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@vyosbot vyosbot requested review from a team, dmbaturin, sarthurdev, zdc, jestabro, sever-sever and c-po and removed request for a team December 4, 2023 20:00
@dmbaturin
Copy link
Member

All those options look like they can be good idea, but I feel really uneasy about changing the hardcoded default setup without giving people a way to disable any of those options. I believe we can only make such changes if we are absolutely certain that they cannot have any adverse effects. I think we should get back to this after 1.3.5 is out.

@fett0
Copy link
Contributor Author

fett0 commented Dec 4, 2023

I'll leave a context here : those headers were tested by the community / reported as well , basically they are old http vulnerabilities and also are recommend in default template from ocserv :

https://forum.vyos.io/t/ocserv-ocserv-config-tmpl-missing-security-headers-from-original-package/12989

https://gitlab.com/openconnect/ocserv/-/blob/master/doc/sample.config?ref_type=heads

but I agree with applied after 1.3.5 , at least we've introduced in 1.4/1.5

@sever-sever
Copy link
Member

Let's make it configurable with

set vpn openconnect http-security-headers

For all versions
By default disable it.

Copy link
Member

@sever-sever sever-sever left a comment

Choose a reason for hiding this comment

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

Let's make it configurable but not by default

@sever-sever sever-sever added the equuleus VyOS 1.3 LTS label Dec 8, 2023
@fett0
Copy link
Contributor Author

fett0 commented Dec 15, 2023

@sever-sever @dmbaturin was added the command to enable http headers :
set vpn openconnect http-security-headers

test 1.3.X configuration :

vyos@vyos:~$ show configuration commands  | match vpn
set vpn openconnect authentication local-users username user4 password 'SecretPassword'
set vpn openconnect authentication mode 'local'
set vpn openconnect http-security-headers
set vpn openconnect network-settings client-ip-settings subnet '100.64.0.0/24'
set vpn openconnect network-settings name-server '10.1.1.1'
set vpn openconnect network-settings name-server '10.1.1.2'
set vpn openconnect ssl ca-cert-file '/config/auth/ca.crt'
set vpn openconnect ssl cert-file '/config/auth/server.crt'
set vpn openconnect ssl key-file '/config/auth/server.key'

@c-po c-po merged commit e5fcf49 into vyos:equuleus Dec 16, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
equuleus VyOS 1.3 LTS
4 participants