Make $pe_installer_source more flexible

[bastelfreak]

This makes the peadm::install plan more flexible. Previously the variable $pe_installer_source could point to an absolute URL directly to the desired .tar.gz. Now it can also point to a web directory. Then peadm will calculate the .tar.gz name and fetch it from the web directory.

A simple test for the peadm::upgrade plan can be done with the following parameters:

{
  "primary_host": "$fqdn",
  "version": "2023.7.0",
  "pe_installer_source": "https://s3.amazonaws.com/pe-builds/released/2023.7.0/"
}

Summary

Provide a detailed description of all the changes present in this pull request.

Additional Context

Add any additional context about the problem here.

Related Issues (if any)

Mention any related issues or pull requests.

Checklist

• 🟢 Spec tests.
• 🟢 Acceptance tests.

Changes include test coverage?

• Yes
• Not needed

Have you updated the documentation?

• Yes, I've updated the appropriate docs
• Not needed

[bastelfreak]

I haven't tested this yet, but a PE user asked me to implement it.

[bastelfreak]

I did some tests for upgrade and install and this works fine for me. @timidri is there a chance you could merge and release this soonish?

[timidri]

I did some tests for upgrade and install and this works fine for me. @timidri is there a chance you could merge and release this soonish?

Hi @bastelfreak, I am not the code owner, so the team does the ultimate merging and releasing.

Question: what problem is solved by this change? /cc @GSPatton

[bastelfreak]

The problem is that Puppet doesn't deliver a public mirror for PE releases. See: puppetlabs/puppet-enterprise_issues#46

Because my customers cannot mirror it in an automated way, they download the installers and put them all in one directory that's served via HTTPS. The different PE instances (around 3000) don't have access to the internet, only to internal mirrors. That's where PEADM will fetch it from. With this patch I can just provide PEADM a URL to the directory, it will calculate the archive name and fetch it from there.

I implemented this, instead of providing an absolute url to the archive, because the archives contain the OS major version (like el-9) and my customers run different operating systems. Figuring out the correct name before calling PEADM is a bit annoying.

[timidri]

@bastelfreak Thank you for the clarifications. This feature seems like a useful addition for a quite specific use case. @GSPatton - what do you think?

From the arch/tech perspective, I would recommend to:

• put the algorithm in a function so code needs not be duplicated across install and upgrade plans
• add a spec test for the logic

[bastelfreak]

This should be good to go now.

[timidri]

This looks fine to me.

[CoMfUcIoS]

CI is failing, also looks like also breaks Convert according to that ticket #469.

[bastelfreak]

@CoMfUcIoS can you please take another look again? CI is fine, and further debugging in #469 showed that this PR isn't related to it.

[bastelfreak]

Hi people, any chance this can be merged soonish? Is there anything missing?

[bastelfreak]

@CoMfUcIoS can you please take another look?

[bastelfreak]

@GSPatton do you have an update for me?

[GSPatton]

Hi @bastelfreak. Thank you for your contribution and your patience. We're currently working to get time with our internal security team to evaluate any potential security risks associated with this. As you can imagine, security is a top priority for us and we want to ensure a thorough review before proceeding.

I'll keep you updated on our progress and let you know as soon as we have more information.

Thanks again for your understanding!

Gavin

[GSPatton]

Hi @bastelfreak . Thank you for your patience on this.

After careful review with our security team, we have decided not to proceed with this request at this time.

This decision was made because we cannot rule out the possibility that a malicious actor could utilise this feature to host malicious content on the URL that the user will be installing from. We value your feedback and hope to address your needs in other ways in the future.

If you have any other suggestions or questions, please feel free to reach out.

Best regards,

Gavin

[bastelfreak]

Okay some thoughts:
I'm a bit disappointed that the security didn't respond directly / that there was no dialogue. I've a bit of a history with a security team (maybe a different one). They never responded with real names in emails and I tried to report security issues via https://www.puppet.com/security. Two times without an answer and the third case I couldn't even report properly because the GPG key wasn't valid for the email address.

to come back to the module:

we cannot rule out the possibility that a malicious actor could utilise this feature to host malicious content on the URL that the user will be installing from

That just doesn't make any sense to me. How is my implementation different to the pe_installer_source parameter in the peadm::upgrade plan from a security point of view? You could already provide a custom absolute URL. With my changes PEADM accepts a custom relative URL and calculates the filename. And this feature isn't even enabled by default, it's opt-in.

Does that mean that you will remove all parameters for user input now because it's considered dangerous? You're essentially saying that 'if someone enters malicious data, bad things could happen'. That's always the case and I don't see how my changes makes this worse?

A user can also set the password to 12345678 and disable all the password checks via Hiera. A User can disable TLS and make the puppetserver listen on http. A user can reconfigure puppetserver to accept any https connection. There are a couple of options that you can use to harden your stack or you use the options in another way to weaken your setup.

[GSPatton]

Update: We met with @bastelfreak and our security team to discuss next steps following the security team's initial rejection of this request. After reviewing the situation, we agreed that @bastelfreak will proceed by using the documented method for handling offline installs/upgrades, as outlined here. We'll remain in contact to provide assistance if needed. Therefore, I will close this PR.

Additionally, during the meeting, it was raised that this use case ties into a broader concern regarding offline installs/upgrades, which is captured in this issue. I'll be logging an internal feature request to address this and will share updates or reach out for any clarification as needed.

Thank you again @bastelfreak for your continued valuable contributions.