Skip to content
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

Delete OCSP Response cache when certificate renewed #6198

Merged
merged 2 commits into from
Sep 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build/test-lua.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ resty \
--shdict "configuration_data 5M" \
--shdict "certificate_data 16M" \
--shdict "certificate_servers 1M" \
--shdict "ocsp_response_cache 1M" \
--shdict "balancer_ewma 1M" \
--shdict "balancer_ewma_last_touched_at 1M" \
--shdict "balancer_ewma_locks 512k" \
Expand Down
2 changes: 1 addition & 1 deletion rootfs/etc/nginx/lua/certificate.lua
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ local function fetch_and_cache_ocsp_response(uid, der_cert)
end
if forcible then
ngx.log(ngx.NOTICE, "removed an existing item when saving OCSP response, ",
"consider increasing shared dictionary size for 'ocsp_reponse_cache'")
"consider increasing shared dictionary size for 'ocsp_response_cache'")
end
end

Expand Down
6 changes: 6 additions & 0 deletions rootfs/etc/nginx/lua/configuration.lua
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ local pairs = pairs
local configuration_data = ngx.shared.configuration_data
local certificate_data = ngx.shared.certificate_data
local certificate_servers = ngx.shared.certificate_servers
local ocsp_response_cache = ngx.shared.ocsp_response_cache

local EMPTY_UID = "-1"

Expand Down Expand Up @@ -100,6 +101,11 @@ local function handle_servers()
end

for uid, cert in pairs(configuration.certificates) do
local old_cert = certificate_data:get(uid)
if old_cert ~= nil and old_cert ~= cert then
ocsp_response_cache:delete(uid)
end

local success, set_err, forcible = certificate_data:set(uid, cert)
if not success then
local err_msg = string.format("error setting certificate for %s: %s\n",
Expand Down
51 changes: 51 additions & 0 deletions rootfs/etc/nginx/lua/test/configuration_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ local configuration = require("configuration")
local unmocked_ngx = _G.ngx
local certificate_data = ngx.shared.certificate_data
local certificate_servers = ngx.shared.certificate_servers
local ocsp_response_cache = ngx.shared.ocsp_response_cache

function get_backends()
return {
Expand Down Expand Up @@ -184,6 +185,56 @@ describe("Configuration", function()
assert.same(ngx.status, ngx.HTTP_BAD_REQUEST)
end)

it("should not delete ocsp_response_cache if certificate remain the same", function()
ngx.shared.certificate_data.get = function(self, uid)
return "pemCertKey"
end

mock_ssl_configuration({
servers = { ["hostname"] = UUID },
certificates = { [UUID] = "pemCertKey" }
})

local s = spy.on(ngx.shared.ocsp_response_cache, "delete")
assert.has_no.errors(configuration.handle_servers)
assert.spy(s).was_not_called()
end)

it("should not delete ocsp_response_cache if certificate is empty", function()
ngx.shared.certificate_data.get = function(self, uid)
return nil
end

mock_ssl_configuration({
servers = { ["hostname"] = UUID },
certificates = { [UUID] = "pemCertKey" }
})

local s = spy.on(ngx.shared.ocsp_response_cache, "delete")
assert.has_no.errors(configuration.handle_servers)
assert.spy(s).was_not_called()
end)

it("should delete ocsp_response_cache if certificate changed", function()
local stored_entries = {
[UUID] = "pemCertKey"
}

ngx.shared.certificate_data.get = function(self, uid)
return stored_entries[uid]
end

mock_ssl_configuration({
servers = { ["hostname"] = UUID },
certificates = { [UUID] = "pemCertKey2" }
})

local s = spy.on(ngx.shared.ocsp_response_cache, "delete")

assert.has_no.errors(configuration.handle_servers)
assert.spy(s).was.called_with(ocsp_response_cache, UUID)
end)

it("deletes server with empty UID without touching the corresponding certificate", function()
mock_ssl_configuration({
servers = { ["hostname"] = UUID },
Expand Down