Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Commit

Permalink
security: don't crash on clair timeouts
Browse files Browse the repository at this point in the history
Moreover, I've also added a configuration option so the timeouts for the
registry and clair can be configured separately. This has been done
because the Clair timeout by default is 900 seconds, which might be a
bit too high for registries.

Fixes #1751

Signed-off-by: Miquel Sabaté Solà <[email protected]>
  • Loading branch information
mssola committed Apr 5, 2018
1 parent 1a73023 commit 459c195
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 11 deletions.
6 changes: 5 additions & 1 deletion config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ registry:
timeout:
value: 2

#Set timeout in seconds for read response from registry.
# Set timeout in seconds for read response from registry.
read_timeout:
value: 120

Expand Down Expand Up @@ -283,6 +283,10 @@ security:
# Clair.
health_port: 6061

# Timeout for HTTP requests with Clair. Defaults to 900 seconds, which is
# the default for Clair too.
timeout: 900

# zypper-docker can be run as a server with its `serve` command. This backend
# fetches the information as given by zypper-docker. Note that this feature
# from zypper-docker is experimental and only available through another branch
Expand Down
19 changes: 14 additions & 5 deletions lib/portus/http_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,22 @@ def parse_unhauthorized_response(res)
[auth_args["Bearer realm"], query_params]
end

# Performs an HTTP request to the given URI and request object. It returns an
# HTTP response that has been sent from the registry.
def get_response_token(uri, req)
# Performs an HTTP request to the given URI and request object. You may
# optionally pass a read and open timeout, otherwise the ones provided in
# the `registry` config will be used. It returns an HTTP response that has
# been sent from the registry.
def get_response_token(uri, req, timeout = nil)
open, read = if timeout
[timeout, timeout]
else
[APP_CONFIG["registry"]["timeout"]["value"],
APP_CONFIG["registry"]["read_timeout"]["value"]]
end

options = {
use_ssl: uri.scheme == "https",
open_timeout: APP_CONFIG["registry"]["timeout"]["value"].to_i,
read_timeout: APP_CONFIG["registry"]["read_timeout"]["value"].to_i
open_timeout: open.to_i,
read_timeout: read.to_i
}

Net::HTTP.start(uri.hostname, uri.port, options) do |http|
Expand Down
11 changes: 8 additions & 3 deletions lib/portus/security_backends/clair.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def fetch_layer(digest)
uri, req = get_request("/v1/layers/#{digest}?features=false&vulnerabilities=true", "get")
req["Accept"] = "application/json"
begin
res = get_response_token(uri, req)
res = get_response_token(uri, req, clair_timeout)
rescue *::Portus::Errors::NET => e
Rails.logger.tagged("clair.get") { Rails.logger.debug e.message }
return
Expand All @@ -94,8 +94,8 @@ def post_layer(index)
req.body = layer_body(digest, parent).to_json

begin
res = get_response_token(uri, req)
rescue SocketError, Errno::ECONNREFUSED => e
res = get_response_token(uri, req, clair_timeout)
rescue *::Portus::Errors::NET => e
Rails.logger.tagged("clair.post") { Rails.logger.debug e.message }
return
end
Expand Down Expand Up @@ -144,6 +144,11 @@ def handle_response(response, digest, kind)
def error_message(msg)
msg["Error"] && msg["Error"]["Message"] ? msg["Error"]["Message"] : msg
end

# Returns the integer value of timeouts for HTTP requests.
def clair_timeout
APP_CONFIG["security"]["clair"]["timeout"]
end
end
end
end
25 changes: 24 additions & 1 deletion spec/lib/portus/security/clair_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ def expect_cve_match(cves, given, expected)
before do
APP_CONFIG["security"] = {
"clair" => {
"server" => "http://my.clair:6060"
"server" => "http://my.clair:6060",
"timeout" => 900
}, "zypper" => {
"server" => ""
}, "dummy" => {
Expand Down Expand Up @@ -138,4 +139,26 @@ def expect_cve_match(cves, given, expected)

expect(res[:clair]).to be_empty
end

it "does not raise an exception for timeouts on post" do
VCR.turn_on!
res = {}

# Digest as returned by the VCR tape.
digest = "sha256:28c417e954d8f9d2439d5b9c7ea3dcb2fd31690bf2d79b94333d889ea26689d2"

# Unfortunately VCR is not good with requests that are meant to time
# out. For this, then, we will manually stub requests so they raise the
# expected error on this situation.
stub_request(:post, "http://my.clair:6060/v1/layers").to_raise(Net::ReadTimeout)
stub_request(:get, "http://my.clair:6060/v1/layers/#{digest}?" \
"features=false&vulnerabilities=true").to_raise(Errno::ECONNREFUSED)

VCR.use_cassette("security/clair-is-unknown", record: :none) do
clair = ::Portus::Security.new("coreos/dex", "unrelated")
res = clair.vulnerabilities
end

expect(res[:clair]).to be_empty
end
end
3 changes: 2 additions & 1 deletion spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@

APP_CONFIG["security"] = {
"clair" => {
"server" => ""
"server" => "",
"timeout" => 900
}, "zypper" => {
"server" => ""
}, "dummy" => {
Expand Down

0 comments on commit 459c195

Please sign in to comment.