You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/06/21 14:43:26 UTC
[GitHub] [incubator-apisix] nic-chen opened a new pull request #1747: feat: Support client-to-server authentication with SSL certificates
nic-chen opened a new pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747
# Summary
Support client-to-server authentication with SSL certificates for admin api.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] moonming commented on a change in pull request #1747: feat: Support client-to-server authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r443343163
##########
File path: conf/config-for-two-side-ssl-auth.yaml
##########
@@ -0,0 +1,174 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+apisix:
+ node_listen: 9080 # APISIX listening port
+ enable_heartbeat: true
+ enable_admin: true
+ enable_admin_cors: true # Admin API support CORS response headers.
+ enable_debug: false
+ enable_dev_mode: false # Sets nginx worker_processes to 1 if set to true
+ enable_reuseport: true # Enable nginx SO_REUSEPORT switch if set to true.
+ enable_ipv6: true
+ config_center: etcd # etcd: use etcd to store the config value
+ # yaml: fetch the config value from local yaml file `/your_path/conf/apisix.yaml`
+
+ #proxy_protocol: # Proxy Protocol configuration
+ # listen_http_port: 9181 # The port with proxy protocol for http, it differs from node_listen and port_admin.
+ # This port can only receive http request with proxy protocol, but node_listen & port_admin
+ # can only receive http request. If you enable proxy protocol, you must use this port to
+ # receive http request with proxy protocol
+ # listen_https_port: 9182 # The port with proxy protocol for https
+ # enable_tcp_pp: true # Enable the proxy protocol for tcp proxy, it works for stream_proxy.tcp option
+ # enable_tcp_pp_to_upstream: true # Enables the proxy protocol to the upstream server
+
+ proxy_cache: # Proxy Caching configuration
+ cache_ttl: 10s # The default caching time if the upstream does not specify the cache time
+ zones: # The parameters of a cache
+ - name: disk_cache_one # The name of the cache, administrator can be specify
+ # which cache to use by name in the admin api
+ memory_size: 50m # The size of shared memory, it's used to store the cache index
+ disk_size: 1G # The size of disk, it's used to store the cache data
+ disk_path: "/tmp/disk_cache_one" # The path to store the cache data
+ cache_levels: "1:2" # The hierarchy levels of a cache
+ # - name: disk_cache_two
+ # memory_size: 50m
+ # disk_size: 1G
+ # disk_path: "/tmp/disk_cache_two"
+ # cache_levels: "1:2"
+
+ allow_admin: # http://nginx.org/en/docs/http/ngx_http_access_module.html#allow
+ - 127.0.0.0/24 # If we don't set any IP list, then any IP access is allowed by default.
+ # - "::/64"
+ port_admin: 9180 # use a separate port
+ https_admin: true # enable HTTPS when use a separate port for Admin API.
+ # Admin API will use conf/apisix_admin_api.crt and conf/apisix_admin_api.key as certificate.
+
+ # Default token when use API to call for Admin API.
+ # *NOTE*: Highly recommended to modify this value to protect APISIX's Admin API.
+ # Disabling this configuration item means that the Admin API does not
+ # require any authentication.
+ admin_key:
+ -
+ name: "admin"
+ key: edd1c9f034335f136f87ad84b625c8f1
+ role: admin # admin: manage all configuration data
+ # viewer: only can view configuration data
+ -
+ name: "viewer"
+ key: 4054f7cf07e344346cd3f287985e76a2
+ role: viewer
+ router:
+ http: 'radixtree_uri' # radixtree_uri: match route by uri(base on radixtree)
+ # radixtree_host_uri: match route by host + uri(base on radixtree)
+ ssl: 'radixtree_sni' # radixtree_sni: match route by SNI(base on radixtree)
+ # stream_proxy: # TCP/UDP proxy
+ # tcp: # TCP proxy port list
+ # - 9100
+ # - 9101
+ # udp: # UDP proxy port list
+ # - 9200
+ # - 9211
+ # dns_resolver: # If not set, read from `/etc/resolv.conf`
+ # - 1.1.1.1
+ # - 8.8.8.8
+ dns_resolver_valid: 30 # valid time for dns result 30 seconds
+ resolver_timeout: 5 # resolver timeout
+ ssl:
+ enable: true
+ enable_http2: true
+ listen_port: 9443
+ ssl_protocols: "TLSv1 TLSv1.1 TLSv1.2 TLSv1.3"
+ ssl_ciphers: "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-SHA256:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA"
+ verify_client: true
Review comment:
the defalut value should be `false`
##########
File path: doc/two-side-auth-with-ssl.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+[Chinese](zh-cn/two-side-auth-with-ssl.md)
Review comment:
I think `Mutual authentication` or `mTLS` is better name.
##########
File path: conf/config.yaml
##########
@@ -93,6 +93,9 @@ apisix:
listen_port: 9443
ssl_protocols: "TLSv1 TLSv1.1 TLSv1.2 TLSv1.3"
ssl_ciphers: "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-SHA256:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA"
+ verify_client: false # Enable or disable client-to-server authentication with HTTPS client certificates
Review comment:
do you means authentication for the API admin caller?
##########
File path: conf/cert/two_side_server.key
##########
@@ -0,0 +1,27 @@
+-----BEGIN RSA PRIVATE KEY-----
Review comment:
I don't think it's a good idea to have a few built in self-signed certs, they should only be used for testing.
##########
File path: .travis.yml
##########
@@ -7,6 +7,8 @@ matrix:
services:
- docker
env: OSNAME=linux_openresty
+ - os: linux
+ env: OSNAME=linux_openresty_two_side_ssl_auth
Review comment:
I think `two_side` should be `mTLS`
##########
File path: conf/config.yaml
##########
@@ -93,6 +93,9 @@ apisix:
listen_port: 9443
ssl_protocols: "TLSv1 TLSv1.1 TLSv1.2 TLSv1.3"
ssl_ciphers: "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-SHA256:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA"
+ verify_client: false # Enable or disable client-to-server authentication with HTTPS client certificates
Review comment:
Should we let the user specify the path of mTLS certs?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] nic-chen commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444686094
##########
File path: .travis/linux_openresty_mtls_runner.sh
##########
@@ -109,13 +109,34 @@ script() {
sleep 1
cat logs/error.log
-
+ # correct certs
code=$(curl -i -o /dev/null -s -w %{http_code} --cacert ./t/certs/mtls_ca.crt --key ./t/certs/mtls_client.key --cert ./t/certs/mtls_client.crt -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' https://admin.apisix.dev:9180/apisix/admin/routes)
if [ ! $code -eq 200 ]; then
echo "failed: failed to enabled mTLS for admin"
exit 1
fi
+ # no certs
+ code=$(curl -i -o /dev/null -s -w %{http_code} -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' https://admin.apisix.dev:9180/apisix/admin/routes)
+ if [ ! $code -eq 000 ]; then
Review comment:
I think it's a default value when `curl` got an empty respond.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] nic-chen commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444627087
##########
File path: conf/config.yaml
##########
@@ -56,6 +56,11 @@ apisix:
# port_admin: 9180 # use a separate port
# https_admin: true # enable HTTPS when use a separate port for Admin API.
# Admin API will use conf/apisix_admin_api.crt and conf/apisix_admin_api.key as certificate.
+ mtls:
+ mtls_enable: false # Enable or disable mTLS. Enable depends on `port_admin` and `https_admin`.
Review comment:
We could set `mtls_enable ` false to disable this feature, and retain the example config.
If remove this switch, we have to remove the example config.
What do you think ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] membphis commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444664848
##########
File path: .travis/linux_openresty_mtls_runner.sh
##########
@@ -109,13 +109,34 @@ script() {
sleep 1
cat logs/error.log
-
+ # correct certs
code=$(curl -i -o /dev/null -s -w %{http_code} --cacert ./t/certs/mtls_ca.crt --key ./t/certs/mtls_client.key --cert ./t/certs/mtls_client.crt -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' https://admin.apisix.dev:9180/apisix/admin/routes)
if [ ! $code -eq 200 ]; then
echo "failed: failed to enabled mTLS for admin"
exit 1
fi
+ # no certs
+ code=$(curl -i -o /dev/null -s -w %{http_code} -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' https://admin.apisix.dev:9180/apisix/admin/routes)
+ if [ ! $code -eq 000 ]; then
Review comment:
Does `000` equal to `0`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] membphis commented on pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#issuecomment-659920720
@nic-chen please rebase your branch with master
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] membphis commented on a change in pull request #1747: feat: Support client-to-server authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r443488615
##########
File path: conf/config.yaml
##########
@@ -56,6 +56,11 @@ apisix:
# port_admin: 9180 # use a separate port
# https_admin: true # enable HTTPS when use a separate port for Admin API.
# Admin API will use conf/apisix_admin_api.crt and conf/apisix_admin_api.key as certificate.
+ mtls:
+ enable: false # Enable or disable mtls. Enable depends on `port_admin` and `https_admin`.
+ ca_cert: "" # Path of your self-signed ca cert.
Review comment:
we should set a default address, eg: `t/cert/****.crt`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] membphis commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r456294589
##########
File path: .travis.yml
##########
@@ -9,6 +9,8 @@ matrix:
services:
- docker
env: OSNAME=linux_openresty
+ - os: linux
Review comment:
I think we can merge this PR first, and then submit the PR separately to fix it. What do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] membphis commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444664848
##########
File path: .travis/linux_openresty_mtls_runner.sh
##########
@@ -109,13 +109,34 @@ script() {
sleep 1
cat logs/error.log
-
+ # correct certs
code=$(curl -i -o /dev/null -s -w %{http_code} --cacert ./t/certs/mtls_ca.crt --key ./t/certs/mtls_client.key --cert ./t/certs/mtls_client.crt -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' https://admin.apisix.dev:9180/apisix/admin/routes)
if [ ! $code -eq 200 ]; then
echo "failed: failed to enabled mTLS for admin"
exit 1
fi
+ # no certs
+ code=$(curl -i -o /dev/null -s -w %{http_code} -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' https://admin.apisix.dev:9180/apisix/admin/routes)
+ if [ ! $code -eq 000 ]; then
Review comment:
Does `000` equal to `0`? Confused about `000`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] nic-chen commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444728892
##########
File path: .travis/linux_openresty_mtls_runner.sh
##########
@@ -0,0 +1,146 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+set -ex
+
+export_or_prefix() {
+ export OPENRESTY_PREFIX="/usr/local/openresty-debug"
+}
+
+create_lua_deps() {
+ echo "Create lua deps cache"
+
+ make deps
+ luarocks install luacov-coveralls --tree=deps --local > build.log 2>&1 || (cat build.log && exit 1)
+
+ sudo rm -rf build-cache/deps
+ sudo cp -r deps build-cache/
+ sudo cp rockspec/apisix-master-0.rockspec build-cache/
+}
+
+before_install() {
+ sudo cpanm --notest Test::Nginx >build.log 2>&1 || (cat build.log && exit 1)
+}
+
+do_install() {
+ export_or_prefix
+
+ wget -qO - https://openresty.org/package/pubkey.gpg | sudo apt-key add -
+ sudo apt-get -y update --fix-missing
+ sudo apt-get -y install software-properties-common
+ sudo add-apt-repository -y "deb http://openresty.org/package/ubuntu $(lsb_release -sc) main"
+ sudo add-apt-repository -y ppa:longsleep/golang-backports
+
+ sudo apt-get update
+ sudo apt-get install openresty-debug lua5.1 liblua5.1-0-dev
+
+ wget https://github.com/luarocks/luarocks/archive/v2.4.4.tar.gz
+ tar -xf v2.4.4.tar.gz
+ cd luarocks-2.4.4
+ ./configure --prefix=/usr > build.log 2>&1 || (cat build.log && exit 1)
+ make build > build.log 2>&1 || (cat build.log && exit 1)
+ sudo make install > build.log 2>&1 || (cat build.log && exit 1)
+ cd ..
+ rm -rf luarocks-2.4.4
+
+ sudo luarocks install luacheck > build.log 2>&1 || (cat build.log && exit 1)
+
+
+ if [ ! -f "build-cache/apisix-master-0.rockspec" ]; then
+ create_lua_deps
+
+ else
+ src=`md5sum rockspec/apisix-master-0.rockspec | awk '{print $1}'`
+ src_cp=`md5sum build-cache/apisix-master-0.rockspec | awk '{print $1}'`
+ if [ "$src" = "$src_cp" ]; then
+ echo "Use lua deps cache"
+ sudo cp -r build-cache/deps ./
+ else
+ create_lua_deps
+ fi
+ fi
+
+ # sudo apt-get install tree -y
+ # tree deps
+
+ git clone https://github.com/iresty/test-nginx.git test-nginx
+ make utils
+
+ git clone https://github.com/apache/openwhisk-utilities.git .travis/openwhisk-utilities
+ cp .travis/ASF* .travis/openwhisk-utilities/scancode/
+
+ ls -l ./
+}
+
+script() {
+ export_or_prefix
+ export PATH=$OPENRESTY_PREFIX/nginx/sbin:$OPENRESTY_PREFIX/luajit/bin:$OPENRESTY_PREFIX/bin:$PATH
+ openresty -V
+ sudo service etcd start
+
+ # enable mTLS
+ sed -i 's/\# port_admin: 9180/port_admin: 9180/' conf/config.yaml
+ sed -i 's/\# https_admin: true/https_admin: true/' conf/config.yaml
+ sed -i 's/mtls_enable: false/mtls_enable: true/' conf/config.yaml
+ sed -i 's#admin_api_ca_cert: ""#admin_api_ca_cert: "../t/certs/mtls_ca.crt"#' conf/config.yaml
+ sed -i 's#admin_ssl_cert_key: ""#admin_ssl_cert_key: "../t/certs/mtls_server.key"#' conf/config.yaml
+ sed -i 's#admin_ssl_cert: ""#admin_ssl_cert: "../t/certs/mtls_server.crt"#' conf/config.yaml
+
+ ./bin/apisix help
+ ./bin/apisix init
+ ./bin/apisix init_etcd
+ ./bin/apisix start
+
+ sleep 1
+ cat logs/error.log
+
+
+ code=$(curl -i -o /dev/null -s -w %{http_code} --cacert ./t/certs/mtls_ca.crt --key ./t/certs/mtls_client.key --cert ./t/certs/mtls_client.crt -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' https://admin.apisix.dev:9180/apisix/admin/routes)
Review comment:
I added some test cases, but they cause exiting that make travis failed.
So I keep only one, which skip the cert auth and get 400 respond because `No required SSL certificate was sent`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] membphis commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444601468
##########
File path: .travis/linux_openresty_mtls_runner.sh
##########
@@ -0,0 +1,146 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+set -ex
+
+export_or_prefix() {
+ export OPENRESTY_PREFIX="/usr/local/openresty-debug"
+}
+
+create_lua_deps() {
+ echo "Create lua deps cache"
+
+ make deps
+ luarocks install luacov-coveralls --tree=deps --local > build.log 2>&1 || (cat build.log && exit 1)
+
+ sudo rm -rf build-cache/deps
+ sudo cp -r deps build-cache/
+ sudo cp rockspec/apisix-master-0.rockspec build-cache/
+}
+
+before_install() {
+ sudo cpanm --notest Test::Nginx >build.log 2>&1 || (cat build.log && exit 1)
+}
+
+do_install() {
+ export_or_prefix
+
+ wget -qO - https://openresty.org/package/pubkey.gpg | sudo apt-key add -
+ sudo apt-get -y update --fix-missing
+ sudo apt-get -y install software-properties-common
+ sudo add-apt-repository -y "deb http://openresty.org/package/ubuntu $(lsb_release -sc) main"
+ sudo add-apt-repository -y ppa:longsleep/golang-backports
+
+ sudo apt-get update
+ sudo apt-get install openresty-debug lua5.1 liblua5.1-0-dev
+
+ wget https://github.com/luarocks/luarocks/archive/v2.4.4.tar.gz
+ tar -xf v2.4.4.tar.gz
+ cd luarocks-2.4.4
+ ./configure --prefix=/usr > build.log 2>&1 || (cat build.log && exit 1)
+ make build > build.log 2>&1 || (cat build.log && exit 1)
+ sudo make install > build.log 2>&1 || (cat build.log && exit 1)
+ cd ..
+ rm -rf luarocks-2.4.4
+
+ sudo luarocks install luacheck > build.log 2>&1 || (cat build.log && exit 1)
+
+
+ if [ ! -f "build-cache/apisix-master-0.rockspec" ]; then
+ create_lua_deps
+
+ else
+ src=`md5sum rockspec/apisix-master-0.rockspec | awk '{print $1}'`
+ src_cp=`md5sum build-cache/apisix-master-0.rockspec | awk '{print $1}'`
+ if [ "$src" = "$src_cp" ]; then
+ echo "Use lua deps cache"
+ sudo cp -r build-cache/deps ./
+ else
+ create_lua_deps
+ fi
+ fi
+
+ # sudo apt-get install tree -y
+ # tree deps
+
+ git clone https://github.com/iresty/test-nginx.git test-nginx
+ make utils
+
+ git clone https://github.com/apache/openwhisk-utilities.git .travis/openwhisk-utilities
+ cp .travis/ASF* .travis/openwhisk-utilities/scancode/
+
+ ls -l ./
+}
+
+script() {
+ export_or_prefix
+ export PATH=$OPENRESTY_PREFIX/nginx/sbin:$OPENRESTY_PREFIX/luajit/bin:$OPENRESTY_PREFIX/bin:$PATH
+ openresty -V
+ sudo service etcd start
+
+ # enable mTLS
+ sed -i 's/\# port_admin: 9180/port_admin: 9180/' conf/config.yaml
+ sed -i 's/\# https_admin: true/https_admin: true/' conf/config.yaml
+ sed -i 's/mtls_enable: false/mtls_enable: true/' conf/config.yaml
+ sed -i 's#ca_cert: ""#ca_cert: "../t/certs/mtls_ca.crt"#' conf/config.yaml
+ sed -i 's#server_key: ""#server_key: "../t/certs/mtls_server.key"#' conf/config.yaml
+ sed -i 's#server_cert: ""#server_cert: "../t/certs/mtls_server.crt"#' conf/config.yaml
+
+ ./bin/apisix help
+ ./bin/apisix init
+ ./bin/apisix init_etcd
+ ./bin/apisix start
+
+ sleep 1
+ cat logs/error.log
+
+
+ code=$(curl -i -o /dev/null -s -w %{http_code} --cacert ./t/certs/mtls_ca.crt --key ./t/certs/mtls_client.key --cert ./t/certs/mtls_client.crt -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' https://admin.apisix.dev:9180/apisix/admin/routes)
Review comment:
It is not good to use `test::nginx` to test this case.
This is mainly to simulate the process of user configuration and how to use it.
The `nginx.conf` generated by `test::nginx` is completely different from the `nginx.conf` generated by the user via `conf/config.yaml`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] nic-chen commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r456269014
##########
File path: .travis.yml
##########
@@ -9,6 +9,8 @@ matrix:
services:
- docker
env: OSNAME=linux_openresty
+ - os: linux
Review comment:
It's good. I will try to move to github actions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] nic-chen commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444726002
##########
File path: bin/apisix
##########
@@ -290,13 +290,23 @@ http {
server {
{%if https_admin then%}
listen {* port_admin *} ssl;
+
+ {%if admin_api_mtls and admin_api_mtls.mtls_enable then%}
+ ssl_verify_client on;
+ ssl_certificate {* admin_api_mtls.admin_ssl_cert *};
+ ssl_certificate_key {* admin_api_mtls.admin_ssl_cert_key *};
+ ssl_client_certificate {* admin_api_mtls.admin_ssl_ca_cert *};
Review comment:
We had that config before.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] moonming commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444633710
##########
File path: .travis/linux_openresty_mtls_runner.sh
##########
@@ -0,0 +1,146 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+set -ex
+
+export_or_prefix() {
+ export OPENRESTY_PREFIX="/usr/local/openresty-debug"
+}
+
+create_lua_deps() {
+ echo "Create lua deps cache"
+
+ make deps
+ luarocks install luacov-coveralls --tree=deps --local > build.log 2>&1 || (cat build.log && exit 1)
+
+ sudo rm -rf build-cache/deps
+ sudo cp -r deps build-cache/
+ sudo cp rockspec/apisix-master-0.rockspec build-cache/
+}
+
+before_install() {
+ sudo cpanm --notest Test::Nginx >build.log 2>&1 || (cat build.log && exit 1)
+}
+
+do_install() {
+ export_or_prefix
+
+ wget -qO - https://openresty.org/package/pubkey.gpg | sudo apt-key add -
+ sudo apt-get -y update --fix-missing
+ sudo apt-get -y install software-properties-common
+ sudo add-apt-repository -y "deb http://openresty.org/package/ubuntu $(lsb_release -sc) main"
+ sudo add-apt-repository -y ppa:longsleep/golang-backports
+
+ sudo apt-get update
+ sudo apt-get install openresty-debug lua5.1 liblua5.1-0-dev
+
+ wget https://github.com/luarocks/luarocks/archive/v2.4.4.tar.gz
+ tar -xf v2.4.4.tar.gz
+ cd luarocks-2.4.4
+ ./configure --prefix=/usr > build.log 2>&1 || (cat build.log && exit 1)
+ make build > build.log 2>&1 || (cat build.log && exit 1)
+ sudo make install > build.log 2>&1 || (cat build.log && exit 1)
+ cd ..
+ rm -rf luarocks-2.4.4
+
+ sudo luarocks install luacheck > build.log 2>&1 || (cat build.log && exit 1)
+
+
+ if [ ! -f "build-cache/apisix-master-0.rockspec" ]; then
+ create_lua_deps
+
+ else
+ src=`md5sum rockspec/apisix-master-0.rockspec | awk '{print $1}'`
+ src_cp=`md5sum build-cache/apisix-master-0.rockspec | awk '{print $1}'`
+ if [ "$src" = "$src_cp" ]; then
+ echo "Use lua deps cache"
+ sudo cp -r build-cache/deps ./
+ else
+ create_lua_deps
+ fi
+ fi
+
+ # sudo apt-get install tree -y
+ # tree deps
+
+ git clone https://github.com/iresty/test-nginx.git test-nginx
+ make utils
+
+ git clone https://github.com/apache/openwhisk-utilities.git .travis/openwhisk-utilities
+ cp .travis/ASF* .travis/openwhisk-utilities/scancode/
+
+ ls -l ./
+}
+
+script() {
+ export_or_prefix
+ export PATH=$OPENRESTY_PREFIX/nginx/sbin:$OPENRESTY_PREFIX/luajit/bin:$OPENRESTY_PREFIX/bin:$PATH
+ openresty -V
+ sudo service etcd start
+
+ # enable mTLS
+ sed -i 's/\# port_admin: 9180/port_admin: 9180/' conf/config.yaml
+ sed -i 's/\# https_admin: true/https_admin: true/' conf/config.yaml
+ sed -i 's/mtls_enable: false/mtls_enable: true/' conf/config.yaml
+ sed -i 's#admin_api_ca_cert: ""#admin_api_ca_cert: "../t/certs/mtls_ca.crt"#' conf/config.yaml
+ sed -i 's#admin_ssl_cert_key: ""#admin_ssl_cert_key: "../t/certs/mtls_server.key"#' conf/config.yaml
+ sed -i 's#admin_ssl_cert: ""#admin_ssl_cert: "../t/certs/mtls_server.crt"#' conf/config.yaml
+
+ ./bin/apisix help
+ ./bin/apisix init
+ ./bin/apisix init_etcd
+ ./bin/apisix start
+
+ sleep 1
+ cat logs/error.log
+
+
+ code=$(curl -i -o /dev/null -s -w %{http_code} --cacert ./t/certs/mtls_ca.crt --key ./t/certs/mtls_client.key --cert ./t/certs/mtls_client.crt -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' https://admin.apisix.dev:9180/apisix/admin/routes)
Review comment:
need add test cases:
- no ssl cert
- wrong ssl cert
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] nic-chen commented on a change in pull request #1747: feat: Support client-to-server authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r443594451
##########
File path: .travis/linux_openresty_two_side_ssl_auth_runner.sh
##########
@@ -0,0 +1,136 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+set -ex
+
+export_or_prefix() {
+ export OPENRESTY_PREFIX="/usr/local/openresty-debug"
+}
+
+create_lua_deps() {
+ echo "Create lua deps cache"
+
+ make deps
+ luarocks install luacov-coveralls --tree=deps --local > build.log 2>&1 || (cat build.log && exit 1)
+
+ sudo rm -rf build-cache/deps
+ sudo cp -r deps build-cache/
+ sudo cp rockspec/apisix-master-0.rockspec build-cache/
+}
+
+before_install() {
+ sudo cpanm --notest Test::Nginx >build.log 2>&1 || (cat build.log && exit 1)
+}
+
+do_install() {
+ export_or_prefix
+
+ wget -qO - https://openresty.org/package/pubkey.gpg | sudo apt-key add -
+ sudo apt-get -y update --fix-missing
+ sudo apt-get -y install software-properties-common
+ sudo add-apt-repository -y "deb http://openresty.org/package/ubuntu $(lsb_release -sc) main"
+ sudo add-apt-repository -y ppa:longsleep/golang-backports
+
+ sudo apt-get update
+ sudo apt-get install openresty-debug lua5.1 liblua5.1-0-dev
+
+ wget https://github.com/luarocks/luarocks/archive/v2.4.4.tar.gz
+ tar -xf v2.4.4.tar.gz
+ cd luarocks-2.4.4
+ ./configure --prefix=/usr > build.log 2>&1 || (cat build.log && exit 1)
+ make build > build.log 2>&1 || (cat build.log && exit 1)
+ sudo make install > build.log 2>&1 || (cat build.log && exit 1)
+ cd ..
+ rm -rf luarocks-2.4.4
+
+ sudo luarocks install luacheck > build.log 2>&1 || (cat build.log && exit 1)
+
+
+ if [ ! -f "build-cache/apisix-master-0.rockspec" ]; then
+ create_lua_deps
+
+ else
+ src=`md5sum rockspec/apisix-master-0.rockspec | awk '{print $1}'`
+ src_cp=`md5sum build-cache/apisix-master-0.rockspec | awk '{print $1}'`
+ if [ "$src" = "$src_cp" ]; then
+ echo "Use lua deps cache"
+ sudo cp -r build-cache/deps ./
+ else
+ create_lua_deps
+ fi
+ fi
+
+ # sudo apt-get install tree -y
+ # tree deps
+
+ git clone https://github.com/iresty/test-nginx.git test-nginx
+ make utils
+
+ git clone https://github.com/apache/openwhisk-utilities.git .travis/openwhisk-utilities
+ cp .travis/ASF* .travis/openwhisk-utilities/scancode/
+
+ ls -l ./
+}
+
+script() {
+ export_or_prefix
+ export PATH=$OPENRESTY_PREFIX/nginx/sbin:$OPENRESTY_PREFIX/luajit/bin:$OPENRESTY_PREFIX/bin:$PATH
+ openresty -V
+ sudo service etcd start
+
+ mv -f ./conf/config-for-two-side-ssl-auth.yaml ./conf/config.yaml
Review comment:
I modify the config in travis. I think it's better.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] moonming commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444627139
##########
File path: .travis/linux_openresty_mtls_runner.sh
##########
@@ -0,0 +1,146 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+set -ex
+
+export_or_prefix() {
+ export OPENRESTY_PREFIX="/usr/local/openresty-debug"
+}
+
+create_lua_deps() {
+ echo "Create lua deps cache"
+
+ make deps
+ luarocks install luacov-coveralls --tree=deps --local > build.log 2>&1 || (cat build.log && exit 1)
+
+ sudo rm -rf build-cache/deps
+ sudo cp -r deps build-cache/
+ sudo cp rockspec/apisix-master-0.rockspec build-cache/
+}
+
+before_install() {
+ sudo cpanm --notest Test::Nginx >build.log 2>&1 || (cat build.log && exit 1)
+}
+
+do_install() {
+ export_or_prefix
+
+ wget -qO - https://openresty.org/package/pubkey.gpg | sudo apt-key add -
+ sudo apt-get -y update --fix-missing
+ sudo apt-get -y install software-properties-common
+ sudo add-apt-repository -y "deb http://openresty.org/package/ubuntu $(lsb_release -sc) main"
+ sudo add-apt-repository -y ppa:longsleep/golang-backports
+
+ sudo apt-get update
+ sudo apt-get install openresty-debug lua5.1 liblua5.1-0-dev
+
+ wget https://github.com/luarocks/luarocks/archive/v2.4.4.tar.gz
+ tar -xf v2.4.4.tar.gz
+ cd luarocks-2.4.4
+ ./configure --prefix=/usr > build.log 2>&1 || (cat build.log && exit 1)
+ make build > build.log 2>&1 || (cat build.log && exit 1)
+ sudo make install > build.log 2>&1 || (cat build.log && exit 1)
+ cd ..
+ rm -rf luarocks-2.4.4
+
+ sudo luarocks install luacheck > build.log 2>&1 || (cat build.log && exit 1)
+
+
+ if [ ! -f "build-cache/apisix-master-0.rockspec" ]; then
+ create_lua_deps
+
+ else
+ src=`md5sum rockspec/apisix-master-0.rockspec | awk '{print $1}'`
+ src_cp=`md5sum build-cache/apisix-master-0.rockspec | awk '{print $1}'`
+ if [ "$src" = "$src_cp" ]; then
+ echo "Use lua deps cache"
+ sudo cp -r build-cache/deps ./
+ else
+ create_lua_deps
+ fi
+ fi
+
+ # sudo apt-get install tree -y
+ # tree deps
+
+ git clone https://github.com/iresty/test-nginx.git test-nginx
+ make utils
+
+ git clone https://github.com/apache/openwhisk-utilities.git .travis/openwhisk-utilities
+ cp .travis/ASF* .travis/openwhisk-utilities/scancode/
+
+ ls -l ./
+}
+
+script() {
+ export_or_prefix
+ export PATH=$OPENRESTY_PREFIX/nginx/sbin:$OPENRESTY_PREFIX/luajit/bin:$OPENRESTY_PREFIX/bin:$PATH
+ openresty -V
+ sudo service etcd start
+
+ # enable mTLS
+ sed -i 's/\# port_admin: 9180/port_admin: 9180/' conf/config.yaml
+ sed -i 's/\# https_admin: true/https_admin: true/' conf/config.yaml
+ sed -i 's/mtls_enable: false/mtls_enable: true/' conf/config.yaml
+ sed -i 's#ca_cert: ""#ca_cert: "../t/certs/mtls_ca.crt"#' conf/config.yaml
+ sed -i 's#server_key: ""#server_key: "../t/certs/mtls_server.key"#' conf/config.yaml
+ sed -i 's#server_cert: ""#server_cert: "../t/certs/mtls_server.crt"#' conf/config.yaml
+
+ ./bin/apisix help
+ ./bin/apisix init
+ ./bin/apisix init_etcd
+ ./bin/apisix start
+
+ sleep 1
+ cat logs/error.log
+
+
+ code=$(curl -i -o /dev/null -s -w %{http_code} --cacert ./t/certs/mtls_ca.crt --key ./t/certs/mtls_client.key --cert ./t/certs/mtls_client.crt -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' https://admin.apisix.dev:9180/apisix/admin/routes)
Review comment:
The current test is not enough
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] nic-chen commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444626150
##########
File path: bin/apisix
##########
@@ -290,13 +290,23 @@ http {
server {
{%if https_admin then%}
listen {* port_admin *} ssl;
+
+ {%if mtls and mtls.mtls_enable then%}
+ ssl_verify_client on;
+ ssl_certificate {* mtls.server_cert *};
+ ssl_certificate_key {* mtls.server_key *};
+ ssl_client_certificate {* mtls.ca_cert *};
Review comment:
yes, `ssl_verify_client` depends on `ssl_client_certificate` to verify client requests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] nic-chen commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444729607
##########
File path: .travis/linux_openresty_mtls_runner.sh
##########
@@ -109,13 +109,34 @@ script() {
sleep 1
cat logs/error.log
-
+ # correct certs
code=$(curl -i -o /dev/null -s -w %{http_code} --cacert ./t/certs/mtls_ca.crt --key ./t/certs/mtls_client.key --cert ./t/certs/mtls_client.crt -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' https://admin.apisix.dev:9180/apisix/admin/routes)
if [ ! $code -eq 200 ]; then
echo "failed: failed to enabled mTLS for admin"
exit 1
fi
+ # no certs
+ code=$(curl -i -o /dev/null -s -w %{http_code} -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' https://admin.apisix.dev:9180/apisix/admin/routes)
+ if [ ! $code -eq 000 ]; then
Review comment:
It caused exiting that make travis failed. So I removed them ..
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] membphis commented on a change in pull request #1747: feat: Support client-to-server authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r443975996
##########
File path: doc/mtls.md
##########
@@ -0,0 +1,42 @@
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+[Chinese](zh-cn/mtls.md)
+
+## Enable mutual TLS authentication
+
+1. Generate self-signed key pairs, including ca, server, client key pairs.
+
+2. Modify configuration items in `conf/config.yaml`:
+```yaml
+ port_admin: 9180
+ https_admin: true
+
+ mtls:
+ enable: true # Enable or disable mTLS. Enable depends on `port_admin` and `https_admin`.
+ ca_cert: "/data/certs/mtls_ca.crt" # Path of your self-signed ca cert.
+ server_key: "/data/certs/mtls_server.key" # Path of your self-signed server side cert.
+ server_cert: "/data/certs/mtls_server.crt" # Path of your self-signed server side key.
+```
+
+3. Run command:
+```shell
Review comment:
better to add a blank line
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] moonming commented on a change in pull request #1747: feat: Support client-to-server authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444597056
##########
File path: conf/config.yaml
##########
@@ -56,6 +56,11 @@ apisix:
# port_admin: 9180 # use a separate port
# https_admin: true # enable HTTPS when use a separate port for Admin API.
# Admin API will use conf/apisix_admin_api.crt and conf/apisix_admin_api.key as certificate.
+ mtls:
+ mtls_enable: false # Enable or disable mTLS. Enable depends on `port_admin` and `https_admin`.
+ ca_cert: "../t/certs/mtls_ca.crt" # Path of your self-signed ca cert.
+ server_key: "../t/certs/mtls_server.key" # Path of your self-signed server side cert.
Review comment:
`admin_ssl_cert` and `admin_ssl_cert_key` will better.
##########
File path: bin/apisix
##########
@@ -290,13 +290,23 @@ http {
server {
{%if https_admin then%}
listen {* port_admin *} ssl;
+
+ {%if mtls and mtls.mtls_enable then%}
+ ssl_verify_client on;
+ ssl_certificate {* mtls.server_cert *};
+ ssl_certificate_key {* mtls.server_key *};
+ ssl_client_certificate {* mtls.ca_cert *};
Review comment:
`ssl_client_certificate` is used to verify client certificates, do we need it?
##########
File path: conf/config.yaml
##########
@@ -56,6 +56,11 @@ apisix:
# port_admin: 9180 # use a separate port
# https_admin: true # enable HTTPS when use a separate port for Admin API.
# Admin API will use conf/apisix_admin_api.crt and conf/apisix_admin_api.key as certificate.
+ mtls:
Review comment:
This is not a good name.
mTLS can be between apisix and upstream, or between dashboard and admin api, which have different meanings.
##########
File path: conf/config.yaml
##########
@@ -56,6 +56,11 @@ apisix:
# port_admin: 9180 # use a separate port
# https_admin: true # enable HTTPS when use a separate port for Admin API.
# Admin API will use conf/apisix_admin_api.crt and conf/apisix_admin_api.key as certificate.
+ mtls:
+ mtls_enable: false # Enable or disable mTLS. Enable depends on `port_admin` and `https_admin`.
Review comment:
I don't think we need this switch, if user set cert and key, then enable it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] moonming commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444599535
##########
File path: conf/config.yaml
##########
@@ -56,6 +56,11 @@ apisix:
# port_admin: 9180 # use a separate port
# https_admin: true # enable HTTPS when use a separate port for Admin API.
# Admin API will use conf/apisix_admin_api.crt and conf/apisix_admin_api.key as certificate.
+ mtls:
Review comment:
And we need to add support mTLS between apisix and upstream later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] nic-chen commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444726150
##########
File path: doc/mtls.md
##########
@@ -0,0 +1,44 @@
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+[Chinese](zh-cn/mtls.md)
+
+## Enable mutual TLS authentication
+
Review comment:
updated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] moonming merged pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
moonming merged pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] moonming commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444631539
##########
File path: conf/config.yaml
##########
@@ -56,6 +56,11 @@ apisix:
# port_admin: 9180 # use a separate port
# https_admin: true # enable HTTPS when use a separate port for Admin API.
# Admin API will use conf/apisix_admin_api.crt and conf/apisix_admin_api.key as certificate.
+ mtls:
+ mtls_enable: false # Enable or disable mTLS. Enable depends on `port_admin` and `https_admin`.
Review comment:
we can set empty string for disable them as default.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] membphis commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r456294589
##########
File path: .travis.yml
##########
@@ -9,6 +9,8 @@ matrix:
services:
- docker
env: OSNAME=linux_openresty
+ - os: linux
Review comment:
I think we can merge this PR first, and then submit the PR separately to fix it. What do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] moonming commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444599535
##########
File path: conf/config.yaml
##########
@@ -56,6 +56,11 @@ apisix:
# port_admin: 9180 # use a separate port
# https_admin: true # enable HTTPS when use a separate port for Admin API.
# Admin API will use conf/apisix_admin_api.crt and conf/apisix_admin_api.key as certificate.
+ mtls:
Review comment:
And we need to add support mTLS between apisix and upstream later, in another PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] membphis commented on pull request #1747: feat: Support client-to-server authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#issuecomment-648098326
@nic-chen the Travis-CI seems been blocked. you can rebase your branch.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] membphis commented on a change in pull request #1747: feat: Support client-to-server authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r443344183
##########
File path: doc/two-side-auth-with-ssl.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+[Chinese](zh-cn/two-side-auth-with-ssl.md)
Review comment:
`mTLS` is a good name
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] membphis commented on pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#issuecomment-659955399
I think we can merge this PR first, and then submit the PR separately to fix it: https://github.com/apache/incubator-apisix/pull/1747#discussion_r456269014
What do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] nic-chen commented on a change in pull request #1747: feat: Support client-to-server authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r443594780
##########
File path: conf/config.yaml
##########
@@ -93,6 +93,9 @@ apisix:
listen_port: 9443
ssl_protocols: "TLSv1 TLSv1.1 TLSv1.2 TLSv1.3"
ssl_ciphers: "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-SHA256:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA"
+ verify_client: false # Enable or disable client-to-server authentication with HTTPS client certificates
Review comment:
fixed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] nic-chen commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444628324
##########
File path: .travis/linux_openresty_mtls_runner.sh
##########
@@ -0,0 +1,146 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+set -ex
+
+export_or_prefix() {
+ export OPENRESTY_PREFIX="/usr/local/openresty-debug"
+}
+
+create_lua_deps() {
+ echo "Create lua deps cache"
+
+ make deps
+ luarocks install luacov-coveralls --tree=deps --local > build.log 2>&1 || (cat build.log && exit 1)
+
+ sudo rm -rf build-cache/deps
+ sudo cp -r deps build-cache/
+ sudo cp rockspec/apisix-master-0.rockspec build-cache/
+}
+
+before_install() {
+ sudo cpanm --notest Test::Nginx >build.log 2>&1 || (cat build.log && exit 1)
+}
+
+do_install() {
+ export_or_prefix
+
+ wget -qO - https://openresty.org/package/pubkey.gpg | sudo apt-key add -
+ sudo apt-get -y update --fix-missing
+ sudo apt-get -y install software-properties-common
+ sudo add-apt-repository -y "deb http://openresty.org/package/ubuntu $(lsb_release -sc) main"
+ sudo add-apt-repository -y ppa:longsleep/golang-backports
+
+ sudo apt-get update
+ sudo apt-get install openresty-debug lua5.1 liblua5.1-0-dev
+
+ wget https://github.com/luarocks/luarocks/archive/v2.4.4.tar.gz
+ tar -xf v2.4.4.tar.gz
+ cd luarocks-2.4.4
+ ./configure --prefix=/usr > build.log 2>&1 || (cat build.log && exit 1)
+ make build > build.log 2>&1 || (cat build.log && exit 1)
+ sudo make install > build.log 2>&1 || (cat build.log && exit 1)
+ cd ..
+ rm -rf luarocks-2.4.4
+
+ sudo luarocks install luacheck > build.log 2>&1 || (cat build.log && exit 1)
+
+
+ if [ ! -f "build-cache/apisix-master-0.rockspec" ]; then
+ create_lua_deps
+
+ else
+ src=`md5sum rockspec/apisix-master-0.rockspec | awk '{print $1}'`
+ src_cp=`md5sum build-cache/apisix-master-0.rockspec | awk '{print $1}'`
+ if [ "$src" = "$src_cp" ]; then
+ echo "Use lua deps cache"
+ sudo cp -r build-cache/deps ./
+ else
+ create_lua_deps
+ fi
+ fi
+
+ # sudo apt-get install tree -y
+ # tree deps
+
+ git clone https://github.com/iresty/test-nginx.git test-nginx
+ make utils
+
+ git clone https://github.com/apache/openwhisk-utilities.git .travis/openwhisk-utilities
+ cp .travis/ASF* .travis/openwhisk-utilities/scancode/
+
+ ls -l ./
+}
+
+script() {
+ export_or_prefix
+ export PATH=$OPENRESTY_PREFIX/nginx/sbin:$OPENRESTY_PREFIX/luajit/bin:$OPENRESTY_PREFIX/bin:$PATH
+ openresty -V
+ sudo service etcd start
+
+ # enable mTLS
+ sed -i 's/\# port_admin: 9180/port_admin: 9180/' conf/config.yaml
+ sed -i 's/\# https_admin: true/https_admin: true/' conf/config.yaml
+ sed -i 's/mtls_enable: false/mtls_enable: true/' conf/config.yaml
+ sed -i 's#ca_cert: ""#ca_cert: "../t/certs/mtls_ca.crt"#' conf/config.yaml
+ sed -i 's#server_key: ""#server_key: "../t/certs/mtls_server.key"#' conf/config.yaml
+ sed -i 's#server_cert: ""#server_cert: "../t/certs/mtls_server.crt"#' conf/config.yaml
+
+ ./bin/apisix help
+ ./bin/apisix init
+ ./bin/apisix init_etcd
+ ./bin/apisix start
+
+ sleep 1
+ cat logs/error.log
+
+
+ code=$(curl -i -o /dev/null -s -w %{http_code} --cacert ./t/certs/mtls_ca.crt --key ./t/certs/mtls_client.key --cert ./t/certs/mtls_client.crt -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' https://admin.apisix.dev:9180/apisix/admin/routes)
Review comment:
On the other hand, I think it doesn't support to sign with client certificates in `test::nginx`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] moonming commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444632469
##########
File path: conf/config.yaml
##########
@@ -53,9 +53,14 @@ apisix:
allow_admin: # http://nginx.org/en/docs/http/ngx_http_access_module.html#allow
- 127.0.0.0/24 # If we don't set any IP list, then any IP access is allowed by default.
# - "::/64"
- # port_admin: 9180 # use a separate port
- # https_admin: true # enable HTTPS when use a separate port for Admin API.
+ port_admin: 9180 # use a separate port
+ https_admin: true # enable HTTPS when use a separate port for Admin API.
# Admin API will use conf/apisix_admin_api.crt and conf/apisix_admin_api.key as certificate.
+ admin_api_mtls:
+ mtls_enable: true # Enable or disable mTLS. Enable depends on `port_admin` and `https_admin`.
+ admin_ssl_cert: "../t/certs/mtls_server.crt" # Path of your self-signed server side cert.
+ admin_ssl_cert_key: "../t/certs/mtls_server.key" # Path of your self-signed server side key.
+ admin_ssl_ca_cert: "../t/certs/mtls_ca.crt" # Path of your self-signed ca cert.
Review comment:
this CA is used to sign all admin api callers' certificates, we should explicitly state that
##########
File path: bin/apisix
##########
@@ -290,13 +290,23 @@ http {
server {
{%if https_admin then%}
listen {* port_admin *} ssl;
+
+ {%if admin_api_mtls and admin_api_mtls.mtls_enable then%}
+ ssl_verify_client on;
+ ssl_certificate {* admin_api_mtls.admin_ssl_cert *};
+ ssl_certificate_key {* admin_api_mtls.admin_ssl_cert_key *};
+ ssl_client_certificate {* admin_api_mtls.admin_ssl_ca_cert *};
Review comment:
do we need `ssl_session_cache `?
##########
File path: conf/config.yaml
##########
@@ -53,9 +53,14 @@ apisix:
allow_admin: # http://nginx.org/en/docs/http/ngx_http_access_module.html#allow
- 127.0.0.0/24 # If we don't set any IP list, then any IP access is allowed by default.
# - "::/64"
- # port_admin: 9180 # use a separate port
- # https_admin: true # enable HTTPS when use a separate port for Admin API.
+ port_admin: 9180 # use a separate port
+ https_admin: true # enable HTTPS when use a separate port for Admin API.
# Admin API will use conf/apisix_admin_api.crt and conf/apisix_admin_api.key as certificate.
+ admin_api_mtls:
+ mtls_enable: true # Enable or disable mTLS. Enable depends on `port_admin` and `https_admin`.
+ admin_ssl_cert: "../t/certs/mtls_server.crt" # Path of your self-signed server side cert.
Review comment:
No, we can not use test cert as default value.
##########
File path: doc/mtls.md
##########
@@ -0,0 +1,44 @@
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+[Chinese](zh-cn/mtls.md)
+
+## Enable mutual TLS authentication
+
Review comment:
We need more detailed documentation to tell the user why it needs to be turned on and how the admin API caller's certificate will be issued.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] moonming commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r444599336
##########
File path: .travis/linux_openresty_mtls_runner.sh
##########
@@ -0,0 +1,146 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+set -ex
+
+export_or_prefix() {
+ export OPENRESTY_PREFIX="/usr/local/openresty-debug"
+}
+
+create_lua_deps() {
+ echo "Create lua deps cache"
+
+ make deps
+ luarocks install luacov-coveralls --tree=deps --local > build.log 2>&1 || (cat build.log && exit 1)
+
+ sudo rm -rf build-cache/deps
+ sudo cp -r deps build-cache/
+ sudo cp rockspec/apisix-master-0.rockspec build-cache/
+}
+
+before_install() {
+ sudo cpanm --notest Test::Nginx >build.log 2>&1 || (cat build.log && exit 1)
+}
+
+do_install() {
+ export_or_prefix
+
+ wget -qO - https://openresty.org/package/pubkey.gpg | sudo apt-key add -
+ sudo apt-get -y update --fix-missing
+ sudo apt-get -y install software-properties-common
+ sudo add-apt-repository -y "deb http://openresty.org/package/ubuntu $(lsb_release -sc) main"
+ sudo add-apt-repository -y ppa:longsleep/golang-backports
+
+ sudo apt-get update
+ sudo apt-get install openresty-debug lua5.1 liblua5.1-0-dev
+
+ wget https://github.com/luarocks/luarocks/archive/v2.4.4.tar.gz
+ tar -xf v2.4.4.tar.gz
+ cd luarocks-2.4.4
+ ./configure --prefix=/usr > build.log 2>&1 || (cat build.log && exit 1)
+ make build > build.log 2>&1 || (cat build.log && exit 1)
+ sudo make install > build.log 2>&1 || (cat build.log && exit 1)
+ cd ..
+ rm -rf luarocks-2.4.4
+
+ sudo luarocks install luacheck > build.log 2>&1 || (cat build.log && exit 1)
+
+
+ if [ ! -f "build-cache/apisix-master-0.rockspec" ]; then
+ create_lua_deps
+
+ else
+ src=`md5sum rockspec/apisix-master-0.rockspec | awk '{print $1}'`
+ src_cp=`md5sum build-cache/apisix-master-0.rockspec | awk '{print $1}'`
+ if [ "$src" = "$src_cp" ]; then
+ echo "Use lua deps cache"
+ sudo cp -r build-cache/deps ./
+ else
+ create_lua_deps
+ fi
+ fi
+
+ # sudo apt-get install tree -y
+ # tree deps
+
+ git clone https://github.com/iresty/test-nginx.git test-nginx
+ make utils
+
+ git clone https://github.com/apache/openwhisk-utilities.git .travis/openwhisk-utilities
+ cp .travis/ASF* .travis/openwhisk-utilities/scancode/
+
+ ls -l ./
+}
+
+script() {
+ export_or_prefix
+ export PATH=$OPENRESTY_PREFIX/nginx/sbin:$OPENRESTY_PREFIX/luajit/bin:$OPENRESTY_PREFIX/bin:$PATH
+ openresty -V
+ sudo service etcd start
+
+ # enable mTLS
+ sed -i 's/\# port_admin: 9180/port_admin: 9180/' conf/config.yaml
+ sed -i 's/\# https_admin: true/https_admin: true/' conf/config.yaml
+ sed -i 's/mtls_enable: false/mtls_enable: true/' conf/config.yaml
+ sed -i 's#ca_cert: ""#ca_cert: "../t/certs/mtls_ca.crt"#' conf/config.yaml
+ sed -i 's#server_key: ""#server_key: "../t/certs/mtls_server.key"#' conf/config.yaml
+ sed -i 's#server_cert: ""#server_cert: "../t/certs/mtls_server.crt"#' conf/config.yaml
+
+ ./bin/apisix help
+ ./bin/apisix init
+ ./bin/apisix init_etcd
+ ./bin/apisix start
+
+ sleep 1
+ cat logs/error.log
+
+
+ code=$(curl -i -o /dev/null -s -w %{http_code} --cacert ./t/certs/mtls_ca.crt --key ./t/certs/mtls_client.key --cert ./t/certs/mtls_client.crt -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' https://admin.apisix.dev:9180/apisix/admin/routes)
Review comment:
I don't think it's a good implementation, and there's no real read/write from admin APIs.
we'd better add test cases in `/t/admin`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] membphis commented on a change in pull request #1747: feat: Support admin API authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r456228783
##########
File path: .travis.yml
##########
@@ -9,6 +9,8 @@ matrix:
services:
- docker
env: OSNAME=linux_openresty
+ - os: linux
Review comment:
run this test case in Github Action, the Travis-CI is slow.
what do you think? @nic-chen
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] nic-chen commented on a change in pull request #1747: feat: Support client-to-server authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r443602800
##########
File path: conf/cert/two_side_server.key
##########
@@ -0,0 +1,27 @@
+-----BEGIN RSA PRIVATE KEY-----
Review comment:
Had move to dir `t/`. And support the paths of certs could be configed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-apisix] membphis commented on a change in pull request #1747: feat: Support client-to-server authentication with SSL certificates
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #1747:
URL: https://github.com/apache/incubator-apisix/pull/1747#discussion_r443277272
##########
File path: .travis/linux_openresty_two_side_ssl_auth_runner.sh
##########
@@ -0,0 +1,136 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+set -ex
+
+export_or_prefix() {
+ export OPENRESTY_PREFIX="/usr/local/openresty-debug"
+}
+
+create_lua_deps() {
+ echo "Create lua deps cache"
+
+ make deps
+ luarocks install luacov-coveralls --tree=deps --local > build.log 2>&1 || (cat build.log && exit 1)
+
+ sudo rm -rf build-cache/deps
+ sudo cp -r deps build-cache/
+ sudo cp rockspec/apisix-master-0.rockspec build-cache/
+}
+
+before_install() {
+ sudo cpanm --notest Test::Nginx >build.log 2>&1 || (cat build.log && exit 1)
+}
+
+do_install() {
+ export_or_prefix
+
+ wget -qO - https://openresty.org/package/pubkey.gpg | sudo apt-key add -
+ sudo apt-get -y update --fix-missing
+ sudo apt-get -y install software-properties-common
+ sudo add-apt-repository -y "deb http://openresty.org/package/ubuntu $(lsb_release -sc) main"
+ sudo add-apt-repository -y ppa:longsleep/golang-backports
+
+ sudo apt-get update
+ sudo apt-get install openresty-debug lua5.1 liblua5.1-0-dev
+
+ wget https://github.com/luarocks/luarocks/archive/v2.4.4.tar.gz
+ tar -xf v2.4.4.tar.gz
+ cd luarocks-2.4.4
+ ./configure --prefix=/usr > build.log 2>&1 || (cat build.log && exit 1)
+ make build > build.log 2>&1 || (cat build.log && exit 1)
+ sudo make install > build.log 2>&1 || (cat build.log && exit 1)
+ cd ..
+ rm -rf luarocks-2.4.4
+
+ sudo luarocks install luacheck > build.log 2>&1 || (cat build.log && exit 1)
+
+
+ if [ ! -f "build-cache/apisix-master-0.rockspec" ]; then
+ create_lua_deps
+
+ else
+ src=`md5sum rockspec/apisix-master-0.rockspec | awk '{print $1}'`
+ src_cp=`md5sum build-cache/apisix-master-0.rockspec | awk '{print $1}'`
+ if [ "$src" = "$src_cp" ]; then
+ echo "Use lua deps cache"
+ sudo cp -r build-cache/deps ./
+ else
+ create_lua_deps
+ fi
+ fi
+
+ # sudo apt-get install tree -y
+ # tree deps
+
+ git clone https://github.com/iresty/test-nginx.git test-nginx
+ make utils
+
+ git clone https://github.com/apache/openwhisk-utilities.git .travis/openwhisk-utilities
+ cp .travis/ASF* .travis/openwhisk-utilities/scancode/
+
+ ls -l ./
+}
+
+script() {
+ export_or_prefix
+ export PATH=$OPENRESTY_PREFIX/nginx/sbin:$OPENRESTY_PREFIX/luajit/bin:$OPENRESTY_PREFIX/bin:$PATH
+ openresty -V
+ sudo service etcd start
+
+ mv -f ./conf/config-for-two-side-ssl-auth.yaml ./conf/config.yaml
Review comment:
that is not a good idea, we should generate `conf/config-for-two-side-ssl-auth.yaml` file by `conf/config.yaml`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org