-
Notifications
You must be signed in to change notification settings - Fork 5.1k
feat(vault): Added an option to use {vault://} in specific fields of plugins #14775
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
98a6475
e2064f6
5ef5005
7df0b8a
7bd8bb3
a852681
c05fb9b
6d52709
59e6de3
89a263c
f3185d8
9f0e84c
2c83103
8be60d5
33e9a90
eebdbf1
a2a110f
6bdaab0
36828d1
cd8a648
f6ae29d
b8e188f
8aa9286
c190c95
ad160aa
b671a5e
17c9ac2
f89c5bb
0877600
cb41634
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| message: Added an option to use {vault://} in specific fields of plugins | ||
| type: feature | ||
| scope: Plugin |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -24,7 +24,7 @@ return { | |||||
| { created_at = typedefs.auto_timestamp_s }, | ||||||
| { consumer = { type = "foreign", reference = "consumers", required = true, on_delete = "cascade", }, }, | ||||||
| { key = { type = "string", required = false, unique = true, auto = true }, }, | ||||||
| { secret = { type = "string", auto = true }, }, | ||||||
| { secret = { type = "string", auto = true, referenceable = true }, }, | ||||||
|
||||||
| { secret = { type = "string", auto = true, referenceable = true }, }, | |
| { secret = { type = "string", auto = true }, }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see this as a valid point. The fields from consumer should be referenceable using the vault semantics. If its not referenceable (empty), then the user hasn't follow the guidelines. This is exactly the same as if user would add EMPTY secret directly into consumer part. Please advise @raoxiaoyan
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -31,8 +31,8 @@ local oauth2_credentials = { | |||||
| { created_at = typedefs.auto_timestamp_s }, | ||||||
| { consumer = { type = "foreign", reference = "consumers", required = true, on_delete = "cascade", }, }, | ||||||
| { name = { type = "string", required = true }, }, | ||||||
| { client_id = { type = "string", required = false, unique = true, auto = true }, }, | ||||||
| { client_secret = { type = "string", required = false, auto = true, encrypted = true }, }, -- encrypted = true is a Kong Enterprise Exclusive feature. It does nothing in Kong CE | ||||||
| { client_id = { type = "string", required = false, unique = true, auto = true, referenceable = true }, }, | ||||||
| { client_secret = { type = "string", required = false, auto = true, encrypted = true, referenceable = true }, }, -- encrypted = true is a Kong Enterprise Exclusive feature. It does nothing in Kong CE | ||||||
|
||||||
| { client_secret = { type = "string", required = false, auto = true, encrypted = true, referenceable = true }, }, -- encrypted = true is a Kong Enterprise Exclusive feature. It does nothing in Kong CE | |
| { client_secret = { type = "string", required = false, auto = true, encrypted = true }, }, -- encrypted = true is a Kong Enterprise Exclusive feature. It does nothing in Kong CE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont see this as an error, almost same as with jwt. Please advise @raoxiaoyan .
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| local helpers = require "spec.helpers" | ||
| local Entity = require "kong.db.schema.entity" | ||
| local plugins_schema_def = require "kong.db.schema.entities.plugins" | ||
| local conf_loader = require "kong.conf_loader" | ||
|
|
||
| local PLUGIN_NAME = "basic-auth" | ||
|
|
||
|
|
||
| describe(PLUGIN_NAME .. ": (schema-vault)", function() | ||
| local plugins_schema = assert(Entity.new(plugins_schema_def)) | ||
|
|
||
| lazy_setup(function() | ||
| local conf = assert(conf_loader(nil, { | ||
| vaults = "bundled", | ||
| plugins = "bundled", | ||
| })) | ||
|
|
||
| local kong_global = require "kong.global" | ||
| _G.kong = kong_global.new() | ||
| kong_global.init_pdk(kong, conf) | ||
|
|
||
| local plugin_schema = require("kong.plugins."..PLUGIN_NAME..".schema") | ||
| assert(plugins_schema:new_subschema(PLUGIN_NAME, plugin_schema)) | ||
| end) | ||
|
|
||
| it("should dereference vault value", function() | ||
| local env_name = "BASIC_AUTH_HIDE_CREDENTIALS" | ||
| local env_value = "true" | ||
|
|
||
| finally(function() | ||
| helpers.unsetenv(env_name) | ||
| end) | ||
|
|
||
| helpers.setenv(env_name, env_value) | ||
|
|
||
| local entity = plugins_schema:process_auto_fields({ | ||
| name = PLUGIN_NAME, | ||
| config = { | ||
| hide_credentials = "{vault://env/basic-auth-hide-credentials}" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And also, you have marked |
||
| }, | ||
| }, "select") | ||
|
|
||
| assert.equal(env_value, entity.config.hide_credentials) | ||
| end) | ||
| end) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| local helpers = require "spec.helpers" | ||
| local Entity = require "kong.db.schema.entity" | ||
| local plugins_schema_def = require "kong.db.schema.entities.plugins" | ||
| local conf_loader = require "kong.conf_loader" | ||
|
|
||
| local PLUGIN_NAME = "jwt" | ||
|
|
||
|
|
||
| describe(PLUGIN_NAME .. ": (schema-vault)", function() | ||
| local plugins_schema = assert(Entity.new(plugins_schema_def)) | ||
|
|
||
| lazy_setup(function() | ||
| local conf = assert(conf_loader(nil, { | ||
| vaults = "bundled", | ||
| plugins = "bundled", | ||
| })) | ||
|
|
||
| local kong_global = require "kong.global" | ||
| _G.kong = kong_global.new() | ||
| kong_global.init_pdk(kong, conf) | ||
|
|
||
| local plugin_schema = require("kong.plugins."..PLUGIN_NAME..".schema") | ||
| assert(plugins_schema:new_subschema(PLUGIN_NAME, plugin_schema)) | ||
| end) | ||
|
|
||
| it("should dereference vault value", function() | ||
| local env_name = "JWT_SECRET_IS_BASE64" | ||
| local env_value = "true" | ||
|
|
||
| finally(function() | ||
| helpers.unsetenv(env_name) | ||
| end) | ||
|
|
||
| helpers.setenv(env_name, env_value) | ||
|
|
||
| local entity = plugins_schema:process_auto_fields({ | ||
| name = PLUGIN_NAME, | ||
| config = { | ||
| secret_is_base64 = "{vault://env/jwt-secret-is-base64}" | ||
| }, | ||
| }, "select") | ||
|
|
||
| assert.equal(env_value, entity.config.secret_is_base64) | ||
| end) | ||
| end) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| local helpers = require "spec.helpers" | ||
| local Entity = require "kong.db.schema.entity" | ||
| local plugins_schema_def = require "kong.db.schema.entities.plugins" | ||
| local conf_loader = require "kong.conf_loader" | ||
|
|
||
| local PLUGIN_NAME = "hmac-auth" | ||
|
|
||
|
|
||
| describe(PLUGIN_NAME .. ": (schema-vault)", function() | ||
| local plugins_schema = assert(Entity.new(plugins_schema_def)) | ||
|
|
||
| lazy_setup(function() | ||
| local conf = assert(conf_loader(nil, { | ||
| vaults = "bundled", | ||
| plugins = "bundled", | ||
| })) | ||
|
|
||
| local kong_global = require "kong.global" | ||
| _G.kong = kong_global.new() | ||
| kong_global.init_pdk(kong, conf) | ||
|
|
||
| local plugin_schema = require("kong.plugins."..PLUGIN_NAME..".schema") | ||
| assert(plugins_schema:new_subschema(PLUGIN_NAME, plugin_schema)) | ||
| end) | ||
|
|
||
| it("should dereference vault value", function() | ||
| local env_name = "HMAC_AUTH_HIDE_CREDENTIALS" | ||
| local env_value = "true" | ||
|
|
||
| finally(function() | ||
| helpers.unsetenv(env_name) | ||
| end) | ||
|
|
||
| helpers.setenv(env_name, env_value) | ||
|
|
||
| local entity = plugins_schema:process_auto_fields({ | ||
| name = PLUGIN_NAME, | ||
| config = { | ||
| hide_credentials = "{vault://env/hmac-auth-hide-credentials}" | ||
| }, | ||
| }, "select") | ||
|
|
||
| assert.equal(env_value, entity.config.hide_credentials) | ||
| end) | ||
| end) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| local helpers = require "spec.helpers" | ||
| local Entity = require "kong.db.schema.entity" | ||
| local plugins_schema_def = require "kong.db.schema.entities.plugins" | ||
| local conf_loader = require "kong.conf_loader" | ||
|
|
||
| local PLUGIN_NAME = "oauth2" | ||
|
|
||
|
|
||
| describe(PLUGIN_NAME .. ": (schema-vault)", function() | ||
| local plugins_schema = assert(Entity.new(plugins_schema_def)) | ||
|
|
||
| lazy_setup(function() | ||
| local conf = assert(conf_loader(nil, { | ||
| vaults = "bundled", | ||
| plugins = "bundled", | ||
| })) | ||
|
|
||
| local kong_global = require "kong.global" | ||
| _G.kong = kong_global.new() | ||
| kong_global.init_pdk(kong, conf) | ||
|
|
||
| local plugin_schema = require("kong.plugins."..PLUGIN_NAME..".schema") | ||
| assert(plugins_schema:new_subschema(PLUGIN_NAME, plugin_schema)) | ||
| end) | ||
|
|
||
| it("should dereference vault value", function() | ||
| local env_name = "OAUTH2_HIDE_CREDENTIALS" | ||
| local env_value = "true" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use a proper value for testing. |
||
|
|
||
| finally(function() | ||
| helpers.unsetenv(env_name) | ||
| end) | ||
|
|
||
| helpers.setenv(env_name, env_value) | ||
|
|
||
| local entity = plugins_schema:process_auto_fields({ | ||
| name = PLUGIN_NAME, | ||
| config = { | ||
| hide_credentials = "{vault://env/oauth2-hide-credentials}" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You have marked these fields |
||
| }, | ||
| }, "select") | ||
|
|
||
| assert.equal(env_value, entity.config.hide_credentials) | ||
| end) | ||
| end) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marking the HMAC credential
secretfield asreferenceablemeans this value will be resolved viakong.vault.geton the data plane. If the vault reference cannot be resolved (for example, a missing or mis-typed environment variable),resolve_referenceinkong.db.schema.initreplaces it with an empty string, sohmac-authwill happily verify signatures using a known empty key, allowing an attacker who guesses the username to forge valid HMAC signatures. This field should fail closed on vault resolution errors (e.g., reject the credential or authentication) instead of silently falling back to an empty secret.