Skip to content

Commit

Permalink
Use 400 for sensitive files permissions on creation with O_EXCL flag (#…
Browse files Browse the repository at this point in the history
…208)

* Use 400 for sensitive files permissions on creation with O_EXCL flag

* Only open with perms on posix OSes
  • Loading branch information
remyroy authored Oct 19, 2024
1 parent 1d23f41 commit 2867c87
Show file tree
Hide file tree
Showing 11 changed files with 58 additions and 40 deletions.
7 changes: 4 additions & 3 deletions ethstaker_deposit/bls_to_execution_change_keystore.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
compute_signing_root,
compute_bls_to_execution_change_keystore_domain,
)
from ethstaker_deposit.utils.file_handling import (
sensitive_opener,
)


def bls_to_execution_change_keystore_generation(
Expand Down Expand Up @@ -64,8 +67,6 @@ def export_bls_to_execution_change_keystore_json(folder: str,
'bls_to_execution_change_keystore_signature-%s-%i.json' % (index, timestamp)
)

with open(filefolder, 'w') as f:
with open(filefolder, 'w', opener=sensitive_opener) as f:
json.dump(signed_bls_to_execution_change_keystore_json, f)
if os.name == 'posix':
os.chmod(filefolder, int('440', 8)) # Read for owner & group
return filefolder
7 changes: 4 additions & 3 deletions ethstaker_deposit/credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@
DepositMessage,
SignedBLSToExecutionChange,
)
from ethstaker_deposit.utils.file_handling import (
sensitive_opener,
)


class WithdrawalType(Enum):
Expand Down Expand Up @@ -364,8 +367,6 @@ def export_bls_to_execution_change_json(self, folder: str, validator_indices: Se
bar.update(1)

filefolder = os.path.join(folder, 'bls_to_execution_change-%i.json' % time.time())
with open(filefolder, 'w') as f:
with open(filefolder, 'w', opener=sensitive_opener) as f:
json.dump(bls_to_execution_changes, f)
if os.name == 'posix':
os.chmod(filefolder, int('440', 8)) # Read for owner & group
return filefolder
9 changes: 5 additions & 4 deletions ethstaker_deposit/key_handling/keystore.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
field as dataclass_field
)
import json
import os

from py_ecc.bls import G2ProofOfPossession as bls
from secrets import randbits
from typing import Any, Dict, Union
Expand All @@ -21,6 +21,9 @@
from ethstaker_deposit.utils.constants import (
UNICODE_CONTROL_CHARS,
)
from ethstaker_deposit.utils.file_handling import (
sensitive_opener,
)

hexdigits = set('0123456789abcdef')

Expand Down Expand Up @@ -96,10 +99,8 @@ def save(self, filefolder: str) -> None:
"""
Save self as a JSON keystore.
"""
with open(filefolder, 'w') as f:
with open(filefolder, 'w', opener=sensitive_opener) as f:
f.write(self.as_json())
if os.name == 'posix':
os.chmod(filefolder, int('440', 8)) # Read for owner & group

@classmethod
def from_json(cls, json_dict: Dict[Any, Any]) -> 'Keystore':
Expand Down
8 changes: 5 additions & 3 deletions ethstaker_deposit/utils/deposit.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import json
import os

from ethstaker_deposit.utils.file_handling import (
sensitive_opener,
)


def export_deposit_data_json(folder: str, timestamp: float, deposit_data: list[dict[str, bytes]]) -> str:
file_folder = os.path.join(folder, 'deposit_data-%i.json' % timestamp)
with open(file_folder, 'w') as f:
with open(file_folder, 'w', opener=sensitive_opener) as f:
json.dump(deposit_data, f, default=lambda x: x.hex())
if os.name == 'posix':
os.chmod(file_folder, int('440', 8)) # Read for owner & group
return file_folder
7 changes: 4 additions & 3 deletions ethstaker_deposit/utils/exit_transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
compute_signing_root,
compute_voluntary_exit_domain,
)
from ethstaker_deposit.utils.file_handling import (
sensitive_opener,
)


def exit_transaction_generation(
Expand Down Expand Up @@ -55,8 +58,6 @@ def export_exit_transaction_json(folder: str, signed_exit: SignedVoluntaryExit,
)
)

with open(filefolder, 'w') as f:
with open(filefolder, 'w', opener=sensitive_opener) as f:
json.dump(signed_exit_json, f)
if os.name == 'posix':
os.chmod(filefolder, int('440', 8)) # Read for owner & group
return filefolder
12 changes: 12 additions & 0 deletions ethstaker_deposit/utils/file_handling.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os
import sys
from typing import Any, Union


def resource_path(relative_path: str) -> str:
Expand All @@ -13,3 +14,14 @@ def resource_path(relative_path: str) -> str:
except Exception:
base_path = os.path.abspath(".")
return os.path.join(base_path, relative_path)


def sensitive_opener(path: Union[str, bytes, 'os.PathLike[Any]'], flags: int) -> int:
"""
Opener to be used with the open built-in function to correctly assign permissions to sensitive
files when created and written to for the first time.
"""
if os.name == 'posix':
return os.open(path, flags | os.O_EXCL, 0o400)
else:
return os.open(path, flags)
2 changes: 1 addition & 1 deletion tests/test_cli/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def get_permissions(path: str, file_name: str) -> str:
def verify_file_permission(os_ref, folder_path, files):
if os_ref.name == 'posix':
for file_name in files:
assert get_permissions(folder_path, file_name) == '0o440'
assert get_permissions(folder_path, file_name) == '0o400'


def prepare_testing_folder(os_ref, testing_folder_name='TESTING_TEMP_FOLDER'):
Expand Down
16 changes: 8 additions & 8 deletions tests/test_cli/test_existing_mnemonic.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def test_existing_mnemonic_bls_withdrawal() -> None:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'
# Clean up
clean_key_folder(my_folder_path)

Expand Down Expand Up @@ -104,7 +104,7 @@ def test_existing_mnemonic_withdrawal_address() -> None:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'
# Clean up
clean_key_folder(my_folder_path)

Expand Down Expand Up @@ -163,7 +163,7 @@ def test_existing_mnemonic_withdrawal_address_bad_checksum() -> None:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'
# Clean up
clean_key_folder(my_folder_path)

Expand Down Expand Up @@ -289,7 +289,7 @@ async def test_script() -> None:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'

# Clean up
clean_key_folder(my_folder_path)
Expand Down Expand Up @@ -338,7 +338,7 @@ async def test_script_abbreviated_mnemonic() -> None:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'

# Clean up
clean_key_folder(my_folder_path)
Expand Down Expand Up @@ -393,7 +393,7 @@ def test_existing_mnemonic_custom_testnet() -> None:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'
# Clean up
clean_key_folder(my_folder_path)

Expand Down Expand Up @@ -438,7 +438,7 @@ def test_existing_mnemonic_multiple_languages() -> None:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'
# Clean up
clean_key_folder(my_folder_path)

Expand Down Expand Up @@ -484,6 +484,6 @@ def test_existing_mnemonic_multiple_languages_argument() -> None:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'
# Clean up
clean_key_folder(my_folder_path)
18 changes: 9 additions & 9 deletions tests/test_cli/test_new_mnemonic.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def mock_get_mnemonic(language, words_path, entropy=None) -> str:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'

# Clean up
clean_key_folder(my_folder_path)
Expand Down Expand Up @@ -125,7 +125,7 @@ def mock_get_mnemonic(language, words_path, entropy=None) -> str:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'

# Clean up
clean_key_folder(my_folder_path)
Expand Down Expand Up @@ -186,7 +186,7 @@ def mock_get_mnemonic(language, words_path, entropy=None) -> str:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'

# Clean up
clean_key_folder(my_folder_path)
Expand Down Expand Up @@ -243,7 +243,7 @@ def mock_get_mnemonic(language, words_path, entropy=None) -> str:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'

# Clean up
clean_key_folder(my_folder_path)
Expand Down Expand Up @@ -301,7 +301,7 @@ def mock_get_mnemonic(language, words_path, entropy=None) -> str:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'

# Clean up
clean_key_folder(my_folder_path)
Expand Down Expand Up @@ -359,7 +359,7 @@ def mock_get_mnemonic(language, words_path, entropy=None) -> str:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'

# Clean up
clean_key_folder(my_folder_path)
Expand Down Expand Up @@ -530,7 +530,7 @@ async def test_script_bls_withdrawal() -> None:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'

# Clean up
clean_key_folder(my_folder_path)
Expand Down Expand Up @@ -609,7 +609,7 @@ async def test_script_abbreviated_mnemonic() -> None:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'

# Clean up
clean_key_folder(my_folder_path)
Expand Down Expand Up @@ -665,7 +665,7 @@ def mock_get_mnemonic(language, words_path, entropy=None) -> str:
# Verify file permissions
if os.name == 'posix':
for file_name in key_files:
assert get_permissions(validator_keys_folder_path, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path, file_name) == '0o400'

# Clean up
clean_key_folder(my_folder_path)
8 changes: 4 additions & 4 deletions tests/test_cli/test_partial_deposit.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def test_partial_deposit(amount: str) -> None:
assert len(deposit_files) == 1

if os.name == 'posix':
assert get_permissions(partial_deposit_folder, deposit_files[0]) == '0o440'
assert get_permissions(partial_deposit_folder, deposit_files[0]) == '0o400'

clean_partial_deposit_folder(my_folder_path)

Expand Down Expand Up @@ -137,7 +137,7 @@ def test_partial_deposit_matches_existing_mnemonic_deposit() -> None:
assert deposit_contents == partial_deposit_contents

if os.name == 'posix':
assert get_permissions(partial_deposit_folder, partial_deposit_files[0]) == '0o440'
assert get_permissions(partial_deposit_folder, partial_deposit_files[0]) == '0o400'

clean_folder(my_folder_path, validator_key_folder, True)
clean_partial_deposit_folder(my_folder_path)
Expand Down Expand Up @@ -218,7 +218,7 @@ def test_partial_deposit_does_not_match_if_amount_differs() -> None:
assert deposit_contents_dict["deposit_cli_version"] == partial_deposit_contents_dict["deposit_cli_version"]

if os.name == 'posix':
assert get_permissions(partial_deposit_folder, partial_deposit_files[0]) == '0o440'
assert get_permissions(partial_deposit_folder, partial_deposit_files[0]) == '0o400'

clean_folder(my_folder_path, validator_key_folder, True)
clean_partial_deposit_folder(my_folder_path)
Expand Down Expand Up @@ -281,7 +281,7 @@ def test_partial_deposit_custom_network(amount: str) -> None:
assert len(deposit_files) == 1

if os.name == 'posix':
assert get_permissions(partial_deposit_folder, deposit_files[0]) == '0o440'
assert get_permissions(partial_deposit_folder, deposit_files[0]) == '0o400'

clean_partial_deposit_folder(my_folder_path)

Expand Down
4 changes: 2 additions & 2 deletions tests/test_cli/test_regeneration.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def mock_get_mnemonic(language, words_path, entropy=None) -> str:
# Verify file permissions
if os.name == 'posix':
for file_name in part_1_key_files:
assert get_permissions(validator_keys_folder_path_1, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path_1, file_name) == '0o400'

# Part 2: existing-mnemonic
runner = CliRunner()
Expand Down Expand Up @@ -98,7 +98,7 @@ def mock_get_mnemonic(language, words_path, entropy=None) -> str:
# Verify file permissions
if os.name == 'posix':
for file_name in part_2_key_files:
assert get_permissions(validator_keys_folder_path_2, file_name) == '0o440'
assert get_permissions(validator_keys_folder_path_2, file_name) == '0o400'

# Clean up
clean_key_folder(folder_path_1)
Expand Down

0 comments on commit 2867c87

Please sign in to comment.