Add script to renew NFS share Stored Access Policies

[JimMadge]

✅ Checklist

• You have given your pull request a meaningful title (e.g. Enable foobar integration rather than 515 foobar).
• You are targeting the appropriate branch. If you're not certain which one this is, it should be develop.
• Your branch is up-to-date with the target branch (it probably was when you started, but it may have changed since then).
• You have marked this pull request as a draft and added '[WIP]' to the title if needed (if you're not yet ready to merge).
• You have formatted your code using appropriate automated tools (for example ./tests/AutoFormat_Powershell.ps1 -TargetPath <path to file or directory> for Powershell).

⤴️ Summary

Add script to update SAS Access Policies for storage mounted to SRDs.
Creating new SAS tokens is not needed as they are bound to the permissions, start, and end dates of the Access Policy.

🌂 Related issues

Closes #895

🔬 Tests

Tested on SRE,

• Policies confirmed to be updated in the portal
• Access to mounted containers still works
• Restarted VM to ensure that the files in containers were not cached

Thanks to @craddm for helping me test and spotting that the SAS tokens are bound to a Stored Access Policy 🙏.

[jemrobinson]

Looks fine to me if someone can test it.

[JimMadge]

IIRC we found a problem with the scripts "arguments" containing disallowed characters and the exception suggested base64 encoding, so we will need to look at that.

[JimMadge]

Base64 issue fixed 🎉.

The script runs and SAS tokens appear to be updated correctly. The script seems to stall then, presumably the systemd commands.

[JimMadge]

Still stalling on the script. Everything seems fine, perhaps the script isn't returning for some reason? Do we need to add exit 0 or something?

[jemrobinson]

How does this deal with the possibility of SAS tokens being leaked (and therefore needing to be replaced)? Or are you arguing that we shouldn't be worried about that? If so, what's the reason for refreshing the policies every year?

[JimMadge]

How does this deal with the possibility of SAS tokens being leaked (and therefore needing to be replaced)? Or are you arguing that we shouldn't be worried about that? If so, what's the reason for refreshing the policies every year?

In that case (which I believe would required privilege escalation on an SRD) you would still only be able to use that token from either inside the SRE (through the private end point) or from one of the approved IP addresses (which we only use for the system managers, who have privileged access already).

So, I'm quite confident that is a low risk. Although, if you did know that it had happened, I think it would be sensible to revoke the old policy and create a new one.

I think here I'm trying to balance fixing the immediate issue in a timely way with making the ideal change. Keeping in mind this is soon to be deprecated.

[jemrobinson]

I guess my point is that if we're not worried about leakage of SAS tokens, then why not just set the default validity to the maximum value? Then we never have to worry about refreshing the tokens or policies.

[JimMadge]

I guess my point is that if we're not worried about leakage of SAS tokens, then why not just set the default validity to the maximum value?

Yes, I'm not trying to dodge that and it is a good question.

Here the maximum seems to be "valid forever". I think that is generally considered bad practice. Probably because of the risk of that forgetting about that kind of token and not revoking it when it is no longer needed.

In our case, that doesn't feel like a big risk as it can only be used in a very restricted number of places and (maybe) we expect the storage to be torn down when it is no longer needed. Might not be true for other users.

My thinking was, that is another issue.
Although, it might be possible to change those Stored Access Policies to be valid forever and do away with the renewal altogether.

[craddm]

LGTM. Tested on the current SRE and seems to be working correctly.

[JimMadge]

@jemrobinson do you want to block/close this PR, or open an issue for a better solution in the future?

My feeling is, a discussion about whether open-ended access policies are really such a bad idea here is worth having. However, I don't think that will be done in time for the next release, so this PR is a sufficient for now fix.

[jemrobinson]

Approving as I don't want to block this PR. I'm not 100% sure what problem it's solving though as I think the solution here is equivalent to just setting a really long default validity. If you could open a new issue under the 4.2.1 milestone, I think that would be fine.