[THREESCALE-328] Allow to set a client SSL cert for upstream #610

davidor · 2018-02-19T14:27:56Z

This PR introduces 2 new ENVs (APICAST_PROXY_HTTPS_CERTIFICATE_KEY and APICAST_PROXY_HTTPS_CERTIFICATE), they are used to set a client SSL certificate using the proxy_ssl_certificate and the proxy_ssl_certificate_key NGINX directives.

As a limitation, this certificate needs to be valid for all the services configured.

Closes THREESCALE-328.

mikz · 2018-02-19T15:45:01Z

gateway/conf.d/apicast.conf

+
+  # {% if proxy_ssl_certificate and proxy_ssl_certificate_key %}
+  #   {% capture proxy_ssl_session_reuse %}
+  #   proxy_ssl_session_reuse off;


Maybe we should expose this as a config option with a default value so it can be customized.

Maybe, but I couldn't make this work properly with proxy_ssl_session_reuse on;

mikz · 2018-02-19T18:25:47Z

gateway/conf.d/apicast.conf

+  #   proxy_ssl_certificate {{ proxy_ssl_certificate }};
+  #   {% endcapture %}
+  #   {% capture ssl_certificate_key %}
+  #   proxy_ssl_certificate_key {{ proxy_ssl_certificate_key }};


Lets not forget proxy_ssl_password_file.

Yes. Otherwise, we'll need to type the password when Apicast boots.

mikz · 2018-02-19T18:28:18Z

gateway/src/apicast/cli/environment.lua

 -- @tfield ?policy_chain policy_chain @{policy_chain} instance
 -- @tfield ?{string,...} nameservers list of nameservers
 -- @tfield ?string package.path path to load Lua files
 -- @tfield ?string package.cpath path to load libraries
 -- @table environment.default_config default configuration
 _M.default_config = {
    ca_bundle = resty_env.value('SSL_CERT_FILE'),
+    proxy_ssl_certificate = resty_env.value('SSL_PROXY_CERTIFICATE'),


We probably should covert them to absolute paths because it fails with relative ones as it changes nginx workdir/prefix with:

nginx: [emerg] BIO_new_file("/tmp/public.crt") failed (SSL: error:02001002:system library:fopen:No such file or directory:fopen('/tmp/public.crt','r') error:2006D080:BIO routines:BIO_new_file:no such file)

But we also should support engine:name:id format which would load the key from OpenSSL.

mikz · 2018-02-19T18:52:00Z

gateway/conf.d/apicast.conf

+  #   {% endcapture %}
+  #   {{ proxy_ssl_session_reuse | replace: "#", "" }}
+  #   {{ ssl_certificate | replace: "#", "" }}
+  #   {{ ssl_certificate_key | replace: "#", "" }}


I find this working quite well:

# {% if proxy_ssl_certificate and proxy_ssl_certificate_key %} # {% capture proxy_ssl %} #{#} proxy_ssl_session_reuse off; #{#} proxy_ssl_certificate {{ proxy_ssl_certificate }}; #{#} proxy_ssl_certificate_key {{ proxy_ssl_certificate_key }}; # {% endcapture %} # {{ proxy_ssl | replace: "#{#}", "" }} # {% endif %}

This makes it obvious that it should strip only the first part of the comment and allows commenting lines within comments.

Good idea 👍

mikz · 2018-02-19T18:58:53Z

gateway/src/apicast/cli/environment.lua

 -- @tfield ?policy_chain policy_chain @{policy_chain} instance
 -- @tfield ?{string,...} nameservers list of nameservers
 -- @tfield ?string package.path path to load Lua files
 -- @tfield ?string package.cpath path to load libraries
 -- @table environment.default_config default configuration
 _M.default_config = {
    ca_bundle = resty_env.value('SSL_CERT_FILE'),
+    proxy_ssl_certificate = resty_env.value('SSL_PROXY_CERTIFICATE'),
+    proxy_ssl_certificate_key = resty_env.value('SSL_PROXY_CERTIFICATE_KEY'),


We probably could name these like APICAST_PROXY_* (APICAST_PROXY_CERTIFICATE and APICAST_PROXY_CERTIFICATE_KEY).

APICAST_PROXY is not the best name. Maybe APICAST_UPSTREAM_HTTPS ?

davidor · 2018-02-19T22:33:55Z

gateway/conf.d/apicast.conf

+
+  # {% if proxy_ssl_certificate and proxy_ssl_certificate_key %}
+  #   {% capture proxy_ssl %}
+  #{#}   # proxy_ssl_session_reuse off;


We'll need to set this one according to APICAST_PROXY_SESSION_REUSE.

davidor · 2018-02-19T22:34:35Z

t/mutual-ssl.t

+use Test::APIcast::Blackbox 'no_plan';
+
+env_to_apicast(
+    'APICAST_PROXY_CERTIFICATE' => '../html/server.crt',


These should be client.crt and client.key.

davidor · 2018-02-19T22:34:59Z

t/mutual-ssl.t

+  ssl_certificate $TEST_NGINX_SERVROOT/html/server.crt;
+  ssl_certificate_key $TEST_NGINX_SERVROOT/html/server.key;
+
+  ssl_client_certificate $TEST_NGINX_SERVROOT/html/server.crt;


davidor · 2018-02-20T09:30:43Z

examples/ssl-verification/badssl.json

+      "proxy": {
+        "api_backend": "https://client.badssl.com",
+        "backend": {
+          "endpoint": "https://echo-api.3scale.net"


Yep. But I could not figure it out from the debug logs. Haven't done wireshark yet.

* so they are read at the very last moment * this allows updating them in between loadng config and rendering the template * depends on 3scale/liquid-lua#4, 3scale/liquid-lua#5

depends on 3scale/Test-APIcast#4

…ERTIFICATE_KEY [THREESCALE-328]

mikz

👍

octobot assigned davidor Feb 19, 2018

davidor requested a review from mikz February 19, 2018 14:27

mikz mentioned this pull request Feb 19, 2018

Define the nginx TLS configuration using environment variables #609

Closed

mikz reviewed Feb 19, 2018

View reviewed changes

mikz mentioned this pull request Feb 19, 2018

An APIcast policy that enables SSL/TLS client certificate authentication #603

Closed

mikz reviewed Feb 19, 2018

View reviewed changes

davidor commented Feb 19, 2018

View reviewed changes

mikz mentioned this pull request Feb 20, 2018

serialize variables that have __tostring in their metatable 3scale/liquid-lua#4

Merged

mikz force-pushed the configurable-proxy-ssl-cert branch 6 times, most recently from e267984 to e9e237f Compare February 20, 2018 09:25

davidor commented Feb 20, 2018

View reviewed changes

mikz force-pushed the configurable-proxy-ssl-cert branch from e9e237f to 5da2d65 Compare February 20, 2018 09:40

mikz added 2 commits February 20, 2018 11:52

[liquid] env variables are loaded when serialized

a9544a5

* so they are read at the very last moment * this allows updating them in between loadng config and rendering the template * depends on 3scale/liquid-lua#4, 3scale/liquid-lua#5

[resty.resolver][ allow overriding upstream port from resolver

776fe76

depends on 3scale/Test-APIcast#4

mikz force-pushed the configurable-proxy-ssl-cert branch from 8288105 to cc4dbfb Compare February 20, 2018 10:57

cli/environment: set ssl cert and key according to ENV

66f77bb

mikz force-pushed the configurable-proxy-ssl-cert branch from cc4dbfb to 66f77bb Compare February 20, 2018 11:13

doc/parameters: add APICAST_PROXY_HTTPS ENVs

850a2a5

davidor force-pushed the configurable-proxy-ssl-cert branch from a3cce31 to 850a2a5 Compare February 20, 2018 11:54

mikz changed the title ~~[WIP] Allow to set a client SSL cert for upstream~~ [THREESCALE-328] Allow to set a client SSL cert for upstream Feb 20, 2018

[changelog] APICAST_PROXY_HTTPS_CERTIFICATE and APICAST_PROXY_HTTPS_C…

071e059

…ERTIFICATE_KEY [THREESCALE-328]

mikz approved these changes Feb 20, 2018

View reviewed changes

mikz merged commit dab3c18 into master Feb 20, 2018

mikz deleted the configurable-proxy-ssl-cert branch February 20, 2018 12:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[THREESCALE-328] Allow to set a client SSL cert for upstream #610

[THREESCALE-328] Allow to set a client SSL cert for upstream #610

davidor commented Feb 19, 2018 •

edited by mayorova

Loading

mikz Feb 19, 2018

davidor Feb 19, 2018

mikz Feb 19, 2018

davidor Feb 19, 2018

mikz Feb 19, 2018

mikz Feb 19, 2018

davidor Feb 19, 2018

mikz Feb 19, 2018

davidor Feb 19, 2018

davidor Feb 19, 2018 •

edited

Loading

davidor Feb 19, 2018

davidor Feb 20, 2018

mikz Feb 20, 2018

mikz left a comment

[THREESCALE-328] Allow to set a client SSL cert for upstream #610

[THREESCALE-328] Allow to set a client SSL cert for upstream #610

Conversation

davidor commented Feb 19, 2018 • edited by mayorova Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidor Feb 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikz left a comment

Choose a reason for hiding this comment

davidor commented Feb 19, 2018 •

edited by mayorova

Loading

davidor Feb 19, 2018 •

edited

Loading