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/09/17 06:08:55 UTC

[GitHub] [apisix] membphis opened a new pull request #2244: bugfix(CLI): if the user used default token and allow any IP to acces…

membphis opened a new pull request #2244:
URL: https://github.com/apache/apisix/pull/2244


   …s Admin API,
   
     will show a WARNING message.
   
   ### What this PR does / why we need it:
   
   if the user used the default token and allow any IP to access Admin API,   
   will show a WARNING message.
   
   ### Pre-submission checklist:
   
   * [ ] Did you explain what problem does this PR solve? Or what new features have been added?
   * [ ] Have you added corresponding test cases?
   * [ ] Have you modified the corresponding document?
   * [ ] Is this PR backward compatible?
   


----------------------------------------------------------------
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] [apisix] moonming commented on a change in pull request #2244: bugfix(CLI): if the user used default token and allow any IP to acces…

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2244:
URL: https://github.com/apache/apisix/pull/2244#discussion_r490027799



##########
File path: bin/apisix
##########
@@ -799,6 +799,8 @@ version:    print the version of apisix
 ]])
 end
 
+
+local is_checked_admin_key

Review comment:
       `is_checked_admin_key` is confusing, please take a better 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] [apisix] moonming merged pull request #2244: bugfix(CLI): if the user used default token and allow any IP to acces…

Posted by GitBox <gi...@apache.org>.
moonming merged pull request #2244:
URL: https://github.com/apache/apisix/pull/2244


   


----------------------------------------------------------------
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] [apisix] moonming merged pull request #2244: bugfix(CLI): if the user used default token and allow any IP to acces…

Posted by GitBox <gi...@apache.org>.
moonming merged pull request #2244:
URL: https://github.com/apache/apisix/pull/2244


   


----------------------------------------------------------------
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] [apisix] membphis commented on a change in pull request #2244: bugfix(CLI): if the user used default token and allow any IP to acces…

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2244:
URL: https://github.com/apache/apisix/pull/2244#discussion_r490290166



##########
File path: bin/apisix
##########
@@ -799,6 +799,8 @@ version:    print the version of apisix
 ]])
 end
 
+
+local is_checked_admin_key

Review comment:
       `checked_admin_key`, any better name suggestions?




----------------------------------------------------------------
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] [apisix] moonming commented on a change in pull request #2244: bugfix(CLI): if the user used default token and allow any IP to acces…

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2244:
URL: https://github.com/apache/apisix/pull/2244#discussion_r490026730



##########
File path: conf/config.yaml
##########
@@ -21,3 +21,9 @@
 #     host:
 #       - "http://127.0.0.1:2379"
 #
+apisix:
+  admin_key:
+    -
+      name: "admin"
+      key: edd1c9f034335f136f87ad84b625c8f1 # please update the default token for safe

Review comment:
       `using fixed API token has security risk, please update it when you deploy to production environment`

##########
File path: bin/apisix
##########
@@ -812,6 +814,51 @@ local function init()
     end
     -- print("etcd: ", yaml_conf.etcd.host)
 
+    -- check the Admin API token
+    if yaml_conf.apisix.enable_admin and yaml_conf.apisix.allow_admin then
+        for _, allow_ip in ipairs(yaml_conf.apisix.allow_admin) do
+            if allow_ip == "127.0.0.0/24" then
+                is_checked_admin_key = true
+            end
+        end
+    end
+
+    if yaml_conf.apisix.enable_admin and not is_checked_admin_key then
+        is_checked_admin_key = true
+        local help = [[
+
+%s
+Please set a new Admin API key and store it in the `conf/config.yaml` file.
+
+]]
+        if type(yaml_conf.apisix.admin_key) ~= "table" or
+           #yaml_conf.apisix.admin_key == 0
+        then
+            io.stderr:write(help:format("ERROR: missing valid Admin API Key"))
+            os.exit(1)
+        end
+
+        for _, admin in ipairs(yaml_conf.apisix.admin_key) do
+            if type(admin.key) == "table" then
+                admin.key = ""
+            else
+                admin.key = tostring(admin.key)
+            end
+
+            if admin.key == "" then
+                io.stderr:write(help:format("ERROR: missing valid Admin API Key"), "\n")
+                os.exit(1)
+            end
+
+            if admin.key == "edd1c9f034335f136f87ad84b625c8f1" then
+                io.stderr:write(
+                    help:format("WARNING: using the default Key is very dangerous."),

Review comment:
       `using the default Key is very dangerous.` -> `using fixed API token has security risk, please modify "admin_key" in 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] [apisix] moonming commented on a change in pull request #2244: bugfix(CLI): if the user used default token and allow any IP to acces…

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2244:
URL: https://github.com/apache/apisix/pull/2244#discussion_r490816408



##########
File path: bin/apisix
##########
@@ -812,6 +814,52 @@ local function init()
     end
     -- print("etcd: ", yaml_conf.etcd.host)
 
+    -- check the Admin API token
+    if yaml_conf.apisix.enable_admin and yaml_conf.apisix.allow_admin then
+        for _, allow_ip in ipairs(yaml_conf.apisix.allow_admin) do
+            if allow_ip == "127.0.0.0/24" then
+                checked_admin_key = true
+            end
+        end
+    end
+
+    if yaml_conf.apisix.enable_admin and not checked_admin_key then
+        checked_admin_key = true
+        local help = [[
+
+%s
+Please set a new Admin API key and store it in the `conf/config.yaml` file.
+
+]]
+        if type(yaml_conf.apisix.admin_key) ~= "table" or
+           #yaml_conf.apisix.admin_key == 0
+        then
+            io.stderr:write(help:format("ERROR: missing valid Admin API Key"))
+            os.exit(1)
+        end
+
+        for _, admin in ipairs(yaml_conf.apisix.admin_key) do
+            if type(admin.key) == "table" then
+                admin.key = ""
+            else
+                admin.key = tostring(admin.key)
+            end
+
+            if admin.key == "" then
+                io.stderr:write(help:format("ERROR: missing valid Admin API Key"), "\n")
+                os.exit(1)
+            end
+
+            if admin.key == "edd1c9f034335f136f87ad84b625c8f1" then
+                io.stderr:write(
+                    help:format([[WARNING: using fixed API token has security risk, ]]
+                                .. [[please modify "admin_key" in conf/config.yaml.]]),

Review comment:
       the same as https://github.com/apache/apisix/pull/2244/files#diff-62edadffc237f13dc28a694080d293a8R831




----------------------------------------------------------------
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