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 2022/01/27 04:01:50 UTC
[GitHub] [apisix] shuaijinchao opened a new pull request #6214: fix(admin-token): fixed token is used by default
shuaijinchao opened a new pull request #6214:
URL: https://github.com/apache/apisix/pull/6214
### What this PR does / why we need it:
https://lists.apache.org/thread/m6s1ozvhp8ww4mcw85q2m5mp4x7fkoxh
### Pre-submission checklist:
* [x] 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?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shuaijinchao commented on a change in pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#discussion_r793319518
##########
File path: conf/config.yaml
##########
@@ -43,5 +43,5 @@
apisix:
admin_key:
- name: admin
- key: edd1c9f034335f136f87ad84b625c8f1 # using fixed API token has security risk, please update it when you deploy to production environment
+ key: ${{APISIX_ADMIN_API_KEY}}
Review comment:
I think it would be more flexible to replace with an environment variable. Users can modify the admin token through system environment variables. If the environment variable is not read. At this point, APISIX will generate a random token for it and prompt it on standard output and logs.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shuaijinchao commented on a change in pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#discussion_r793338641
##########
File path: apisix/cli/file.lua
##########
@@ -52,6 +58,32 @@ local function tab_is_array(t)
end
+local function generate_random_str(len)
+ local rand_str = ""
+ local rand_num = 0
+
+ -- set random seed
+ randomseed(tostring(os_time()):reverse():sub(1, 5))
Review comment:
I got it, thanks for the pointers.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#discussion_r793302104
##########
File path: conf/config.yaml
##########
@@ -43,5 +43,5 @@
apisix:
admin_key:
- name: admin
- key: edd1c9f034335f136f87ad84b625c8f1 # using fixed API token has security risk, please update it when you deploy to production environment
+ key: ${{APISIX_ADMIN_API_KEY}}
Review comment:
any do need to remove https://github.com/apache/apisix/blob/master/apisix/admin/init.lua#L64-L68?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#discussion_r793310867
##########
File path: apisix/cli/file.lua
##########
@@ -52,6 +58,32 @@ local function tab_is_array(t)
end
+local function generate_random_str(len)
+ local rand_str = ""
+ local rand_num = 0
+
+ -- set random seed
+ randomseed(tostring(os_time()):reverse():sub(1, 5))
Review comment:
https://github.com/apache/apisix/blob/master/apisix/cli/ops.lua#L149 Maybe you can put this function in cli `init`
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shuaijinchao commented on a change in pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#discussion_r793329918
##########
File path: apisix/cli/file.lua
##########
@@ -52,6 +58,32 @@ local function tab_is_array(t)
end
+local function generate_random_str(len)
+ local rand_str = ""
+ local rand_num = 0
+
+ -- set random seed
+ randomseed(tostring(os_time()):reverse():sub(1, 5))
Review comment:
APISIX and CLI are two completely separate programs. There is no sharing between them. APISIX runs on OpenResty and the CLI runs on luajit or native lua.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shuaijinchao commented on a change in pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#discussion_r793524717
##########
File path: apisix/init.lua
##########
@@ -86,6 +88,12 @@ function _M.http_init(args)
core.log.error("failed to load the configuration: ", err)
end
end
+
+ local is_cli_token = getenv(cli_util.ADMIN_TOKEN_TAG_KEY)
+ if is_cli_token and tonumber(is_cli_token) == 1 then
+ core.log.warn("admin token has been automatically created by the system, ",
Review comment:
updated, thx~
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#discussion_r793270205
##########
File path: apisix/cli/file.lua
##########
@@ -52,6 +58,32 @@ local function tab_is_array(t)
end
+local function generate_random_str(len)
+ local rand_str = ""
+ local rand_num = 0
+
+ -- set random seed
+ randomseed(tostring(os_time()):reverse():sub(1, 5))
Review comment:
I don't think we need to set seed again https://github.com/apache/apisix/blob/427e9269efe32836051bf280ea2af4a566730ae4/apisix/init.lua#L98
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a change in pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#discussion_r793454944
##########
File path: apisix/init.lua
##########
@@ -86,6 +88,12 @@ function _M.http_init(args)
core.log.error("failed to load the configuration: ", err)
end
end
+
+ local is_cli_token = getenv(cli_util.ADMIN_TOKEN_TAG_KEY)
+ if is_cli_token and tonumber(is_cli_token) == 1 then
+ core.log.warn("admin token has been automatically created by the system, ",
Review comment:
system is too generic, using `apisix` will be 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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#discussion_r794145536
##########
File path: apisix/init.lua
##########
@@ -86,6 +88,12 @@ function _M.http_init(args)
core.log.error("failed to load the configuration: ", err)
end
end
+
+ local is_cli_token = getenv(cli_util.ADMIN_TOKEN_TAG_KEY)
+ if is_cli_token and tonumber(is_cli_token) == 1 then
+ core.log.warn("admin token has been automatically created by the apisix, ",
+ "value: `", getenv(cli_util.ADMIN_TOKEN_KEY), "`")
+ end
Review comment:
This is just a one-time token and should be very explicit about this
##########
File path: apisix/init.lua
##########
@@ -86,6 +88,12 @@ function _M.http_init(args)
core.log.error("failed to load the configuration: ", err)
end
end
+
+ local is_cli_token = getenv(cli_util.ADMIN_TOKEN_TAG_KEY)
+ if is_cli_token and tonumber(is_cli_token) == 1 then
+ core.log.warn("admin token has been automatically created by the apisix, ",
+ "value: `", getenv(cli_util.ADMIN_TOKEN_KEY), "`")
+ end
Review comment:
After Apache APISIX restarts, the token will change
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#discussion_r793300246
##########
File path: conf/config.yaml
##########
@@ -43,5 +43,5 @@
apisix:
admin_key:
- name: admin
- key: edd1c9f034335f136f87ad84b625c8f1 # using fixed API token has security risk, please update it when you deploy to production environment
+ key: ${{APISIX_ADMIN_API_KEY}}
Review comment:
According to the discussion on the mailing list, the default value should be empty? I don't understand what the environment variables here mean
##########
File path: conf/config.yaml
##########
@@ -43,5 +43,5 @@
apisix:
admin_key:
- name: admin
- key: edd1c9f034335f136f87ad84b625c8f1 # using fixed API token has security risk, please update it when you deploy to production environment
+ key: ${{APISIX_ADMIN_API_KEY}}
Review comment:
do we need to update https://github.com/apache/apisix/blob/master/conf/config-default.yaml#L91-L100?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#discussion_r793332466
##########
File path: apisix/cli/file.lua
##########
@@ -52,6 +58,32 @@ local function tab_is_array(t)
end
+local function generate_random_str(len)
+ local rand_str = ""
+ local rand_num = 0
+
+ -- set random seed
+ randomseed(tostring(os_time()):reverse():sub(1, 5))
Review comment:
I means you can make random seed of CLI in https://github.com/apache/apisix/blob/master/apisix/cli/ops.lua#L149
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shuaijinchao edited a comment on pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
shuaijinchao edited a comment on pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#issuecomment-1022925095
hi @moonming , Regarding setting the admin token to remove, I think it would be more flexible to replace with an environment variable. Users can modify the admin token through system environment variables. If the environment variable is not read. At this point, APISIX will generate a random token for it and prompt it on standard output and logs.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shuaijinchao commented on pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#issuecomment-1022925095
hi @moonming , Regarding setting the admin token to remove https://github.com/apache/apisix/pull/6214#discussion_r793302104, I think it would be more flexible to replace with an environment variable. Users can modify the admin token through system environment variables. If the environment variable is not read. At this point, APISIX will generate a random token for it and prompt it on standard output and logs.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shuaijinchao commented on pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#issuecomment-1055342991
the problem has not been confirmed, and the PR is temporarily closed.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shuaijinchao closed pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
shuaijinchao closed pull request #6214:
URL: https://github.com/apache/apisix/pull/6214
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shuaijinchao commented on a change in pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#discussion_r793302288
##########
File path: apisix/cli/file.lua
##########
@@ -52,6 +58,32 @@ local function tab_is_array(t)
end
+local function generate_random_str(len)
+ local rand_str = ""
+ local rand_num = 0
+
+ -- set random seed
+ randomseed(tostring(os_time()):reverse():sub(1, 5))
Review comment:
the cli program runs independently of APISIX, and the logic here has not been triggered when the cli is executed.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#discussion_r794148153
##########
File path: ci/linux_apisix_current_luarocks_runner.sh
##########
@@ -66,6 +67,10 @@ script() {
ulimit -n -S
ulimit -n -H
+ # set default admin token
+ sudo yamlq e -i '.apisix.admin_key[0].key = "edd1c9f034335f136f87ad84b625c8f1"' conf/config-default.yaml
Review comment:
why set token twice https://github.com/apache/apisix/pull/6214/files#diff-210dceffdb6579327475828287fd91c559b209de07ebf0323ab8005b589d9c0fR44？
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shuaijinchao commented on a change in pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on a change in pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#discussion_r794180151
##########
File path: ci/linux_apisix_current_luarocks_runner.sh
##########
@@ -66,6 +67,10 @@ script() {
ulimit -n -S
ulimit -n -H
+ # set default admin token
+ sudo yamlq e -i '.apisix.admin_key[0].key = "edd1c9f034335f136f87ad84b625c8f1"' conf/config-default.yaml
Review comment:
There are two role users in the default configuration template of APISIX, namely `admin` and `viewer`.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] shuaijinchao commented on pull request #6214: fix(admin-token): fixed token is used by default
Posted by GitBox <gi...@apache.org>.
shuaijinchao commented on pull request #6214:
URL: https://github.com/apache/apisix/pull/6214#issuecomment-1035812852
@moonming The current known security issue of `Admin API` is due to `X-REAL-IP` and has been fixed, should this PR continue?
I think the implementation of this PR can still improve the security capabilities of APISIX and does not conflict with the repair of `X-REAL-IP`.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org