Mandate minimum TLS version of 1.2 for all storage accounts

alan-turing-institute / data-safe-haven

https://data-safe-haven.readthedocs.io

BSD 3-Clause "New" or "Revised" License

57 stars 15 forks source link

Mandate minimum TLS version of 1.2 for all storage accounts #2133

Closed craddm closed 1 month ago

craddm commented 1 month ago

:white_check_mark: Checklist

[x] You have given your pull request a meaningful title (e.g. Enable foobar integration rather than 515 foobar).
[x] 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).

:vertical_traffic_light: Depends on

:arrow_heading_up: Summary

As per pentest report, storage accounts in SREs currently allow a minimum TLS version of 1.0, which is deprecated.

Note that this also applies to the SHM storage account, and this PR also modifies the minimum version there.

:closed_umbrella: Related issues

:microscope: Tests

Deployed SRE using these changes.

[x] mounting the accounts inside the SRE
[x] reading/writing from inside the SRE
[x] accessing them from Azure Store Explorer

jemrobinson commented 1 month ago

Is this a from-scratch SRE? I'd like to know whether this causes any problems for any of:

mounting the accounts inside the SRE
reading/writing (as appropriate) from inside the SRE
accessing them from Azure Storage Explorer

Could you give details on this issue when you have them?

craddm commented 1 month ago

Is this a from-scratch SRE? I'd like to know whether this causes any problems for any of:

mounting the accounts inside the SRE

reading/writing (as appropriate) from inside the SRE

accessing them from Azure Storage Explorer

Could you give details on this issue when you have them?

It isn't a from scratch SRE - I deleted and redeployed the storage accounts. However, currently having issues restarting the gitea container group. Going to try a fresh deployment instead.

github-actions[bot] commented 1 month ago

Coverage report

This PR does not seem to contain any modification to coverable code.

craddm commented 1 month ago

Is this a from-scratch SRE? I'd like to know whether this causes any problems for any of:

mounting the accounts inside the SRE

reading/writing (as appropriate) from inside the SRE

accessing them from Azure Storage Explorer

Could you give details on this issue when you have them?

All worked fine as far as I can tell. Successfully uploaded and downloaded from ingress and egress via storage explore. Successfully copied from input and to output from the SRE desktop

jemrobinson commented 1 month ago

Cool - that's /ingress and /egress. Can you check /home (presumably working if you logged in) and /shared as well?

jemrobinson commented 1 month ago

@JimMadge : I'm comfortable that Matt has tested /data and /output (also /home and maybe /shared). Do you think we need/want an independent test of these?

craddm commented 1 month ago

@JimMadge : I'm comfortable that Matt has tested /data and /output (also /home and maybe /shared). Do you think we need/want an independent test of these?

Yes, home and shared working fine too

Just now deployed an SHM with the updated TLS too

jemrobinson commented 1 month ago

Happy to merge this @JimMadge ?