Skip to content

Commit

Permalink
Merge pull request #2328 from craddm/template-checklist
Browse files Browse the repository at this point in the history
[WIP] Add downloadable template security checklist
  • Loading branch information
JimMadge authored Dec 5, 2024
2 parents 4409e5c + 89579f7 commit 8ab3117
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 74 deletions.
30 changes: 22 additions & 8 deletions data_safe_haven/commands/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import typer

from data_safe_haven import console
from data_safe_haven.administration.users import UserHandler
from data_safe_haven.config import ContextManager, DSHPulumiConfig, SHMConfig, SREConfig
from data_safe_haven.exceptions import DataSafeHavenError
Expand Down Expand Up @@ -120,9 +121,9 @@ def register(
# Load SHMConfig
try:
shm_config = SHMConfig.from_remote(context)
except DataSafeHavenError:
except DataSafeHavenError as exc:
logger.error("Have you deployed the SHM?")
raise
raise typer.Exit(1) from exc

# Load Pulumi config
pulumi_config = DSHPulumiConfig.from_remote(context)
Expand All @@ -132,7 +133,7 @@ def register(
if sre_config.name not in pulumi_config.project_names:
msg = f"Could not load Pulumi settings for '{sre_config.name}'. Have you deployed the SRE?"
logger.error(msg)
raise DataSafeHavenError(msg)
raise typer.Exit(1)

# Load GraphAPI
graph_api = GraphApi.from_scopes(
Expand All @@ -146,16 +147,29 @@ def register(

# List users
users = UserHandler(context, graph_api)
available_usernames = users.get_usernames_entra_id()
available_users = users.entra_users.list()
user_dict = {
user.preferred_username.split("@")[0]: user.preferred_username.split("@")[1]
for user in available_users
}
usernames_to_register = []
for username in usernames:
if username in available_usernames:
usernames_to_register.append(username)
if user_domain := user_dict.get(username):
if shm_config.shm.fqdn not in user_domain:
console.print(
f"User [green]'{username}[/green]'s principal domain name is [blue]'{user_domain}'[/blue].\n"
f"SRE [yellow]'{sre}'[/yellow] belongs to SHM domain [blue]'{shm_config.shm.fqdn}'[/blue]."
)
logger.error(
"The user's principal domain name must match the domain of the SRE to be registered."
)
else:
usernames_to_register.append(username)
else:
logger.error(
f"Username '{username}' does not belong to this Data Safe Haven deployment."
" Please use 'dsh users add' to create it."
)
console.print("Please use 'dsh users add' to create this user.")
users.register(sre_config.name, usernames_to_register)
except DataSafeHavenError as exc:
logger.critical(f"Could not register Data Safe Haven users with SRE '{sre}'.")
Expand Down Expand Up @@ -259,8 +273,8 @@ def unregister(
else:
logger.error(
f"Username '{username}' does not belong to this Data Safe Haven deployment."
" Please use 'dsh users add' to create it."
)
console.print("Please use 'dsh users add' to create it.")
for group_name in (
f"{sre_config.name} Users",
f"{sre_config.name} Privileged Users",
Expand Down
1 change: 1 addition & 0 deletions docs/source/deployment/security_checklist.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Organisations are responsible for making their own decisions about the suitabili
```

In this check list we aim to evaluate our deployment against the {ref}`security configuration <design_turing_security_configuration>` that we apply at the Alan Turing Institute.
A copy of this template in Markdown format is {download}`available for download <security_checklist/security_checklist_template.md>`.
The security checklist currently focuses on checks that can evaluate these security requirements for {ref}`policy_tier_2` (or greater) SREs (with some steps noted as specific to a tier):

## How to use this checklist
Expand Down
Original file line number Diff line number Diff line change
@@ -1,139 +1,136 @@
# Security checklist

Running on SHM/SREs deployed using commit <abc>
Running on SHM/SREs deployed using commit xxxxxx

## Summary

- :white_check_mark: <x> tests passed
- :partly_sunny: <x> tests partially passed (see below for more details)
- :fast_forward: <x> tests skipped (see below for more details)
- :x: <x> tests failed (see below for more details)
- :white_check_mark: x tests passed
- :partly_sunny: x tests partially passed (see below for more details)
- :fast_forward: x tests skipped (see below for more details)
- :x: x tests failed (see below for more details)

## Details

Some security checks were skipped because:

-
-
- Any additional details as referred to in the summary

### Multifactor Authentication and Password strength

- :white_check_mark: Check: Users can reset their own password
- <summary><b>Verify that:</b> User can reset their own password</summary>
<img src=""/>
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Check: Users can reset their own password
- <summary><b>Verify that:</b> User can reset their own password</summary>
<img src=""/>
- :white_check_mark: Check: non-registered users cannot connect to any SRE workspace
- <summary> <b>Verify that:</b> User can authenticate but cannot see any workspaces</summary>
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Check: non-registered users cannot connect to any SRE workspace
- <summary> <b>Verify that:</b> User can authenticate but cannot see any workspaces</summary>
<img src=""/>
- :white_check_mark: Check: registered users can see SRE workspaces
- <summary> <b>Verify that:</b> User can authenticate and can see workspaces</summary>
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Check: registered users can see SRE workspaces
- <summary> <b>Verify that:</b> User can authenticate and can see workspaces</summary>
<img src=""/>
- :white_check_mark: Check: Authenticated user can access workspaces
- <summary> <b>Verify that:</b> You can connect to any workspace</i> </summary>
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Check: Authenticated user can access workspaces
- <summary> <b>Verify that:</b> You can connect to any workspace</i> </summary>
<img src=""/>

### Isolated Network

- :white_check_mark: Fail to connect to the internet from a workspace
- <summary> <b>Verify that:</b> Browsing to the service fails</summary>
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Fail to connect to the internet from a workspace
- <summary> <b>Verify that:</b> Browsing to the service fails</summary>
<img src=""/>
- <summary> <b>Verify that:</b> You cannot access the service using curl</summary>
- <summary> <b>Verify that:</b> You cannot access the service using curl</summary>
<img src=""/>
- <summary> <b>Verify:</b> You cannot get the IP address for the service using nslookup</summary>
- <summary> <b>Verify:</b> You cannot get the IP address for the service using nslookup</summary>
<img src=""/>

### User devices

#### Tier 2
#### Tier 2:

- Connect to the environment using an allowed IP address and credentials
- :white_check_mark: <b>Verify that:</b> Connection succeeds
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> Connection succeeds
- Connect to the environment from an IP address that is not allowed but with correct credentials
- :white_check_mark: <b>Verify that:</b> Connection fails
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> Connection fails

#### Tier 3
#### Tier 3:

- All managed devices should be provided by a known IT team at an approved organisation.
- :fast_forward: <b>Verify that:</b> the IT team of the approved organisation take responsibility for managing the device.
- :fast_forward: <b>Verify that:</b> the user does not have administrator permissions on the device.
- :fast_forward: <b>Verify that:</b> allowed IP addresses are exclusive to managed devices.
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> the IT team of the approved organisation take responsibility for managing the device.
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> the user does not have administrator permissions on the device.
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> allowed IP addresses are exclusive to managed devices.
- Connect to the environment using an allowed IP address and credentials
- :fast_forward: <b>Verify that:</b> Connection succeeds
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> Connection succeeds
- Connect to the environment from an IP address that is not allowed but with correct credentials
- :fast_forward: <b>Verify that:</b> Connection fails
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> Connection fails

#### Tiers 2 and above
#### Tiers 2 and above:

- :white_check_mark: Network rules permit access only from allow-listed IP addresses
- In the Azure portal navigate to the Guacamole application gateway NSG for this SRE shm-<SHM NAME>-sre-<SRE NAME>-nsg-application-gateway
- <summary> <b>Verify that:</b> the NSG has network rules allowing Inbound access from allowed IP addresses only</summary>
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Network rules permit access only from allow-listed IP addresses
- In the Azure portal navigate to the Guacamole application gateway NSG for this SRE shm-<SHM NAME>-sre-<SRE NAME>-nsg-application-gateway
- <summary> <b>Verify that:</b> the NSG has network rules allowing Inbound access from allowed IP addresses only</summary>
<img src=""/>
- :white_check_mark: all other NSGs have an inbound Deny All rule and no higher priority rule allowing inbound connections from outside the Virtual Network
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: all other NSGs have an inbound Deny All rule and no higher priority rule allowing inbound connections from outside the Virtual Network

### Physical security

#### Tier 3 only

- Attempt to connect to the Tier 3 SRE web client from home using a managed device and the correct VPN connection and credentials.
- :fast_forward: <b>Verify that</b>: connection fails.
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that</b>: connection fails.
- Attempt to connect from research office using a managed device and the correct VPN connection and credentials.
- :fast_forward: <b>Verify that</b>: connection succeeds
- :fast_forward: <b>Verify that</b>: the network IP ranges corresponding to the research spaces correspond to those allowed by storage account firewall
- :fast_forward: <b>Verify that</b>: physical measures such as screen adaptions or desk partitions are present if risk of visual eavesdropping is high
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that</b>: connection succeeds
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that</b>: the network IP ranges corresponding to the research spaces correspond to those allowed by storage account firewall
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that</b>: physical measures such as screen adaptions or desk partitions are present if risk of visual eavesdropping is high

### Remote connections

- :white_check_mark: Unable to connect as a user to the remote desktop server via SSH
- <summary> <b>Verify that:</b> SSH login by fully-qualified domain name fails</summary>
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Unable to connect as a user to the remote desktop server via SSH
- <summary> <b>Verify that:</b> SSH login by fully-qualified domain name fails</summary>
<img src=""/>
- <summary> <b>Verify that:</b> SSH login by public IP address fails</summary>
- <summary> <b>Verify that:</b> SSH login by public IP address fails</summary>
<img src=""/>
- :white_check_mark: <b>Verify that:</b> the remote desktop web client application gateway (shm-<SHM ID>-sre-<SRE ID>-ag-entrypoint) and the firewall are the only SRE resources with public IP addresses.

- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> the remote desktop web client application gateway (shm-<SHM ID>-sre-<SRE ID>-ag-entrypoint) and the firewall are the only SRE resources with public IP addresses.

### Copy-and-paste

- Unable to paste text from a local device into a workspace
- :white_check_mark: <b>Verify that:</b> paste fails
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> paste fails
- Unable to copy text from a workspace to a local device
- :white_check_mark: <b>Verify that:</b> paste fails
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> paste fails

### Data ingress

- Check that the **System Manager** can send an upload token to the **Dataset Provider Representative**
- :white_check_mark: <b>Verify that:</b> the upload token is successfully created.
- :white_check_mark: <b>Verify that:</b> you are able to send this token using a secure mechanism.
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> the upload token is successfully created.
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> you are able to send this token using a secure mechanism.
- Ensure that data ingress works only for connections from the accepted IP address range
- :white_check_mark: <b>Verify that:</b> writing succeeds by uploading a file
- :white_check_mark: <b>Verify that:</b> attempting to open or download any of the files results in the following error: "Failed to start transfer: Insufficient credentials" under the Activities pane at the bottom of the MS Azure Storage Explorer window.
- :white_check_mark: <b>Verify that:</b> the access token fails when using a device with a non-allowed IP address
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> writing succeeds by uploading a file
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> attempting to open or download any of the files results in the following error: "Failed to start transfer: Insufficient credentials" under the Activities pane at the bottom of the MS Azure Storage Explorer window.
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> the access token fails when using a device with a non-allowed IP address
- Check that the upload fails if the token has expired
- :white_check_mark: <b>Verify that:</b> you can connect and write with the token during the duration
- :white_check_mark: <b>Verify that:</b> you cannot connect and write with the token after the duration has expired
- :white_check_mark: <b>Verify that:</b>the data ingress process works by uploading different kinds of files, e.g. data, images, scripts (if appropriate)
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> you can connect and write with the token during the duration
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> you cannot connect and write with the token after the duration has expired
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b>the data ingress process works by uploading different kinds of files, e.g. data, images, scripts (if appropriate)

### Data egress

- Confirm that a non-privileged user is able to read the different storage volumes and write to output
- :white_check_mark: <b>Verify that:</b> the `/mnt/output` volume exists and can be written to
- :white_check_mark: <b>Verify that:</b> the permissions of other storage volumes match that described in the user guide
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> the `/mnt/output` volume exists and can be written to
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> the permissions of other storage volumes match that described in the user guide
- Confirm that <b>System Manager</b> can see and download files from output
- :white_check_mark: <b>Verify that:</b> you can see the files written to the `/mnt/output` storage volume.
- :white_check_mark: <b>Verify that:</b> a written file can be taken out of the environment via download
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> you can see the files written to the `/mnt/output` storage volume.
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: <b>Verify that:</b> a written file can be taken out of the environment via download

### Software package repositories

#### Tier 2
#### Tier 2:

- :white_check_mark: Can install any packages
- <summary> <b>Verify that:</b> pytz can be installed</summary>
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Can install any packages
- <summary> <b>Verify that:</b> pytz can be installed</summary>
<img src=""/>
- <summary> <b>Verify that:</b> awscli can be installed</summary>
- <summary> <b>Verify that:</b> awscli can be installed</summary>
<img src=""/>

#### Tier 3
#### Tier 3:

- :white_check_mark: Can install only allow-listed packages
- <summary> <b>Verify:</b> pytz can be installed</summary>
- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Can install only allow-listed packages
- <summary> <b>Verify:</b> pytz can be installed</summary>
<img src=""/>
- <summary> <b>Verify:</b> awscli cannot be installed</summary>
- <summary> <b>Verify:</b> awscli cannot be installed</summary>
<img src=""/>
13 changes: 13 additions & 0 deletions tests/commands/conftest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from pytest import fixture
from typer.testing import CliRunner

from data_safe_haven.administration.users.entra_users import EntraUsers
from data_safe_haven.administration.users.research_user import ResearchUser
from data_safe_haven.config import (
Context,
ContextManager,
Expand Down Expand Up @@ -260,3 +262,14 @@ def tmp_contexts_none(tmp_path, context_yaml):
with open(config_file_path, "w") as f:
f.write(context_yaml)
return tmp_path


@fixture
def mock_entra_user_list(mocker):
test_user = ResearchUser(
given_name="Harry",
surname="Lime",
sam_account_name="harry.lime",
user_principal_name="[email protected]",
)
mocker.patch.object(EntraUsers, "list", return_value=[test_user])
20 changes: 20 additions & 0 deletions tests/commands/test_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,26 @@ def test_invalid_shm(
assert result.exit_code == 1
assert "Have you deployed the SHM?" in result.stdout

def test_mismatched_domain(
self,
mock_graphapi_get_credential, # noqa: ARG002
mock_pulumi_config_no_key_from_remote, # noqa: ARG002
mock_shm_config_from_remote, # noqa: ARG002
mock_sre_config_from_remote, # noqa: ARG002
mock_entra_user_list, # noqa: ARG002
runner,
tmp_contexts, # noqa: ARG002
):
result = runner.invoke(
users_command_group, ["register", "-u", "harry.lime", "sandbox"]
)

assert result.exit_code == 0
assert (
"principal domain name must match the domain of the SRE to be registered"
in result.stdout
)

def test_invalid_sre(
self,
mock_pulumi_config_from_remote, # noqa: ARG002
Expand Down

0 comments on commit 8ab3117

Please sign in to comment.