From f77e5536c406cebdbb3ebd1e7e5b66e5cc4eefc3 Mon Sep 17 00:00:00 2001 From: ronantakizawa Date: Thu, 8 Aug 2024 13:09:00 -0400 Subject: [PATCH 1/6] feat: remove 2048-bit restriction for key sizes --- taf/api/yubikey.py | 6 ++++-- taf/keys.py | 16 ++++++++++++---- taf/tools/yubikey/__init__.py | 10 ++++++---- taf/yubikey.py | 10 +++++++--- 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/taf/api/yubikey.py b/taf/api/yubikey.py index 8addc88e..0d80a0a9 100644 --- a/taf/api/yubikey.py +++ b/taf/api/yubikey.py @@ -121,7 +121,9 @@ def get_yk_roles(path: str) -> Dict: on_exceptions=TAFError, reraise=True, ) -def setup_signing_yubikey(certs_dir: Optional[str] = None) -> None: +def setup_signing_yubikey( + certs_dir: Optional[str] = None, key_size: int = 2048 +) -> None: """ Delete everything from the inserted YubiKey, generate a new key and copy it to the YubiKey. Optionally export and save the certificate to a file. @@ -146,7 +148,7 @@ def setup_signing_yubikey(certs_dir: Optional[str] = None) -> None: pin_repeat=True, prompt_message="Please insert the new Yubikey and press ENTER", ) - key = yk.setup_new_yubikey(serial_num) + key = yk.setup_new_yubikey(serial_num, key_size=key_size) yk.export_yk_certificate(certs_dir, key) diff --git a/taf/keys.py b/taf/keys.py index e56cf4e7..27164930 100644 --- a/taf/keys.py +++ b/taf/keys.py @@ -330,6 +330,7 @@ def setup_roles_keys( yubikeys: Optional[Dict] = None, users_yubikeys_details: Optional[Dict[str, UserKeyData]] = None, skip_prompt: Optional[bool] = False, + key_size: int = 2048, ): if role.name is None: @@ -347,7 +348,7 @@ def setup_roles_keys( if is_yubikey: yubikey_keys = _setup_yubikey_roles_keys( - yubikey_ids, users_yubikeys_details, yubikeys, role, certs_dir + yubikey_ids, users_yubikeys_details, yubikeys, role, certs_dir, key_size ) else: if keystore is None: @@ -374,7 +375,7 @@ def setup_roles_keys( def _setup_yubikey_roles_keys( - yubikey_ids, users_yubikeys_details, yubikeys, role, certs_dir + yubikey_ids, users_yubikeys_details, yubikeys, role, certs_dir, key_size ): loaded_keys_num = 0 yk_with_public_key = {} @@ -397,7 +398,13 @@ def _setup_yubikey_roles_keys( key_scheme = users_yubikeys_details[key_id].scheme key_scheme = key_scheme or role.scheme public_key = _setup_yubikey( - yubikeys, role.name, key_id, yubikey_keys, key_scheme, certs_dir + yubikeys, + role.name, + key_id, + yubikey_keys, + key_scheme, + certs_dir, + key_size, ) loaded_keys_num += 1 yubikey_keys.append(public_key) @@ -526,6 +533,7 @@ def _setup_yubikey( loaded_keys: List[str], scheme: Optional[str] = DEFAULT_RSA_SIGNATURE_SCHEME, certs_dir: Optional[Union[Path, str]] = None, + key_size: int = 2048, ) -> Dict: print(f"Registering keys for {key_name}") while True: @@ -551,7 +559,7 @@ def _setup_yubikey( print("Key already loaded. Please insert a different YubiKey") else: if not use_existing: - key = yk.setup_new_yubikey(serial_num, scheme) + key = yk.setup_new_yubikey(serial_num, scheme, key_size=key_size) if certs_dir is not None: yk.export_yk_certificate(certs_dir, key) diff --git a/taf/tools/yubikey/__init__.py b/taf/tools/yubikey/__init__.py index 68dd8848..3fc53aa8 100644 --- a/taf/tools/yubikey/__init__.py +++ b/taf/tools/yubikey/__init__.py @@ -61,8 +61,9 @@ def setup_signing_key_command(): to the specified directory. WARNING - this will delete everything from the inserted key.""") @click.option("--certs-dir", help="Path of the directory where the exported certificate will be saved. Set to the user home directory by default") - def setup_signing_key(certs_dir): - setup_signing_yubikey(certs_dir) + @click.option("--key-size", default=2048, type=int, help="Size of the RSA key to be generated (Currently supports 1024 and 2048-bits)") + def setup_signing_key(certs_dir, key_size): + setup_signing_yubikey(certs_dir, key_size) return setup_signing_key @@ -70,8 +71,9 @@ def setup_test_key_command(): @click.command(help="""Copies the specified key onto the inserted YubiKey WARNING - this will reset the inserted key.""") @click.argument("key-path") - def setup_test_key(key_path): - setup_test_yubikey(key_path) + @click.option("--key-size", default=2048, type=int, help="Size of the RSA key to be generated (Currently supports 1024 and 2048-bits)") + def setup_test_key(key_path, key_size): + setup_test_yubikey(key_path, key_size) return setup_test_key diff --git a/taf/yubikey.py b/taf/yubikey.py index 1f365009..7f69423b 100644 --- a/taf/yubikey.py +++ b/taf/yubikey.py @@ -302,6 +302,7 @@ def setup( pin_retries=10, private_key_pem=None, mgm_key=generate_random_management_key(MANAGEMENT_KEY_TYPE.TDES), + key_size=2048, ): """Use to setup inserted Yubikey, with following steps (order is important): - reset to factory settings @@ -326,6 +327,7 @@ def setup( Raises: - YubikeyError """ + with _yk_piv_ctrl() as (ctrl, _): # Factory reset and set PINs ctrl.reset() @@ -335,7 +337,7 @@ def setup( # Generate RSA2048 if private_key_pem is None: - private_key = rsa.generate_private_key(65537, 2048, default_backend()) + private_key = rsa.generate_private_key(65537, key_size, default_backend()) pub_key = private_key.public_key() else: try: @@ -382,11 +384,13 @@ def setup( ) -def setup_new_yubikey(serial_num, scheme=DEFAULT_RSA_SIGNATURE_SCHEME): +def setup_new_yubikey(serial_num, scheme=DEFAULT_RSA_SIGNATURE_SCHEME, key_size=2048): pin = get_key_pin(serial_num) cert_cn = input("Enter key holder's name: ") print("Generating key, please wait...") - pub_key_pem = setup(pin, cert_cn, cert_exp_days=EXPIRATION_INTERVAL).decode("utf-8") + pub_key_pem = setup( + pin, cert_cn, cert_exp_days=EXPIRATION_INTERVAL, key_size=key_size + ).decode("utf-8") scheme = DEFAULT_RSA_SIGNATURE_SCHEME key = import_rsakey_from_pem(pub_key_pem, scheme) return key From 0c3ca8f95ef763c9786a16d41139b028de4fdd34 Mon Sep 17 00:00:00 2001 From: ronantakizawa Date: Thu, 8 Aug 2024 18:02:47 -0400 Subject: [PATCH 2/6] fix: fix previous bugs --- CHANGELOG.md | 3 +- taf/api/conf.py | 2 +- taf/api/roles.py | 8 +++-- taf/tools/metadata/__init__.py | 8 ++--- taf/tools/yubikey/__init__.py | 2 -- taf/yubikey.py | 64 +++++++++++++++++----------------- 6 files changed, 43 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e24c0cc..85b4a5d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning][semver]. ### Added - +- Removed 2048-bit key restriction [494] - Added lazy loading to CLI [481] - Testing repositories with dependencies ([479], [487]) - Hid plaintext when users are prompted to insert YubiKey and press ENTER [473] @@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning][semver]. ### Fixed +[494]: https://github.com/openlawlibrary/taf/pull/494 [487]: https://github.com/openlawlibrary/taf/pull/489 [487]: https://github.com/openlawlibrary/taf/pull/487 [487]: https://github.com/openlawlibrary/taf/pull/485 diff --git a/taf/api/conf.py b/taf/api/conf.py index 0820c061..9b3b7dd6 100644 --- a/taf/api/conf.py +++ b/taf/api/conf.py @@ -86,7 +86,7 @@ def init( if not roles_key_infos: roles_key_infos = "." if roles_key_infos: - generate_keys(taf_directory, str(keystore_directory), roles_key_infos) + generate_keys(taf_directory, keystore_directory, roles_key_infos) taf_logger.info( f"Successfully generated keys inside the {keystore_directory} directory" ) diff --git a/taf/api/roles.py b/taf/api/roles.py index c6ca289e..28da4854 100644 --- a/taf/api/roles.py +++ b/taf/api/roles.py @@ -326,6 +326,7 @@ def add_signing_key( push: Optional[bool] = True, commit_msg: Optional[str] = None, ) -> None: + print("lol") """ Add a new signing key to the listed roles. Update root metadata if one or more roles is one of the main TUF roles, parent target role if one of the roles is a delegated target role and timestamp and snapshot in any case. @@ -498,12 +499,12 @@ def _enter_role_info( while click.confirm( f"Add {'another' if len(delegated_roles) else 'a'} delegated targets role of role {role}?" ): - role_name = _read_val(str, "role name", True) + role_name = _read_val(str, "role name", "role_name", True) delegated_paths: List[str] = [] while not len(delegated_paths) or click.confirm("Enter another path?"): delegated_paths.append( _read_val( - str, f"path or glob pattern delegated to {role_name}", True + str, f"path or glob pattern delegated to {role_name}", "delegated_paths", True ) ) delegated_roles[role_name]["paths"] = delegated_paths @@ -521,6 +522,8 @@ def _read_val(input_type, name, param=None, required=False): default_value_msg = "" default_value = None if param is not None: + if not isinstance(param, str): + raise TypeError(f"Parameter 'param' must be a string, got {type(param).__name__}") default_value = DEFAULT_ROLE_SETUP_PARAMS[param] if default_value is not None: default_value_msg = f"(default {default_value}) " @@ -602,6 +605,7 @@ def _initialize_roles_and_keystore( ) if keystore is not None: + print(keystore) keystore = resolve_keystore_path(keystore, roles_key_infos) roles_key_infos_dict["keystore"] = keystore diff --git a/taf/tools/metadata/__init__.py b/taf/tools/metadata/__init__.py index ae3b1c2d..9db16f2b 100644 --- a/taf/tools/metadata/__init__.py +++ b/taf/tools/metadata/__init__.py @@ -77,9 +77,5 @@ def update_expiration_dates(path, role, interval, keystore, scheme, start_date, def attach_to_group(group): - metadata_group = click.Group(name='metadata') - - metadata_group.add_command(check_expiration_dates_command(), name='check-expiration-dates') - metadata_group.add_command(update_expiration_dates_command(), name='update-expiration-dates') - - group.add_command(metadata_group) + group.add_command(check_expiration_dates_command(), name='check-expiration-dates') + group.add_command(update_expiration_dates_command(), name='update-expiration-dates') diff --git a/taf/tools/yubikey/__init__.py b/taf/tools/yubikey/__init__.py index 3fc53aa8..a7d63629 100644 --- a/taf/tools/yubikey/__init__.py +++ b/taf/tools/yubikey/__init__.py @@ -61,7 +61,6 @@ def setup_signing_key_command(): to the specified directory. WARNING - this will delete everything from the inserted key.""") @click.option("--certs-dir", help="Path of the directory where the exported certificate will be saved. Set to the user home directory by default") - @click.option("--key-size", default=2048, type=int, help="Size of the RSA key to be generated (Currently supports 1024 and 2048-bits)") def setup_signing_key(certs_dir, key_size): setup_signing_yubikey(certs_dir, key_size) return setup_signing_key @@ -71,7 +70,6 @@ def setup_test_key_command(): @click.command(help="""Copies the specified key onto the inserted YubiKey WARNING - this will reset the inserted key.""") @click.argument("key-path") - @click.option("--key-size", default=2048, type=int, help="Size of the RSA key to be generated (Currently supports 1024 and 2048-bits)") def setup_test_key(key_path, key_size): setup_test_yubikey(key_path, key_size) return setup_test_key diff --git a/taf/yubikey.py b/taf/yubikey.py index 7f69423b..ba763b06 100644 --- a/taf/yubikey.py +++ b/taf/yubikey.py @@ -230,11 +230,15 @@ def export_piv_pub_key(pub_key_format=serialization.Encoding.PEM, pub_key_pem=No - YubikeyError """ with _yk_piv_ctrl(pub_key_pem=pub_key_pem) as (ctrl, _): - x509 = ctrl.get_certificate(SLOT.SIGNATURE) - return x509.public_key().public_bytes( - encoding=pub_key_format, - format=serialization.PublicFormat.SubjectPublicKeyInfo, - ) + try: + x509_cert = ctrl.get_certificate(SLOT.SIGNATURE) + public_key = x509_cert.public_key() + return public_key.public_bytes( + encoding=pub_key_format, + format=serialization.PublicFormat.SubjectPublicKeyInfo, + ) + except Exception as e: + raise YubikeyError(f"Failed to export public key: {str(e)}") from e @raise_yubikey_err("Cannot export yk certificate.") @@ -425,44 +429,40 @@ def yubikey_prompt( hide_already_loaded_message=False, ): def _read_and_check_yubikey( - key_name, - role, - taf_repo, - registering_new_key, - creating_new_key, - loaded_yubikeys, - pin_confirm, - pin_repeat, - prompt_message, - retrying, - ): - + key_name, + role, + taf_repo, + registering_new_key, + creating_new_key, + loaded_yubikeys, + pin_confirm, + pin_repeat, + prompt_message, + retrying, +): if retrying: if prompt_message is None: prompt_message = f"Please insert {key_name} YubiKey and press ENTER" getpass(prompt_message) - # make sure that YubiKey is inserted + + # Ensure YubiKey is inserted try: serial_num = get_serial_num() except Exception: print("YubiKey not inserted") return False, None, None - # check if this key is already loaded as the provided role's key (we can use the same key - # to sign different metadata) - if ( - loaded_yubikeys is not None - and serial_num in loaded_yubikeys - and role in loaded_yubikeys[serial_num] - ): - if not hide_already_loaded_message: - print("Key already loaded") - return False, None, None + # Read the public key unless a new key needs to be generated + if not creating_new_key: + try: + public_key = get_piv_public_key_tuf() + except YubikeyError as e: + print(f"Error retrieving public key: {str(e)}") + return False, None, None + else: + public_key = None - # read the public key, unless a new key needs to be generated on the yubikey - public_key = get_piv_public_key_tuf() if not creating_new_key else None - # check if this yubikey is can be used for signing the provided role's metadata - # if the key was already registered as that role's key + # Check if the YubiKey can be used for signing the specified role's metadata if not registering_new_key and role is not None and taf_repo is not None: if not taf_repo.is_valid_metadata_yubikey(role, public_key): print(f"The inserted YubiKey is not a valid {role} key") From 984c9f71569311c24ddb32f7e0eb76f013d187ba Mon Sep 17 00:00:00 2001 From: ronantakizawa Date: Thu, 8 Aug 2024 18:04:12 -0400 Subject: [PATCH 3/6] fix: removed debugging logs --- taf/api/roles.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/taf/api/roles.py b/taf/api/roles.py index 28da4854..943d7692 100644 --- a/taf/api/roles.py +++ b/taf/api/roles.py @@ -326,7 +326,6 @@ def add_signing_key( push: Optional[bool] = True, commit_msg: Optional[str] = None, ) -> None: - print("lol") """ Add a new signing key to the listed roles. Update root metadata if one or more roles is one of the main TUF roles, parent target role if one of the roles is a delegated target role and timestamp and snapshot in any case. @@ -605,7 +604,6 @@ def _initialize_roles_and_keystore( ) if keystore is not None: - print(keystore) keystore = resolve_keystore_path(keystore, roles_key_infos) roles_key_infos_dict["keystore"] = keystore From 0f1ddc28c2097f90974711510415321563f1fc18 Mon Sep 17 00:00:00 2001 From: ronantakizawa Date: Thu, 8 Aug 2024 18:26:14 -0400 Subject: [PATCH 4/6] fix: fix linting issues --- taf/api/conf.py | 2 +- taf/api/roles.py | 9 +++++++-- taf/yubikey.py | 50 ++++++++++++++++++++++++++---------------------- 3 files changed, 35 insertions(+), 26 deletions(-) diff --git a/taf/api/conf.py b/taf/api/conf.py index 9b3b7dd6..0820c061 100644 --- a/taf/api/conf.py +++ b/taf/api/conf.py @@ -86,7 +86,7 @@ def init( if not roles_key_infos: roles_key_infos = "." if roles_key_infos: - generate_keys(taf_directory, keystore_directory, roles_key_infos) + generate_keys(taf_directory, str(keystore_directory), roles_key_infos) taf_logger.info( f"Successfully generated keys inside the {keystore_directory} directory" ) diff --git a/taf/api/roles.py b/taf/api/roles.py index 943d7692..06c24e49 100644 --- a/taf/api/roles.py +++ b/taf/api/roles.py @@ -503,7 +503,10 @@ def _enter_role_info( while not len(delegated_paths) or click.confirm("Enter another path?"): delegated_paths.append( _read_val( - str, f"path or glob pattern delegated to {role_name}", "delegated_paths", True + str, + f"path or glob pattern delegated to {role_name}", + "delegated_paths", + True, ) ) delegated_roles[role_name]["paths"] = delegated_paths @@ -522,7 +525,9 @@ def _read_val(input_type, name, param=None, required=False): default_value = None if param is not None: if not isinstance(param, str): - raise TypeError(f"Parameter 'param' must be a string, got {type(param).__name__}") + raise TypeError( + f"Parameter 'param' must be a string, got {type(param).__name__}" + ) default_value = DEFAULT_ROLE_SETUP_PARAMS[param] if default_value is not None: default_value_msg = f"(default {default_value}) " diff --git a/taf/yubikey.py b/taf/yubikey.py index ba763b06..1cac0212 100644 --- a/taf/yubikey.py +++ b/taf/yubikey.py @@ -429,40 +429,44 @@ def yubikey_prompt( hide_already_loaded_message=False, ): def _read_and_check_yubikey( - key_name, - role, - taf_repo, - registering_new_key, - creating_new_key, - loaded_yubikeys, - pin_confirm, - pin_repeat, - prompt_message, - retrying, -): + key_name, + role, + taf_repo, + registering_new_key, + creating_new_key, + loaded_yubikeys, + pin_confirm, + pin_repeat, + prompt_message, + retrying, + ): + if retrying: if prompt_message is None: prompt_message = f"Please insert {key_name} YubiKey and press ENTER" getpass(prompt_message) - - # Ensure YubiKey is inserted + # make sure that YubiKey is inserted try: serial_num = get_serial_num() except Exception: print("YubiKey not inserted") return False, None, None - # Read the public key unless a new key needs to be generated - if not creating_new_key: - try: - public_key = get_piv_public_key_tuf() - except YubikeyError as e: - print(f"Error retrieving public key: {str(e)}") - return False, None, None - else: - public_key = None + # check if this key is already loaded as the provided role's key (we can use the same key + # to sign different metadata) + if ( + loaded_yubikeys is not None + and serial_num in loaded_yubikeys + and role in loaded_yubikeys[serial_num] + ): + if not hide_already_loaded_message: + print("Key already loaded") + return False, None, None - # Check if the YubiKey can be used for signing the specified role's metadata + # read the public key, unless a new key needs to be generated on the yubikey + public_key = get_piv_public_key_tuf() if not creating_new_key else None + # check if this yubikey is can be used for signing the provided role's metadata + # if the key was already registered as that role's key if not registering_new_key and role is not None and taf_repo is not None: if not taf_repo.is_valid_metadata_yubikey(role, public_key): print(f"The inserted YubiKey is not a valid {role} key") From 0441cd1f577332e9f9620bf936610236f4fa8455 Mon Sep 17 00:00:00 2001 From: ronantakizawa Date: Thu, 8 Aug 2024 18:55:14 -0400 Subject: [PATCH 5/6] fix: fix key parameter --- taf/tools/yubikey/__init__.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/taf/tools/yubikey/__init__.py b/taf/tools/yubikey/__init__.py index a7d63629..90bc0e0d 100644 --- a/taf/tools/yubikey/__init__.py +++ b/taf/tools/yubikey/__init__.py @@ -61,8 +61,8 @@ def setup_signing_key_command(): to the specified directory. WARNING - this will delete everything from the inserted key.""") @click.option("--certs-dir", help="Path of the directory where the exported certificate will be saved. Set to the user home directory by default") - def setup_signing_key(certs_dir, key_size): - setup_signing_yubikey(certs_dir, key_size) + def setup_signing_key(certs_dir): + setup_signing_yubikey(certs_dir,key_size=2048) return setup_signing_key @@ -70,13 +70,12 @@ def setup_test_key_command(): @click.command(help="""Copies the specified key onto the inserted YubiKey WARNING - this will reset the inserted key.""") @click.argument("key-path") - def setup_test_key(key_path, key_size): - setup_test_yubikey(key_path, key_size) + def setup_test_key(key_path): + setup_test_yubikey(key_path,key_size=2048) return setup_test_key def attach_to_group(group): - group.add_command(check_pin_command(), name='check-pin') group.add_command(export_pub_key_command(), name='export-pub-key') group.add_command(get_roles_command(), name='get-roles') From bb4d2ca43304c7a5014cee6ac439b508d93bc1f6 Mon Sep 17 00:00:00 2001 From: ronantakizawa Date: Tue, 20 Aug 2024 16:39:21 -0400 Subject: [PATCH 6/6] fix: fix unnecessary check in _read_val --- taf/api/roles.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/taf/api/roles.py b/taf/api/roles.py index 06c24e49..f1d3cbd1 100644 --- a/taf/api/roles.py +++ b/taf/api/roles.py @@ -524,10 +524,6 @@ def _read_val(input_type, name, param=None, required=False): default_value_msg = "" default_value = None if param is not None: - if not isinstance(param, str): - raise TypeError( - f"Parameter 'param' must be a string, got {type(param).__name__}" - ) default_value = DEFAULT_ROLE_SETUP_PARAMS[param] if default_value is not None: default_value_msg = f"(default {default_value}) "