Skip to content

fix(*): fix security vulnerability and database selection #177

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

Conversation

outsinre
Copy link

  1. If two redis clients share the same connection but use different database, both clients might receive error upon set/get data.
  2. If a client without authentication shares a connection that is authenticated by other clients, this client bypass ACL.

FTI-5839

1. If two redis clients share the same connection but use different
   database, both clients might receive error upon set/get data.
2. If a client without authentication shares a connection that is
   authenticated by other clients, this client bypass ACL.
@bungle
Copy link
Owner

bungle commented May 2, 2024

@outsinre could you add tests?

@bungle
Copy link
Owner

bungle commented May 2, 2024

@outsinre can you also explain security vulnerability? I guess this is somewhat related to connection pooling that can be shared by some other app?

@bungle
Copy link
Owner

bungle commented May 2, 2024

This library supports pool_name:
https://github.com/bungle/lua-resty-session/blob/master/lib/resty/session/redis.lua#L191

So I guess it is just the default pool name generation (which this library does not try to do, it leaves that to either user of this library or the library this library uses for connecting redis)?

Comment on lines +778 to +827
--- calculate MAC with SHA256 that is compatibile with OpenSSL FIPS mode
local mac_sha256 do
local mac = require "resty.openssl.mac"
local to_hex = require "resty.string".to_hex

local MAC_ALGORITHM = "HMAC"
local DIGEST_ALGORITHM = "sha256"

-- @function utils.mac_sha256
-- @param string key HMAC key
-- @param string value payload
-- @return[1] string MAC in HEX format
-- @return[2] nil
-- @return[2] error message
--
-- @usage
-- local utils = require "resty.session.utils"
-- local ikm = utils.rand_bytes(32)
-- local nonce = utils.rand_bytes(32)
-- local key, err = utils.derive_hmac_sha256_key(ikm, nonce)
-- local mac, err = utils.mac_sha256(key, "hello world")
mac_sha256 = function(key, value)
if type(key) ~= "string" then
return nil, "key must be a string"
end
if type(value) ~= "string" then
return nil, "value must be a string"
end

local hmac, err = mac.new(key, MAC_ALGORITHM, nil, DIGEST_ALGORITHM)
if not hmac then
return nil, err
end

hmac:update(value)

local digest
digest, err = hmac:final()
if not digest then
return nil, err
end

local hex_mac
hex_mac, err = to_hex(digest)
if not hex_mac then
return nil, err
end
return hex_mac
end
end
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost no difference to (what is differences really to hmac_sha256):

hmac_sha256 = function(key, value)

@bungle
Copy link
Owner

bungle commented May 2, 2024

To be honest, I don't see any problem with this library. Should we have default pool name generation? Not sure. Should it be part of resty.redis library, why here? Or should it be problem of system that is using this library, in this case Kong?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants