Repair snapin download not working on HTTPS enabled servers

[Sebastian-Roth]

The code added in commit d0162c8 breaks snapin download on HTTPS enabled servers and is therefore not appropriate in all cases. There is no quick fix for that as we don't have the information about a node being setup as HTTP or HTTPS in the database object for each storage node. This was missed when adding HTTPS to FOG and needs some thorough consideration.

The quick fix is to just set "https" in the code or remove those five lines that were added when trying to quickly fix the issue with snapins not downloading in locations.

[Sebastian-Roth]

This was mentioned in the forums: https://forums.fogproject.org/topic/15981/snapin-cert-issues

Would be great to solve this before the next release.

[bthomas1234]

Is there any workaround to fix this?
The ones mentioned in the forums didnt work.
Using new smart installer gave new issues like token errors - https://forums.fogproject.org/topic/15981/snapin-cert-issues/6

And trying to modify the code to https didnt bring much difference - "As a quick fix you can edit /var/www/html/fog/lib/client/snapinclient.class.php go to line 166 or search for the string “location” to get to the right spot. Change “http” to “https” in line 166 and save."

Storage nodes have https enabled and locations tagged for client to pickup snapins but failing to download snapins as per fog.log on clients

[Yarli]

I managed to get around this by doing the modification as described by editing line 166.
You need to make sure you drop one of the %s from the sprintf and put in your full URL.
For example: sfprintf("http://%s%s",.....
becomes: sprintf("https://fog.mydomain.com/%s".....

I also have a proposed solution based around that but i've not fully explored if this is possible yet or not.

If we can add an additional field to the nfsGroupMembers (Storage Nodes) table.
Add a field called ngmFullAddress which will then hold the full URI or IP address, for accessing this system.
For example you can then set that to https://fog.mydomain.com etc

If we can then ensure this field is passed through to the $StorageNode class. Again I've not looked at that yet otherwise I'd post that code too.
Then modify line 165 of the /var/www/html/fog/lib/client/snapinclient.class.php file to be as follows:
$location = sprintf( '%s/%s', (empty($StorageNode->get('fulladdress'))?"http://".$StorageNode->get('ip'):$StorageNode->get('externaladdress'), $StorageNode->get('webroot') );
Obviously there's a cleaner solution by also adding another field for "Use HTTPS" etc. Just popping up basic solution to see whether it's viable or if there's issues with doing this.

If I get time I'll do a pull request and make the changes and test. If it works then i'll submit that as a proposed solution to this, unless @Sebastian-Roth can see any flaws in this proposed solution ?

[Sebastian-Roth]

@bthomas1234 Thanks for posting here. I wasn't sure whether this is actually causing people trouble or if no one cares about it. Don't get me wrong. I just say this due to the fact we didn't have much talk about this in the forums (main source of input from users). We are going towards a next release and I am hoping we can get this fixed - but we don't have enough people to work on this.

@Yarli Awesome you've looked into this. I appreciate your input. You are very welcome to send a pull request (towards the dev-branch please). But I consider it wise to discuss things here before you dig into the details of coding. I can see two different routes here though I have not looked into it thoroughly yet. So it's more of a rough idea.

• Add a field to nfsGroupMembers as suggested. Be it something like UseHTTPS or fulladdress. Either of those need code to manage that new field, plus the users need to properly set the field information to make it work (or more code logic to ensure this is correct). As we are in a project phase where changing database fields causes a lot of overhead (!) I tend to evade this wherever possible. I know this is not great and we need to get out of this as soon as possible but that's what we are at right now.
• Instead adding a logic to the code to decide whether HTTP works or HTTPS is needed (or maybe even test HTTPS first).
  • This has several benefits:
    • misconfiguration cannot happen (but we need to make sure the logic is rock solid)
    • working towards HTTPS-only with FOG in the future we don't need to make sure the field is updated for existing installations upgrading to HTTPS-only
    • evade all the extra work it needs to add a database filed in the current situation
  • Though there is a downside to this as well:
    • This kind of attitude (put in more logic to help the user) has made FOG complex in certain aspects and it makes the user depend on someone to fix issues. Some months ago we decided to leave it to the user more often and provide better documentation instead of adding logic to the code.

Considering the situation we have here I tend to go the second route. What do you think about it?

[Sebastian-Roth]

@Yarli Do you think you'll find some time to look into this in the next few days?

[Yarli]

Ive got a busy week lined up but ill see if i can pull the dev branch down at work today and have a go. My biggest issue is i need to understand the process on getting a new field added to the database. I presume thats part of the fog install script ? Adding the UI changes and backend changes should hopefully be fairly straightforward.

…

On 3 Mar 2022, at 14:53, Sebastian-Roth ***@***.***> wrote:  @Yarli Do you think you'll find some time to look into this in the next few days? — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.

[Sebastian-Roth]

@Yarli Did you read my post start to end? I still don't think adding the field is the best way of fixing this issue. That way you don't need to worry about DB fields at all.

[Yarli]

Sorry about that, I didn't fully digest your reply as I working off my phone at the time.

Looking at your suggestion of doing a detect HTTPS method, I think that'll work for most users providing it's internal only.
Those exposing their FOG server to the internet like we are, will run into problems with it still trying to use the internal IP of the storage node. Changing the IP to the external address would then pose a new problem if you are not also exposing the TFTP ports to the internet.

In our setup, which is a bit weird and is certainly an edge case, we use Cloudflare and HA Proxy together to expose the HTTP part our internal FOG server ONLY, which allows us to remotely deploy snapins our end points without them needing to be on our VPN.
We still require the end point to be on our internal network if we are imaging them etc.

Clients sat on our internal network still connect through our HAProxy setup, since we have the IP for our FOG DNS record pointing at our HAProxy. This allows us to still use HTTPS even though there's no HTTPS enabled on our FOG server. This also allows us to manage certificates at the proxy instead of within the FOG server and allows us to take advantage of letsencrypt free certs too, which auto renew on our HAProxy as well.

That was my thinking behind having an external address field on the storage node for the snapin downloads since I suspect no one will try to image over the Internet, but would like the option to still deploy snapins etc.
Changing the IP of the storage node, certainly for us, would then break the imaging side of things since we don't expose the TFTP ports externally.

Happy to assist with the code on this, but wanted to explain a little about our setup and why it causes us an issue first so that we can work on a solution which hopefully works for everyone.

[Sebastian-Roth]

@Yarli Sorry for the long delay. Thanks for the detailed explanation on your setup. I am not sure how to handle this topic. On the one hand it would be easy to fix the basic issue by trying HTTPS and then HTTP URL subsequently. On the other hand I can see why you suggest adding the DB field (fulladdress) as it helps with your special setup just as well. With a lack of time on my end I tend to just implement the simple route for now. You are welcome to look into adding the later yourself.

[Yarli]

Ok thank you. Sorry that ive not had chance to test or develop a solution yet either. Work is fairly busy at the moment and I've not had time to setup a sandbox environment to work on this. Hoping once things die down a bit i can get onto that but that might be a while. Feel free for anyone to push out a simple solution in the mean time and ill work on a more long term solution once i get a but more free time, if that sounds ok ?

…

On 16 Apr 2022, at 17:22, Sebastian-Roth ***@***.***> wrote:  @Yarli Sorry for the long delay. Thanks for the detailed explanation on your setup. I am not sure how to handle this topic. On the one hand it would be easy to fix the basic issue by trying HTTPS and then HTTP URL subsequently. On the other hand I can see why you suggest adding the DB field (fulladdress) as it helps with your special setup just as well. With a lack of time on my end I tend to just implement the simple route for now. You are welcome to look into adding the later yourself. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

[Sebastian-Roth]

@Yarli @bthomas1234 I finally found some time to look into this again and came up with a nice solution I reckon. The URL for snapin download is only different from the default master node in case you are using the location plugin (and SNAPIN_LOCATION_SEND_ENABLED is set). So rather than adding a database field to the storage node table I added a new field to the location table which is not part of the general FOG schema definition and changes are less complicated to do.

This works out of the box for fresh installs as we can detect if a storage node is HTTP or HTTPS when a new location definition is being added.

For existing installations I suggest you first add the DB column manually:

ALTER TABLE location ADD lStorageNodeProto ENUM('http', 'https') AFTER lTftpEnabled;

Then update the protocol information for each existing location like this:

UPDATE location SET lStorageNodeProto="https" WHERE lStorageNodeID=X;

Or you can update all the locations if they are all on the same protocol (http or https):

UPDATE location SET lStorageNodeProto="https";

[Sebastian-Roth]

Closing as fixed - see commit b33e4d9.