From c37e330efd35af20cdba267ff4a82d3df4d19efb Mon Sep 17 00:00:00 2001 From: Chris Wood Date: Fri, 27 Jan 2023 10:02:59 -0500 Subject: [PATCH 01/15] Add charset_normalizer.md__mypyc to hidden imports Fixes: https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3966 Overrides: https://github.com/tahoe-lafs/tahoe-lafs/pull/1248 Ref.: https://github.com/pyinstaller/pyinstaller-hooks-contrib/issues/534 --- pyinstaller.spec | 1 + 1 file changed, 1 insertion(+) diff --git a/pyinstaller.spec b/pyinstaller.spec index eece50757..01b1ac4ac 100644 --- a/pyinstaller.spec +++ b/pyinstaller.spec @@ -36,6 +36,7 @@ hidden_imports = [ 'allmydata.stats', 'base64', 'cffi', + 'charset_normalizer.md__mypyc', 'collections', 'commands', 'Crypto', From 87dad9bd2bf96eedfd671e17fe003932358d7157 Mon Sep 17 00:00:00 2001 From: Chris Wood Date: Fri, 27 Jan 2023 10:07:50 -0500 Subject: [PATCH 02/15] Remove "charset_normalizer < 3" constraint --- setup.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/setup.py b/setup.py index f867e901d..1974145cb 100644 --- a/setup.py +++ b/setup.py @@ -147,13 +147,6 @@ install_requires = [ # for pid-file support "psutil", "filelock", - - # treq needs requests, requests needs charset_normalizer, - # charset_normalizer breaks PyInstaller - # (https://github.com/Ousret/charset_normalizer/issues/253). So work around - # this by using a lower version number. Once upstream issue is fixed, or - # requests drops charset_normalizer, this can go away. - "charset_normalizer < 3", ] setup_requires = [ From a292f52de1aa44e34e9b72ab57d4ac608b6b5da3 Mon Sep 17 00:00:00 2001 From: Chris Wood Date: Fri, 27 Jan 2023 11:47:50 -0500 Subject: [PATCH 03/15] Try debugging CI/ubuntu-20.04 integration tests.. Does restoring the "charset_normalizer < 3" pin make the tests pass? --- setup.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/setup.py b/setup.py index 1974145cb..f867e901d 100644 --- a/setup.py +++ b/setup.py @@ -147,6 +147,13 @@ install_requires = [ # for pid-file support "psutil", "filelock", + + # treq needs requests, requests needs charset_normalizer, + # charset_normalizer breaks PyInstaller + # (https://github.com/Ousret/charset_normalizer/issues/253). So work around + # this by using a lower version number. Once upstream issue is fixed, or + # requests drops charset_normalizer, this can go away. + "charset_normalizer < 3", ] setup_requires = [ From e046627d3178846bdc581ce5f38de07f52f0cdca Mon Sep 17 00:00:00 2001 From: Chris Wood Date: Fri, 27 Jan 2023 11:59:24 -0500 Subject: [PATCH 04/15] Try debugging CI/ubuntu-20.04 integration tests... Does removing the `charset_normalizer.md__mypyc` hidden import make the tests pass? --- pyinstaller.spec | 1 - 1 file changed, 1 deletion(-) diff --git a/pyinstaller.spec b/pyinstaller.spec index 01b1ac4ac..eece50757 100644 --- a/pyinstaller.spec +++ b/pyinstaller.spec @@ -36,7 +36,6 @@ hidden_imports = [ 'allmydata.stats', 'base64', 'cffi', - 'charset_normalizer.md__mypyc', 'collections', 'commands', 'Crypto', From 15c7916e0812e6baa2a931cd54b18f3382a8456e Mon Sep 17 00:00:00 2001 From: Chris Wood Date: Fri, 27 Jan 2023 12:46:30 -0500 Subject: [PATCH 05/15] Revert previous two commits (e046627, a292f52) --- pyinstaller.spec | 1 + setup.py | 7 ------- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/pyinstaller.spec b/pyinstaller.spec index eece50757..01b1ac4ac 100644 --- a/pyinstaller.spec +++ b/pyinstaller.spec @@ -36,6 +36,7 @@ hidden_imports = [ 'allmydata.stats', 'base64', 'cffi', + 'charset_normalizer.md__mypyc', 'collections', 'commands', 'Crypto', diff --git a/setup.py b/setup.py index f867e901d..1974145cb 100644 --- a/setup.py +++ b/setup.py @@ -147,13 +147,6 @@ install_requires = [ # for pid-file support "psutil", "filelock", - - # treq needs requests, requests needs charset_normalizer, - # charset_normalizer breaks PyInstaller - # (https://github.com/Ousret/charset_normalizer/issues/253). So work around - # this by using a lower version number. Once upstream issue is fixed, or - # requests drops charset_normalizer, this can go away. - "charset_normalizer < 3", ] setup_requires = [ From ff964b2310382b92c9d0392fa94705d3b64f2dec Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sat, 28 Jan 2023 08:53:53 -0500 Subject: [PATCH 06/15] news fragment --- newsfragments/3969.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3969.minor diff --git a/newsfragments/3969.minor b/newsfragments/3969.minor new file mode 100644 index 000000000..e69de29bb From 230ce346c578c7a0969661b60f931397d4081c36 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sat, 28 Jan 2023 08:54:00 -0500 Subject: [PATCH 07/15] circleci env var notes --- .circleci/circleci.txt | 78 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 .circleci/circleci.txt diff --git a/.circleci/circleci.txt b/.circleci/circleci.txt new file mode 100644 index 000000000..c7adf9ec1 --- /dev/null +++ b/.circleci/circleci.txt @@ -0,0 +1,78 @@ +# A master build looks like this: + +# BASH_ENV=/tmp/.bash_env-63d018969ca480003a031e62-0-build +# CI=true +# CIRCLECI=true +# CIRCLE_BRANCH=master +# CIRCLE_BUILD_NUM=76545 +# CIRCLE_BUILD_URL=https://circleci.com/gh/tahoe-lafs/tahoe-lafs/76545 +# CIRCLE_JOB=NixOS 21.11 +# CIRCLE_NODE_INDEX=0 +# CIRCLE_NODE_TOTAL=1 +# CIRCLE_PROJECT_REPONAME=tahoe-lafs +# CIRCLE_PROJECT_USERNAME=tahoe-lafs +# CIRCLE_REPOSITORY_URL=git@github.com:tahoe-lafs/tahoe-lafs.git +# CIRCLE_SHA1=ed0bda2d7456f4a2cd60870072e1fe79864a49a1 +# CIRCLE_SHELL_ENV=/tmp/.bash_env-63d018969ca480003a031e62-0-build +# CIRCLE_USERNAME=alice +# CIRCLE_WORKFLOW_ID=6d9bb71c-be3a-4659-bf27-60954180619b +# CIRCLE_WORKFLOW_JOB_ID=0793c975-7b9f-489f-909b-8349b72d2785 +# CIRCLE_WORKFLOW_WORKSPACE_ID=6d9bb71c-be3a-4659-bf27-60954180619b +# CIRCLE_WORKING_DIRECTORY=~/project + +# A build of an in-repo PR looks like this: + +# BASH_ENV=/tmp/.bash_env-63d1971a0298086d8841287e-0-build +# CI=true +# CIRCLECI=true +# CIRCLE_BRANCH=3946-less-chatty-downloads +# CIRCLE_BUILD_NUM=76612 +# CIRCLE_BUILD_URL=https://circleci.com/gh/tahoe-lafs/tahoe-lafs/76612 +# CIRCLE_JOB=NixOS 21.11 +# CIRCLE_NODE_INDEX=0 +# CIRCLE_NODE_TOTAL=1 +# CIRCLE_PROJECT_REPONAME=tahoe-lafs +# CIRCLE_PROJECT_USERNAME=tahoe-lafs +# CIRCLE_PULL_REQUEST=https://github.com/tahoe-lafs/tahoe-lafs/pull/1251 +# CIRCLE_PULL_REQUESTS=https://github.com/tahoe-lafs/tahoe-lafs/pull/1251 +# CIRCLE_REPOSITORY_URL=git@github.com:tahoe-lafs/tahoe-lafs.git +# CIRCLE_SHA1=921a2083dcefdb5f431cdac195fc9ac510605349 +# CIRCLE_SHELL_ENV=/tmp/.bash_env-63d1971a0298086d8841287e-0-build +# CIRCLE_USERNAME=bob +# CIRCLE_WORKFLOW_ID=5e32c12e-be37-4868-9fa8-6a6929fec2f1 +# CIRCLE_WORKFLOW_JOB_ID=316ca408-81b4-4c96-bbdd-644e4c3e01e5 +# CIRCLE_WORKFLOW_WORKSPACE_ID=5e32c12e-be37-4868-9fa8-6a6929fec2f1 +# CIRCLE_WORKING_DIRECTORY=~/project +# CI_PULL_REQUEST=https://github.com/tahoe-lafs/tahoe-lafs/pull/1251 + +# A build of a PR from a fork looks like this: + +# BASH_ENV=/tmp/.bash_env-63d40f7b2e89cd3de10e0db9-0-build +# CI=true +# CIRCLECI=true +# CIRCLE_BRANCH=pull/1252 +# CIRCLE_BUILD_NUM=76678 +# CIRCLE_BUILD_URL=https://circleci.com/gh/tahoe-lafs/tahoe-lafs/76678 +# CIRCLE_JOB=NixOS 21.05 +# CIRCLE_NODE_INDEX=0 +# CIRCLE_NODE_TOTAL=1 +# CIRCLE_PROJECT_REPONAME=tahoe-lafs +# CIRCLE_PROJECT_USERNAME=tahoe-lafs +# CIRCLE_PR_NUMBER=1252 +# CIRCLE_PR_REPONAME=tahoe-lafs +# CIRCLE_PR_USERNAME=carol +# CIRCLE_PULL_REQUEST=https://github.com/tahoe-lafs/tahoe-lafs/pull/1252 +# CIRCLE_PULL_REQUESTS=https://github.com/tahoe-lafs/tahoe-lafs/pull/1252 +# CIRCLE_REPOSITORY_URL=git@github.com:tahoe-lafs/tahoe-lafs.git +# CIRCLE_SHA1=15c7916e0812e6baa2a931cd54b18f3382a8456e +# CIRCLE_SHELL_ENV=/tmp/.bash_env-63d40f7b2e89cd3de10e0db9-0-build +# CIRCLE_USERNAME= +# CIRCLE_WORKFLOW_ID=19c917c8-3a38-4b20-ac10-3265259fa03e +# CIRCLE_WORKFLOW_JOB_ID=58e95215-eccf-4664-a231-1dba7fd2d323 +# CIRCLE_WORKFLOW_WORKSPACE_ID=19c917c8-3a38-4b20-ac10-3265259fa03e +# CIRCLE_WORKING_DIRECTORY=~/project +# CI_PULL_REQUEST=https://github.com/tahoe-lafs/tahoe-lafs/pull/1252 + +# A build of a PR from a fork where the owner has enabled CircleCI looks +# the same as a build of an in-repo PR, except it runs on th owner's +# CircleCI namespace. From 3d58194c3ae73ae2048df952418bbdaaf82c6156 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sat, 28 Jan 2023 08:56:48 -0500 Subject: [PATCH 08/15] Complexify the upstream-vs-forked detection --- .circleci/lib.sh | 95 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 83 insertions(+), 12 deletions(-) diff --git a/.circleci/lib.sh b/.circleci/lib.sh index 7717cdb18..b5c900371 100644 --- a/.circleci/lib.sh +++ b/.circleci/lib.sh @@ -1,21 +1,20 @@ # Run a command, enabling cache writes to cachix if possible. The command is # accepted as a variable number of positional arguments (like argv). function cache_if_able() { - # The `cachix watch-exec ...` does our cache population. When it sees - # something added to the store (I guess) it pushes it to the named cache. - # - # We can only *push* to it if we have a CACHIX_AUTH_TOKEN, though. - # in-repo jobs will get this from CircleCI configuration but jobs from - # forks may not. - echo "Building PR from user/org: ${CIRCLE_PROJECT_USERNAME}" - if [ -v CACHIX_AUTH_TOKEN ]; then + # Dump some info about our build environment. + describe_build + + if is_cache_writeable; then + # If the cache is available we'll use it. This lets fork owners set + # up their own caching if they want. echo "Cachix credentials present; will attempt to write to cache." + + # The `cachix watch-exec ...` does our cache population. When it sees + # something added to the store (I guess) it pushes it to the named + # cache. cachix watch-exec "${CACHIX_NAME}" -- "$@" else - # If we're building a from a forked repository then we're allowed to - # not have the credentials (but it's also fine if the owner of the - # fork supplied their own). - if [ "${CIRCLE_PROJECT_USERNAME}" == "tahoe-lafs" ]; then + if is_cache_required; then echo "Required credentials (CACHIX_AUTH_TOKEN) are missing." return 1 else @@ -24,3 +23,75 @@ function cache_if_able() { fi fi } + +function is_cache_writeable() { + # We can only *push* to the cache if we have a CACHIX_AUTH_TOKEN. in-repo + # jobs will get this from CircleCI configuration but jobs from forks may + # not. + [ -v CACHIX_AUTH_TOKEN ] +} + +function is_cache_required() { + # If we're building in tahoe-lafs/tahoe-lafs then we must use the cache. + # If we're building anything from a fork then we're allowed to not have + # the credentials. + is_upstream +} + +# Return success if the origin of this build is the tahoe-lafs/tahoe-lafs +# repository itself (and so we expect to have cache credentials available), +# failure otherwise. +# +# See circleci.txt for notes about how this determination is made. +function is_upstream() { + # CIRCLE_PROJECT_USERNAME is set to the org the build is happening for. + # If a PR targets a fork of the repo then this is set to something other + # than "tahoe-lafs". + [ "$CIRCLE_PROJECT_USERNAME" == "tahoe-lafs" ] && + + # CIRCLE_BRANCH is set to the real branch name for in-repo PRs and + # "pull/NNNN" for pull requests from forks. + # + # CIRCLE_PULL_REQUEST is set to the full URL of the PR page which ends + # with that same "pull/NNNN" for PRs from forks. + ! endswith "/$CIRCLE_BRANCH" "$CIRCLE_PULL_REQUEST" +} + +# Return success if $2 ends with $1, failure otherwise. +function endswith() { + suffix=$1 + shift + + haystack=$1 + shift + + case "$haystack" in + *${suffix}) + return 0 + ;; + + *) + return 1 + ;; + esac +} + +function describe_build() { + echo "Building PR for user/org: ${CIRCLE_PROJECT_USERNAME}" + echo "Building branch: ${CIRCLE_BRANCH}" + if is_upstream; then + echo "Upstream build." + else + echo "Non-upstream build." + fi + if is_cache_required; then + echo "Cache is required." + else + echo "Cache not required." + fi + if is_cache_writeable; then + echo "Cache is writeable." + else + echo "Cache not writeable." + fi +} From 4ea4286a7f52429addb4dbd1eb0467659fb2b66f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sat, 28 Jan 2023 09:21:34 -0500 Subject: [PATCH 09/15] Use CIRCLE_PULL_REQUESTS in case there are multiple which, of course, there never are, except for during testing of this branch --- .circleci/lib.sh | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/.circleci/lib.sh b/.circleci/lib.sh index b5c900371..c692b5f88 100644 --- a/.circleci/lib.sh +++ b/.circleci/lib.sh @@ -52,9 +52,31 @@ function is_upstream() { # CIRCLE_BRANCH is set to the real branch name for in-repo PRs and # "pull/NNNN" for pull requests from forks. # - # CIRCLE_PULL_REQUEST is set to the full URL of the PR page which ends - # with that same "pull/NNNN" for PRs from forks. - ! endswith "/$CIRCLE_BRANCH" "$CIRCLE_PULL_REQUEST" + # CIRCLE_PULL_REQUESTS is set to a comma-separated list of the full + # URLs of the PR pages which share an underlying branch, with one of + # them ended with that same "pull/NNNN" for PRs from forks. + ! any_element_endswith "/$CIRCLE_BRANCH" "," "$CIRCLE_PULL_REQUESTS" +} + +# Return success if splitting $3 on $2 results in an array with any element +# that ends with $1, failure otherwise. +function any_element_endswith() { + suffix=$1 + shift + + sep=$1 + shift + + haystack=$1 + shift + + IFS="${sep}" read -r -a elements <<< "$haystack" + for elem in "${elements[@]}"; do + if endswith "$suffix" "$elem"; then + return 0 + fi + done + return 1 } # Return success if $2 ends with $1, failure otherwise. From cad81c9bdda0ad5d1bfe066d0ae8fccdcb851615 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sat, 28 Jan 2023 16:21:45 -0500 Subject: [PATCH 10/15] Twiddle the news fragment to pass codechecks --- newsfragments/3966.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newsfragments/3966.bugfix b/newsfragments/3966.bugfix index ead94c47c..384dcf797 100644 --- a/newsfragments/3966.bugfix +++ b/newsfragments/3966.bugfix @@ -1 +1 @@ -Fix incompatibility with newer versions of the transitive charset_normalizer dependency when using PyInstaller. \ No newline at end of file +Fix incompatibility with transitive dependency charset_normalizer >= 3 when using PyInstaller. From 7f3af6a8ede45dcabc24af0903b41e582c5a5ca6 Mon Sep 17 00:00:00 2001 From: dlee Date: Fri, 3 Feb 2023 16:30:07 -0600 Subject: [PATCH 11/15] typechecks made more strict using more flags --- mypy.ini | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mypy.ini b/mypy.ini index 01cbb57a8..c6d8affe3 100644 --- a/mypy.ini +++ b/mypy.ini @@ -1,3 +1,9 @@ [mypy] ignore_missing_imports = True plugins=mypy_zope:plugin +show_column_numbers = True +pretty = True +show_error_codes = True +warn_unused_configs =True +warn_redundant_casts = True +strict_equality = True \ No newline at end of file From e2e33933a88db59e98e749ab59d7115f3e169aa6 Mon Sep 17 00:00:00 2001 From: dlee Date: Fri, 3 Feb 2023 16:48:06 -0600 Subject: [PATCH 12/15] Forgot to push newsfragment --- newsfragments/3971.minor | 1 + src/allmydata/crypto/rsa.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 newsfragments/3971.minor diff --git a/newsfragments/3971.minor b/newsfragments/3971.minor new file mode 100644 index 000000000..a6cbb6a89 --- /dev/null +++ b/newsfragments/3971.minor @@ -0,0 +1 @@ +Changes made to mypy.ini to make mypy more 'strict' and prevent future regressions. \ No newline at end of file diff --git a/src/allmydata/crypto/rsa.py b/src/allmydata/crypto/rsa.py index e579a3d2a..dca908467 100644 --- a/src/allmydata/crypto/rsa.py +++ b/src/allmydata/crypto/rsa.py @@ -127,7 +127,7 @@ def der_string_from_signing_key(private_key: PrivateKey) -> bytes: :returns: bytes representing `private_key` """ _validate_private_key(private_key) - return private_key.private_bytes( # type: ignore[attr-defined] + return private_key.private_bytes( encoding=Encoding.DER, format=PrivateFormat.PKCS8, encryption_algorithm=NoEncryption(), From 80db4a9de4b3c95ec231d1a55dc7f48f96a014f5 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 3 Feb 2023 21:25:24 -0600 Subject: [PATCH 13/15] Delete rsa.py --- src/allmydata/crypto/rsa.py | 225 ------------------------------------ 1 file changed, 225 deletions(-) delete mode 100644 src/allmydata/crypto/rsa.py diff --git a/src/allmydata/crypto/rsa.py b/src/allmydata/crypto/rsa.py deleted file mode 100644 index dca908467..000000000 --- a/src/allmydata/crypto/rsa.py +++ /dev/null @@ -1,225 +0,0 @@ -""" -Helper functions for cryptography-related operations inside Tahoe -using RSA public-key encryption and decryption. - -In cases where these functions happen to use and return objects that -are documented in the `cryptography` library, code outside this module -should only use functions from allmydata.crypto.rsa and not rely on -features of any objects that `cryptography` documents. - -That is, the public and private keys are opaque objects; DO NOT depend -on any of their methods. -""" - -from __future__ import annotations - -from typing_extensions import TypeAlias -from typing import Callable - -from functools import partial - -from cryptography.exceptions import InvalidSignature -from cryptography.hazmat.backends import default_backend -from cryptography.hazmat.primitives import hashes -from cryptography.hazmat.primitives.asymmetric import rsa, padding -from cryptography.hazmat.primitives.serialization import load_der_private_key, load_der_public_key, \ - Encoding, PrivateFormat, PublicFormat, NoEncryption - -from allmydata.crypto.error import BadSignature - -PublicKey: TypeAlias = rsa.RSAPublicKey -PrivateKey: TypeAlias = rsa.RSAPrivateKey - -# This is the value that was used by `pycryptopp`, and we must continue to use it for -# both backwards compatibility and interoperability. -# -# The docs for `cryptography` suggest to use the constant defined at -# `cryptography.hazmat.primitives.asymmetric.padding.PSS.MAX_LENGTH`, but this causes old -# signatures to fail to validate. -RSA_PSS_SALT_LENGTH = 32 - -RSA_PADDING = padding.PSS( - mgf=padding.MGF1(hashes.SHA256()), - salt_length=RSA_PSS_SALT_LENGTH, -) - - - -def create_signing_keypair(key_size: int) -> tuple[PrivateKey, PublicKey]: - """ - Create a new RSA signing (private) keypair from scratch. Can be used with - `sign_data` function. - - :param key_size: length of key in bits - - :returns: 2-tuple of (private_key, public_key) - """ - priv_key = rsa.generate_private_key( - public_exponent=65537, - key_size=key_size, - backend=default_backend() - ) - return priv_key, priv_key.public_key() - - -def create_signing_keypair_from_string(private_key_der: bytes) -> tuple[PrivateKey, PublicKey]: - """ - Create an RSA signing (private) key from previously serialized - private key bytes. - - :param private_key_der: blob as returned from `der_string_from_signing_keypair` - - :returns: 2-tuple of (private_key, public_key) - """ - _load = partial( - load_der_private_key, - private_key_der, - password=None, - backend=default_backend(), - ) - - def load_with_validation() -> PrivateKey: - k = _load() - assert isinstance(k, PrivateKey) - return k - - def load_without_validation() -> PrivateKey: - k = _load(unsafe_skip_rsa_key_validation=True) - assert isinstance(k, PrivateKey) - return k - - # Load it once without the potentially expensive OpenSSL validation - # checks. These have superlinear complexity. We *will* run them just - # below - but first we'll apply our own constant-time checks. - load: Callable[[], PrivateKey] = load_without_validation - try: - unsafe_priv_key = load() - except TypeError: - # cryptography<39 does not support this parameter, so just load the - # key with validation... - unsafe_priv_key = load_with_validation() - # But avoid *reloading* it since that will run the expensive - # validation *again*. - load = lambda: unsafe_priv_key - - if not isinstance(unsafe_priv_key, rsa.RSAPrivateKey): - raise ValueError( - "Private Key did not decode to an RSA key" - ) - if unsafe_priv_key.key_size != 2048: - raise ValueError( - "Private Key must be 2048 bits" - ) - - # Now re-load it with OpenSSL's validation applied. - safe_priv_key = load() - - return safe_priv_key, safe_priv_key.public_key() - - -def der_string_from_signing_key(private_key: PrivateKey) -> bytes: - """ - Serializes a given RSA private key to a DER string - - :param private_key: a private key object as returned from - `create_signing_keypair` or `create_signing_keypair_from_string` - - :returns: bytes representing `private_key` - """ - _validate_private_key(private_key) - return private_key.private_bytes( - encoding=Encoding.DER, - format=PrivateFormat.PKCS8, - encryption_algorithm=NoEncryption(), - ) - - -def der_string_from_verifying_key(public_key: PublicKey) -> bytes: - """ - Serializes a given RSA public key to a DER string. - - :param public_key: a public key object as returned from - `create_signing_keypair` or `create_signing_keypair_from_string` - - :returns: bytes representing `public_key` - """ - _validate_public_key(public_key) - return public_key.public_bytes( - encoding=Encoding.DER, - format=PublicFormat.SubjectPublicKeyInfo, - ) - - -def create_verifying_key_from_string(public_key_der: bytes) -> PublicKey: - """ - Create an RSA verifying key from a previously serialized public key - - :param bytes public_key_der: a blob as returned by `der_string_from_verifying_key` - - :returns: a public key object suitable for use with other - functions in this module - """ - pub_key = load_der_public_key( - public_key_der, - backend=default_backend(), - ) - assert isinstance(pub_key, PublicKey) - return pub_key - - -def sign_data(private_key: PrivateKey, data: bytes) -> bytes: - """ - :param private_key: the private part of a keypair returned from - `create_signing_keypair_from_string` or `create_signing_keypair` - - :param data: the bytes to sign - - :returns: bytes which are a signature of the bytes given as `data`. - """ - _validate_private_key(private_key) - return private_key.sign( - data, - RSA_PADDING, - hashes.SHA256(), - ) - -def verify_signature(public_key: PublicKey, alleged_signature: bytes, data: bytes) -> None: - """ - :param public_key: a verifying key, returned from `create_verifying_key_from_string` or `create_verifying_key_from_private_key` - - :param bytes alleged_signature: the bytes of the alleged signature - - :param bytes data: the data which was allegedly signed - """ - _validate_public_key(public_key) - try: - public_key.verify( - alleged_signature, - data, - RSA_PADDING, - hashes.SHA256(), - ) - except InvalidSignature: - raise BadSignature() - - -def _validate_public_key(public_key: PublicKey) -> None: - """ - Internal helper. Checks that `public_key` is a valid cryptography - object - """ - if not isinstance(public_key, rsa.RSAPublicKey): - raise ValueError( - f"public_key must be an RSAPublicKey not {type(public_key)}" - ) - - -def _validate_private_key(private_key: PrivateKey) -> None: - """ - Internal helper. Checks that `public_key` is a valid cryptography - object - """ - if not isinstance(private_key, rsa.RSAPrivateKey): - raise ValueError( - f"private_key must be an RSAPrivateKey not {type(private_key)}" - ) From 31c5b78e6ab9d17efffd647767fbf8224ced77f6 Mon Sep 17 00:00:00 2001 From: dlee Date: Fri, 3 Feb 2023 21:35:55 -0600 Subject: [PATCH 14/15] Add back rsa.py accidentally removed file on website --- src/allmydata/crypto/rsa.py | 225 ++++++++++++++++++++++++++++++++++++ 1 file changed, 225 insertions(+) create mode 100644 src/allmydata/crypto/rsa.py diff --git a/src/allmydata/crypto/rsa.py b/src/allmydata/crypto/rsa.py new file mode 100644 index 000000000..e579a3d2a --- /dev/null +++ b/src/allmydata/crypto/rsa.py @@ -0,0 +1,225 @@ +""" +Helper functions for cryptography-related operations inside Tahoe +using RSA public-key encryption and decryption. + +In cases where these functions happen to use and return objects that +are documented in the `cryptography` library, code outside this module +should only use functions from allmydata.crypto.rsa and not rely on +features of any objects that `cryptography` documents. + +That is, the public and private keys are opaque objects; DO NOT depend +on any of their methods. +""" + +from __future__ import annotations + +from typing_extensions import TypeAlias +from typing import Callable + +from functools import partial + +from cryptography.exceptions import InvalidSignature +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives.asymmetric import rsa, padding +from cryptography.hazmat.primitives.serialization import load_der_private_key, load_der_public_key, \ + Encoding, PrivateFormat, PublicFormat, NoEncryption + +from allmydata.crypto.error import BadSignature + +PublicKey: TypeAlias = rsa.RSAPublicKey +PrivateKey: TypeAlias = rsa.RSAPrivateKey + +# This is the value that was used by `pycryptopp`, and we must continue to use it for +# both backwards compatibility and interoperability. +# +# The docs for `cryptography` suggest to use the constant defined at +# `cryptography.hazmat.primitives.asymmetric.padding.PSS.MAX_LENGTH`, but this causes old +# signatures to fail to validate. +RSA_PSS_SALT_LENGTH = 32 + +RSA_PADDING = padding.PSS( + mgf=padding.MGF1(hashes.SHA256()), + salt_length=RSA_PSS_SALT_LENGTH, +) + + + +def create_signing_keypair(key_size: int) -> tuple[PrivateKey, PublicKey]: + """ + Create a new RSA signing (private) keypair from scratch. Can be used with + `sign_data` function. + + :param key_size: length of key in bits + + :returns: 2-tuple of (private_key, public_key) + """ + priv_key = rsa.generate_private_key( + public_exponent=65537, + key_size=key_size, + backend=default_backend() + ) + return priv_key, priv_key.public_key() + + +def create_signing_keypair_from_string(private_key_der: bytes) -> tuple[PrivateKey, PublicKey]: + """ + Create an RSA signing (private) key from previously serialized + private key bytes. + + :param private_key_der: blob as returned from `der_string_from_signing_keypair` + + :returns: 2-tuple of (private_key, public_key) + """ + _load = partial( + load_der_private_key, + private_key_der, + password=None, + backend=default_backend(), + ) + + def load_with_validation() -> PrivateKey: + k = _load() + assert isinstance(k, PrivateKey) + return k + + def load_without_validation() -> PrivateKey: + k = _load(unsafe_skip_rsa_key_validation=True) + assert isinstance(k, PrivateKey) + return k + + # Load it once without the potentially expensive OpenSSL validation + # checks. These have superlinear complexity. We *will* run them just + # below - but first we'll apply our own constant-time checks. + load: Callable[[], PrivateKey] = load_without_validation + try: + unsafe_priv_key = load() + except TypeError: + # cryptography<39 does not support this parameter, so just load the + # key with validation... + unsafe_priv_key = load_with_validation() + # But avoid *reloading* it since that will run the expensive + # validation *again*. + load = lambda: unsafe_priv_key + + if not isinstance(unsafe_priv_key, rsa.RSAPrivateKey): + raise ValueError( + "Private Key did not decode to an RSA key" + ) + if unsafe_priv_key.key_size != 2048: + raise ValueError( + "Private Key must be 2048 bits" + ) + + # Now re-load it with OpenSSL's validation applied. + safe_priv_key = load() + + return safe_priv_key, safe_priv_key.public_key() + + +def der_string_from_signing_key(private_key: PrivateKey) -> bytes: + """ + Serializes a given RSA private key to a DER string + + :param private_key: a private key object as returned from + `create_signing_keypair` or `create_signing_keypair_from_string` + + :returns: bytes representing `private_key` + """ + _validate_private_key(private_key) + return private_key.private_bytes( # type: ignore[attr-defined] + encoding=Encoding.DER, + format=PrivateFormat.PKCS8, + encryption_algorithm=NoEncryption(), + ) + + +def der_string_from_verifying_key(public_key: PublicKey) -> bytes: + """ + Serializes a given RSA public key to a DER string. + + :param public_key: a public key object as returned from + `create_signing_keypair` or `create_signing_keypair_from_string` + + :returns: bytes representing `public_key` + """ + _validate_public_key(public_key) + return public_key.public_bytes( + encoding=Encoding.DER, + format=PublicFormat.SubjectPublicKeyInfo, + ) + + +def create_verifying_key_from_string(public_key_der: bytes) -> PublicKey: + """ + Create an RSA verifying key from a previously serialized public key + + :param bytes public_key_der: a blob as returned by `der_string_from_verifying_key` + + :returns: a public key object suitable for use with other + functions in this module + """ + pub_key = load_der_public_key( + public_key_der, + backend=default_backend(), + ) + assert isinstance(pub_key, PublicKey) + return pub_key + + +def sign_data(private_key: PrivateKey, data: bytes) -> bytes: + """ + :param private_key: the private part of a keypair returned from + `create_signing_keypair_from_string` or `create_signing_keypair` + + :param data: the bytes to sign + + :returns: bytes which are a signature of the bytes given as `data`. + """ + _validate_private_key(private_key) + return private_key.sign( + data, + RSA_PADDING, + hashes.SHA256(), + ) + +def verify_signature(public_key: PublicKey, alleged_signature: bytes, data: bytes) -> None: + """ + :param public_key: a verifying key, returned from `create_verifying_key_from_string` or `create_verifying_key_from_private_key` + + :param bytes alleged_signature: the bytes of the alleged signature + + :param bytes data: the data which was allegedly signed + """ + _validate_public_key(public_key) + try: + public_key.verify( + alleged_signature, + data, + RSA_PADDING, + hashes.SHA256(), + ) + except InvalidSignature: + raise BadSignature() + + +def _validate_public_key(public_key: PublicKey) -> None: + """ + Internal helper. Checks that `public_key` is a valid cryptography + object + """ + if not isinstance(public_key, rsa.RSAPublicKey): + raise ValueError( + f"public_key must be an RSAPublicKey not {type(public_key)}" + ) + + +def _validate_private_key(private_key: PrivateKey) -> None: + """ + Internal helper. Checks that `public_key` is a valid cryptography + object + """ + if not isinstance(private_key, rsa.RSAPrivateKey): + raise ValueError( + f"private_key must be an RSAPrivateKey not {type(private_key)}" + ) From eb26c97ef70edeafe3f6777759dcca7d6a08c98c Mon Sep 17 00:00:00 2001 From: dlee Date: Mon, 6 Feb 2023 15:29:53 -0600 Subject: [PATCH 15/15] implicit_optional flag added and errors related to flag fixed --- mypy.ini | 1 + src/allmydata/web/common.py | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/mypy.ini b/mypy.ini index c6d8affe3..e6e7d16ff 100644 --- a/mypy.ini +++ b/mypy.ini @@ -5,5 +5,6 @@ show_column_numbers = True pretty = True show_error_codes = True warn_unused_configs =True +no_implicit_optional = True warn_redundant_casts = True strict_equality = True \ No newline at end of file diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index c49354217..3d85b1c4d 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -707,12 +707,12 @@ def url_for_string(req, url_string): T = TypeVar("T") @overload -def get_arg(req: IRequest, argname: str | bytes, default: T = None, *, multiple: Literal[False] = False) -> T | bytes: ... +def get_arg(req: IRequest, argname: str | bytes, default: Optional[T] = None, *, multiple: Literal[False] = False) -> T | bytes: ... @overload -def get_arg(req: IRequest, argname: str | bytes, default: T = None, *, multiple: Literal[True]) -> T | tuple[bytes, ...]: ... +def get_arg(req: IRequest, argname: str | bytes, default: Optional[T] = None, *, multiple: Literal[True]) -> T | tuple[bytes, ...]: ... -def get_arg(req: IRequest, argname: str | bytes, default: T = None, *, multiple: bool = False) -> None | T | bytes | tuple[bytes, ...]: +def get_arg(req: IRequest, argname: str | bytes, default: Optional[T] = None, *, multiple: bool = False) -> None | T | bytes | tuple[bytes, ...]: """Extract an argument from either the query args (req.args) or the form body fields (req.fields). If multiple=False, this returns a single value (or the default, which defaults to None), and the query args take