security: don't crash on clair timeouts

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 SUSE#1751

Signed-off-by: Miquel Sabaté Solà <msabate@suse.com>

config/config.yml

@@ -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
 
@@ -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

lib/portus/http_helpers.rb

@@ -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|

lib/portus/security_backends/clair.rb

@@ -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
@@ -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
@@ -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

spec/lib/portus/security/clair_spec.rb

@@ -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" => {
@@ -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

spec/spec_helper.rb

@@ -78,7 +78,8 @@
 
     APP_CONFIG["security"] = {
       "clair" => {
-        "server" => ""
+        "server"  => "",
+        "timeout" => 900
       }, "zypper" => {
         "server" => ""
       }, "dummy" => {