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

T5683: Fix reverse-proxy PKI filenames mismatch #2405

Merged
merged 1 commit into from
Oct 25, 2023
Merged

Conversation

sever-sever
Copy link
Member

@sever-sever sever-sever commented Oct 25, 2023

Change Summary

The current names for certificates are hardcoded in the generated config to:

 - ca.pem
 - cert.pem.key
 - cert.pem

It causes a generated config certificates and certificates themselves are different (test-ca-1.pem vs ca.pem)

It is a bug in the initial implementation. Fix required correct names from PKI certificates.

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

reverse-proxy

Proposed changes

How to test

VyOS configuration:

set pki ca test-ca-1 certificate 'REDACTED'
set pki certificate test-cert-1 certificate 'REDACTED'
set pki certificate test-cert-1 private key 'REDACTED'

set load-balancing reverse-proxy backend test-backend-1 mode 'http'
set load-balancing reverse-proxy backend test-backend-1 server test-server-1 address '10.11.12.1'
set load-balancing reverse-proxy backend test-backend-1 server test-server-1 port '443'
set load-balancing reverse-proxy backend test-backend-1 ssl ca-certificate 'test-ca-1'
set load-balancing reverse-proxy service test-frontend-1 mode 'http'
set load-balancing reverse-proxy service test-frontend-1 port '8080'
set load-balancing reverse-proxy service test-frontend-1 ssl certificate 'test-cert-1'

commit

Before the fix we had wrong certificate names:

vyos@r4# ls -l /run/haproxy/
total 16
-rw-r--r-- 1 root haproxy 1294 Oct 25 12:54 ca.pem
-rw-r--r-- 1 root haproxy 1322 Oct 25 12:54 cert.pem
-rw-r--r-- 1 root haproxy 1678 Oct 25 12:54 cert.pem.key
-rw-r--r-- 1 root haproxy 1788 Oct 25 12:54 haproxy.cfg
[edit]
vyos@r4# 

vyos@r4# haproxy -c -- /run/haproxy/haproxy.cfg 
[NOTICE]   (6741) : haproxy version is 2.6.12-1
[NOTICE]   (6741) : path to executable is /usr/sbin/haproxy
[ALERT]    (6741) : config : parsing [/run/haproxy/haproxy.cfg:40] : 'bind :::8080' in section 'frontend' : unable to stat SSL certificate from file '/run/haproxy/test-cert-1.pem' : No such file or directory.
[ALERT]    (6741) : config : [/run/haproxy/haproxy.cfg:51] : 'server test-backend-1/test-server-1' : Couldn't open the ca-file '/run/haproxy/test-ca-1.pem' (No such file or directory).
[ALERT]    (6741) : config : 'ca-file' : unable to load /run/haproxy/test-ca-1.pem.
[ALERT]    (6741) : config : Error(s) found in configuration file : /run/haproxy/haproxy.cfg
[ALERT]    (6741) : config : Fatal errors found in configuration.
[edit]
vyos@r4#

haproxy config:

# Frontend
frontend test-frontend-1
    bind :::8080 v4v6 ssl crt /run/haproxy/test-cert-1.pem
    mode http


# Backend
backend test-backend-1
    balance roundrobin
    option forwardfor
    http-request set-header X-Forwarded-Port %[dst_port]
    http-request add-header X-Forwarded-Proto https if { ssl_fc }
    mode http
    server test-server-1 10.11.12.1:443 ssl ca-file /run/haproxy/test-ca-1.pem

After the fix we have correct certificate names:

vyos@r4# ls -la /run/haproxy/
total 16
drwxr-xr-x  2 root root  140 Oct 25 13:11 .
drwxr-xr-x 38 root root 1160 Oct 25 13:11 ..
srw-rw----  1 root root    0 Oct 25 13:11 admin.sock
-rw-r--r--  1 root root 1788 Oct 25 13:11 haproxy.cfg
-rw-r--r--  1 root root 1294 Oct 25 13:11 test-ca-1.pem
-rw-r--r--  1 root root 1322 Oct 25 13:11 test-cert-1.pem
-rw-r--r--  1 root root 1678 Oct 25 13:11 test-cert-1.pem.key
[edit]
vyos@r4# 

vyos@r4# haproxy -c -- /run/haproxy/haproxy.cfg 
Configuration file is valid
[edit]
vyos@r4# 

Smoketest result

vyos@r4:~$ /usr/libexec/vyos/tests/smoke/cli/test_load_balancing_reverse_proxy.py
test_01_lb_reverse_proxy_domain (__main__.TestLoadBalancingReverseProxy.test_01_lb_reverse_proxy_domain) ... ok

----------------------------------------------------------------------
Ran 1 test in 5.974s

OK

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

The current named for certificates are hardcoded in generated config to:
 - ca.pem
 - cert.pem.key
 - cert.pem

It cause a generated config certificates and certificates itself
are different (test-cert-1.pem and ca.pem)

  bind :::8080 v4v6 ssl crt /run/haproxy/test-cert-1.pem

  /run/haproxy/ca.pem

It is a bug of initial impelemtation. Fix required correct names
from PKI certificates
@vyosbot vyosbot requested review from a team, dmbaturin, sarthurdev, zdc, jestabro and c-po and removed request for a team October 25, 2023 10:31
@c-po c-po merged commit 73eb777 into vyos:current Oct 25, 2023
@c-po
Copy link
Member

c-po commented Oct 25, 2023

@Mergifyio backport sagitta

@mergify
Copy link
Contributor

mergify bot commented Oct 25, 2023

backport sagitta

✅ Backports have been created

sever-sever added a commit that referenced this pull request Oct 26, 2023
T5683: Fix reverse-proxy PKI filenames mismatch (backport #2405)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants