Fixes #32513 - Check certificate serial only when in standalone mode

[adamruzicka]

No description provided.

[ekohl]

Is this really correct? Doesn't this open up a security hole? I think any certificate signed by the CA is allowed to connect and will get here.

[adamruzicka]

I wouldn't say so.

In the standalone deployment a client talks to smart proxy and if its certificate is signed by the ca, it is allowed to connect. Smart proxy then takes the body of the request and sends it over to smart proxy dynflow core, this time using the proxy's cert. Smart proxy dynflow core checks that the incoming connection uses the same cert in which case it lets the connection in.

When running inside smart proxy, we just drop the re-sending part and the rest works the same way.

Or is there something I'm missing?

[ekohl]

Is there a way we can verify this? Perhaps the forwarding can add a var to request.env to indicate it is forwarded and doesn't originate externally. That feels safer to me.

Am I right that this happens here:

smart_proxy_dynflow/lib/smart_proxy_dynflow/helpers.rb

Lines 4 to 9 in 93fe5c3

	def relay_request(from = %r{^/dynflow}, to = '')
	response = Proxy::Dynflow::Callback::Core.relay(request, from, to)
	content_type response.content_type
	status response.code
	body response.body
	end

If so, it looks like it's still a real HTTP request.

You may be right - now my knowledge of dynflow(_core) is limited so I'd appreciate some pointers.

[adamruzicka]

Is there a way we can verify this?

Verify what exactly?

Am I right that this happens here:

Correct, with relay itself being defined here https://github.com/theforeman/smart_proxy_dynflow/blob/master/lib/smart_proxy_dynflow/callback.rb#L11

[ekohl]

My reasoning is this:

• I applied your patch
• I changed the trusted_hosts in /etc/foreman-proxy/settings.yml so no host was trusted anymore
• curl --cert /etc/foreman/client_cert.pem --cacert /etc/foreman/proxy_ca.pem --key /etc/foreman/client_key.pem https://foreman-proxy.example.com:9090/dynflow/tasks/count?state=running

This returned a result. I suspect that without any checks on it, any host with a cert signed the CA can access dynflow tasks. In a Puppet CA based env this can also be hosts you don't want to trust. This is essentially what CVE-2016-3728 was about.

I think that do_authorize_with_trusted_hosts (or similar) should be called in non-standalone mode to verify the certificate.

[ekohl]

I think the PR/commit description no longer matches the content. That could use an update.

Will you also handle the Redmine issue? Looks like it may be in the wrong project.

[ekohl]

We also have this:
https://github.com/theforeman/smart-proxy/blob/6c0ee87d874fd10bb1757538182b8332a1c96833/lib/smart_proxy_main.rb#L49

That makes me think you can use helpers :Sinatra::Authorization or may even be already included in base.

Btw, I do think we should write real Rack middleware to handle auth. I submitted my WIP to theforeman/smart-proxy#789 just so it's not only on my desktop.

For the short term: is Settings.instance initialized when this Api class is loaded? Should it be if defined?(::Sinatra::Authorization::Helpers)?

[adamruzicka]

To be honest I'm not all that familiar with how exactly helpers in sinatra work. Just using helpers ... doesn't seem to work (see gist), maybe I'm missing something somewhere. Do you want me to fiddle with it or are we ok with an include, which the helpers seem to do under the hood anyway?

For the short term: is Settings.instance initialized when this Api class is loaded?

It should be. But let's play it safe, I don't want any surprises.

[ekohl]

I'm ok with include. Looks like it's how it's commonly used in the Smart Proxy itself as well.

[ekohl]

Verified on a nightly box:

# cd /usr/share/gems/gems/smart_proxy_dynflow_core-0.3.2
# curl https://patch-diff.githubusercontent.com/raw/theforeman/smart_proxy_dynflow/pull/79.patch | patch -p1
# systemctl restart foreman-proxy
# curl --cert /etc/foreman/client_cert.pem --cacert /etc/foreman/proxy_ca.pem --key /etc/foreman/client_key.pem https://server.example.com:9090/dynflow/tasks/count?state=running ; echo
{"count":0,"state":"running"}
# sed -i 's/- server.example.com/- doesnotmatch/' /etc/foreman-proxy/settings.yml
# systemctl restart foreman-proxy
#  curl --cert /etc/foreman/client_cert.pem --cacert /etc/foreman/proxy_ca.pem --key /etc/foreman/client_key.pem https://server.example.com:9090/dynflow/tasks/count?state=running ; echo
Untrusted client server.example.com attempted to access /tasks/count. Check :trusted_hosts: in settings.yml