Fix client key permissions

[Sploder12]

There was a change in logic in when permissions are set for ssl certificates which caused newly created client certificates to have overly restrictive permissions. This fixes that.

Also there was a bug where unix permissions weren't converted to Windows permissions correctly. This fixes that.

Setting the permissions of the file to read by everyone does not change security since the parent folder maintains the user only access. However, in the future it would be nice to give read permissions directly to the user instead of everyone.

closes #4193

[ricab]

How come this affected only Windows? Or was that not so?

[Sploder12]

How come this affected only Windows? Or was that not so?

I never checked if it affected the other platforms, I wouldn't be surprised if it did

[ricab]

I tried this and it worked fine on a pristine install on Windows, but read-by-all private key?

[Sploder12]

I tried this and it worked fine on a pristine install on Windows, but read-by-all private key?

Yep, unfortunately that's how it was and as far as I know how it needs to be without a big change in the clients.

[holta]

I tried this and it worked fine on a pristine install on Windows, but read-by-all private key?

The failure was repeated for me: hypothetically (might) also relate to residue left over from 1.15.1 and/or 1.16 RC?

[Sploder12]

The failure was repeated for me: hypothetically (might) also relate to residue left over from 1.15.1 and/or 1.16 RC?

This fix doesn't affect existing files so residue from 1.16 RC would still cause this error

[ricab]

OK, we've discussed and I have understood why this isn't a problem.

[xmkg]

There was a change in logic in when permissions are set for ssl certificates which caused newly created client certificates to have overly restrictive permissions. This fixes that.

Setting the permissions of the file to read by everyone does not change security since the parent folder maintains the user only access. However, in the future it would be nice to give read permissions directly to the user instead of everyone.

closes #4193

@Sploder12 thanks for the fix!

If I understand it correctly, we're basically falling back to the parent folder's permissions, right? The user who installed the multipass would naturally have the proper permissions to the parent folder; therefore, they can access the file, but not the others.

[Sploder12]

If I understand it correctly, we're basically falling back to the parent folder's permissions, right? The user who installed the multipass would naturally have the proper permissions to the parent folder; therefore, they can access the file, but not the others.

Yep!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.31%. Comparing base (46b4059) to head (d307dbd).
⚠️ Report is 413 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4194   +/-   ##
=======================================
  Coverage   89.30%   89.31%           
=======================================
  Files         259      259           
  Lines       15684    15685    +1     
=======================================
+ Hits        14007    14009    +2     
+ Misses       1677     1676    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ricab]

Alright, I've verified this on Linux and Windows and all looks good now. Permissions on the root certificate, private keys, and client certificates all look correct now.

[ricab]

Why does this need to receive an int, rather than the original perms? All of set_permissions should be able to use constants therein, rather than magic numbers, no? Anyway, something for another time.

[ricab]

@xmkg I am going ahead and merging this, to speed things up, but if you could still review it after the fact (either here or in a new RC), I'd appreciate it. Thanks!