Add support for blacklisting hosts to the HTTP runner

[Kami]

This pull request adds support for blacklisting hosts to the HTTP runner by adding a new hosts_blacklist attribute.

With this attribute, pack author can specify a list of hosts which will be blacklisted (aka won't work) with the HTTP runner.

For example, if an operator wants to allow StackStorm users to use HTTP runner, without being able to hit some hosts, they could define new mypack.http action which metadata could look something like this:

---
name: http
runner_type: "http-request"
parameters:
  hosts_blacklist:
    default:
        - "localhost"
        - "127.0.0.1"
        - "::1"
    immutable: true

NOTE: As with any feature which could be used in a security sensitive context, it's important to know that this feature doesn't cover all scenarios and security is all about layers.

This means that runner level filtering should be just one of those layers. Where security is very important, it should be combined with additional filtering on the network layer (e.g. firewall) and potentially also in combination with a custom StackStorm action / workflow which performs more complex logic (e.g. also resolves the hostname and checks the IP, etc.).

[Kami]

If others think it makes sense, I could also add new hosts_whitelist runner level parameter which would act as a whitelist and would be mutually exclusive with hosts_blacklist.

[m4dcoder]

This is a little bit confusing. At first, I thought host blacklist is indicating which action runner hosts cannot run the HTTP operation. Looking further into the code, it's actually restricting which web address in the URL is blacklisted. Maybe name this url_hosts_blacklist?

[Kami]

@m4dcoder Correct.

I'm fine with calling the parameter urls_hosts_blacklist. I will also update the parameter description to make it more clear.

[m4dcoder]

@Kami Does it make sense to make this a configurable option in st2.conf? I'm wondering if st2 admins want to add to the list, how would they configure it?

[Kami]

It could be solved both ways.

Nice thing about the runner parameter approach is that it's not global.

For example, with RBAC you could allow some (super) users to run / execute core.http action against arbitrary hosts, but for other regular users you could only allow them to run / execute custom limited.http action which limits a list of hosts on the runner (action metadata) level.

[m4dcoder]

@Kami Do you have an example how the RBAC policy would configure this? I'm ok with either or with both approach. I'm just wondering how admins can add to this black list.

[Kami]

Sorry, maybe I wasn't clear in my previous comment.

I just described one possible approach which would utilize existing RBAC primitives for limiting which actions a particular user can run in combination with immutable default action parameter value.

To define a blacklist, operator would need to define a custom action which sets a default value for the url_hosts_blacklist parameter and make that parameter immutable (as per the PR descriptions).

That's a similar approach which is already used for various use cases (e.g. chatops, etc.).

[Kami]

And to clarify it further - I also didn't put it in st2.conf because runners should be more or less standalone and I don't think putting runner specific config options inside st2.conf is a good idea.

Having said that, our only approach for configuring / changing runner behavior right now is via runner parameters and that's why I went with this approach. It might not be 100% ideal, but it's already there and this pattern has already been used before.

[Kami]

I pushed a change which renames the existing runner attribute and adds a new one for the whitelist approach - c074677.

[m4dcoder]

Can you add a unit test where both whitelist and blacklist are provided?

[Kami]

Added in c07f6d6.

While at it, I might also look at adding some integration tests (if it doesn't become too big of a rabbit hole), since we don't have any at the moment.

[Kami]

For the time being, I just decided to add some additional unit tests for the runner class itself - 25b43cb.

[m4dcoder]

LGTM. But just a note on the use case you are trying to address. The real solution you need to address is to sandbox actions from executing system commands and also network policies to restrict access to internal endpoints. This solution here is complementary to that and does not offer sufficient protection alone.