Infrastructure roles

Now that we have the ability to generate JWT tokens with limited scope to a specific action, perhaps we could actually get rid of the oauth tokens and use a JWT instead?

Does it matter? The signature would be generated by the installer when it runs and deploys the custom facts. Once that is done, the secret should not be needed any more

Not at all. All I’m suggesting is extending my original fact based proposal with a signature fact in addition to each originally proposed uuid fact.

I like this. This means the initial fact upload performed from the installer has the information signed by the same mechanism. I assume the custom fact lives only for the sake of the installation. And if someone wants to setup continous update of the mapping, they need to create such signature as a custom fact.

Even if it lived longer, I assume the custom fact deployed by the installer is not any worse than the answer file. It does not contain the credential, just the signature. If it can only be read by root, that’s good even in that scenario.

Long term, I’m all for replacing the oauth with other type of credentials. But that increases the scope of this feature that we wanted to get to 2.5, because it’s changing what we already have and rely on in the installer registration feature.

Thinking out loud here: how will we ensure the facts are uploaded? There are plans to make Puppet optional and off by default. Then how does it work? On the other hand, it would also mean that the host is not present in Foreman at all so the relation doesn’t have to be there.

I think there’s no plan to stop using puppet in the installer, so this should continue working. But using facts mechanism is easily reusable by any other fact source, be it chef, ansible, salt, subscription-manager in case one wants to use that without puppet.

Correct, but the installer does not send facts. It runs Puppet in an agentless way and we take steps to isolate it from any config so it doesn’t attempt to connect to any server. Today the installer defaults to enabling Puppet on a machine. A Puppet check in results in the Puppetserver sending facts to Foreman which ends up creating a database entry. If you disable Puppetserver, there is no foreman.example.com host in Foreman (unless you use other means).

Roughly the flow is:

  • Install Foreman
  • Install Foreman Proxy
  • When both are done, register the Foreman Proxy (if desired)
  • If registration is enabled, ensure Puppet is started after the Proxy has been registered

This piece of code takes care of the last bit.

Only after that you can see some host entry show up.

You can verify this if you follow the steps documented in Defaulting Puppet to off in the Katello Scenario.

Ah, I was unaware of the fact, the puppetserver is necessary to be running for this. I thought we’d call the enc script after the proxy is registered. Today, the proxy needs to have Puppet feature enabled for the authorization, however we’d allow the alternative authorization by the facts signature. Is it a hard change to use the script directly with isolated puppet agent run? Or could we even use the facts gathered during the installer run?

Sounds like a very good idea. Thanks.

I guess, this would also mean, that use-cases like this are possible then: Run REX Job directly on Smart Proxy

Run a REX job on a smart proxy, even if the host (= which is the smart proxy) is not in the same org/location as the currently logged in user.

Update as of 2021-04-12:

To ensure an uuid fact (be it foreman or foreman-proxy) can be trusted, it has to be accompanied by a signature fact. This signature fact is a HMAC-SHA512 of the uuid, where oauth consumer secret is used as a key. If the signature is invalid or missing, the uuid fact is ignored and no linking is performed.

Foreman, puppet-foreman and puppet-foreman_proxy PRs were updated to reflect this.

The plan for when puppet is removed is to make the installer upload the necessary facts to foreman on its own by using modified enc script. Since we still have time until puppet goes away, I’d be glad if people could start looking at the opened PRs so we could get this in for 2.5 as-is. I would then proceed with getting it ready for uploading facts ourselves for 2.6.

1 Like

FWIW, I raised concerns about this design in the past, especially about the (secure) handling of the instance_id.

With the HMAC-SHA256 signature on the fact, and the way Foreman validates/filters it, I think this is sufficiently secure for our use case.

One thing that I’d raise: we make sure that the signature fact is not leaking in Foreman, but the users might have additional Puppet reporting endpoints configured (PuppetDB etc). While we don’t deploy these, we should document very prominently that we use facts for authentication and which facts should be filtered.

I still have my doubts about using facts for this. Why can’t we have the installer explicitly create the connection? Thinking out loud here.

What if you have the following endpoints:

  • GET /api/smart_proxies/:smart_proxy_id/hosts
  • POST /api/smart_proxies/:smart_proxy_id/hosts
  • DELETE /api/smart_proxies/:smart_proxy_id/hosts/:host_id

This would rely on the fact that we have a certname for a host that we can use to uniquely identify a host. Within Puppet (and thus the installer) this certname should be available.

It’d remove all of the concerns with the fact handling and move to a static assignment that the installer takes care of.

Implementation wise we can create a Puppet type (+ provider) smart_proxy_host that ensures the entity exists.

The only complexity is that at the time the installer runs, there is no host. We can solve that by also sending facts in the payload so that a host is created on the fly (using the regular fact parser) if needed.

So the POST payload would effectively be a set of facts:

{
  "certname": "myhost.example.com",
  "facts": {}
}

Security wise it would use the same API auth that’s also used elsewhere. For the installer that means the oauth credentials but other systems could also use username/password. In that case, the user permissions must be checked.

I do think this should be flexible enough to create an Ansible module for it as well.

I’m probably missing something, so please have a look.

Can you be more specific please?

What all concerns? Security concerns were IMHO resolved above.

These become very similar API endpoints as fact upload, with just one difference. It will also say the host represented by these facts is the smart proxy. Why don’t we rather extend the existing facts endpoint to do that for us? What is the benefit of explicit endpoints for this? Is that the existing oauth authorization code instead of verifying oauth secret based signature?

As @evgeni noted: facts can end up in other databases. Essentially it’s adding secrets in there which can make it a trusted channel. I can’t exactly say what could go wrong and that’s precisely why I’m cautious.

What may help to understand it is this. Foreman has capabilities to update its data from facts and you can use configuration management to make the facts fit what Foreman thinks (like with the hostname, domain, network interfaces). My general impression is that updating Foreman from facts (as the proposal is here as well) generally leads to problems. Making Foreman the source of truth is generally more reliable.

That’s why I proposed to be explicit in the installer.

I didn’t think the exact API through that much. Perhaps there’s a better way to get done.

My opinion is still that enhancing the Fact Parser with this challenging.

Looking ahead, it may also conflict with a general design that looks better to me regarding fact uploads. That is:

  1. Accept facts
  2. Store facts unaltered
  3. Start async fact processing
  4. Return job ID

If you would implement this, there is a moment when the secret data is in the raw fact upload.

I think the proposal here is to extend the existing capability of Foreman DB being updated based on incoming facts. Extension here is that some facts that would have impact on the infrastructure must be verified to be created by the real proxy. That’s why we introduce the signature that can only be created by the proxy knowing the oauth secret.

What secret data? The signature is not a secret. The hostname is not a secret either. Also I thought you suggest we use the same facts importing mechanism in these endpoints, would that be prone to the same issue then? I still don’t see the issue though.

The signed UUID is secret (which is why it’s filtered out).

Let’s say you have a group of admins who can see the hosts that actually run Foreman. Then there’s different users who do have root on their machine but are not Foreman admins. If they know the signed UUID, they can ensure facts are sent so that their machine will be registered as a host that runs a Smart Proxy.

So I disagree that it’s not secret. It very much is.

As @evgeni mentioned, Puppet uploads facts to FacterDB and there may be other places where it leaks.

Ok, that helps me to understand the concern, thanks. Now one thing I still don’t understand. How I can take advantage of the UUID if I don’t know the oauth and I can’t add the signature (and confirm, I’m the proxy who can register this UUID). Note that the signature is not something that would be automatically added to every fact upload coming from other hosts (or at least that was my understanding). Foreman would take the UUID fact into consideration only if the signature in the same fact set is valid.

Now my assumption is, the signature depends on more things than just the UUID value. Therefore it can’t be replayed. If that’s not the case, I see the problem of just stealing both facts and using it from another host.

If there is something that makes the UUID secret or if there’s a way to abuse it without knowing the oauth secret, then I agree, it should not be sent as a fact and I’d like your proposal more.

The signature is the secret, as it allows you to perform an attack like I pointed out in Infrastructure roles - #19 by evgeni

That perhaps depends on the signature implementation. If the signature takes also the timestamp into the signed data, you can’t take it and reuse the signature elsewhere. I think we agreed on using HMAC-SHA256, so if we sha256(oauth_secret + _timestamp + uuid) and on the Foreman side we verify the same, the only secret is the oauth_secret. That however is not send as any fact. Of course Foreman would need to have a few minutes time window to compare the incoming facts _timestamp and the time of the actual request.

If I’m not mistaken, the step 3 of your attack vector is mitigated by the above. There’s no way of injecting the instance_id fact to non-proxy host facts without knowing the oauth_secret. That is typically only stored in the proxy installer. If attacker gets access to it, that’s of course a problem, but a problem we already have today.

Correct, if we would add a time window, this would be eliminated. But I’d very much prefer if we would not design yet another signing scheme here – JWT exists and supports expiration.

But I also like Ewouds proposal with a simple API that doesn’t have those problems to begin with.

Alright, so to summarize there’s no security concern with the original proposal. The alternative with explicit endpoints seems more straightforward.

The con I see with the alternative is, the host must exist before the installer is ran. To mitigate that, it was suggested to also send facts so we can also create the host. To me that feels as a drawback because of duplicate the facts import logic, so I’d vote for the original proposal. What we use as a signature mechanism isn’t that important to me (JWT or whatever else) as long as it’s based on the same secret we use in the installer today - oauth_secret. Having said that I can live with additional endpoints too.

@aruzicka thoughts? Would it make sense to do the poll? Do you lean strongly towards one of these? At the end you’re implementing it, so the final decision is up to you.