From 15e22dcc52c73721f05a2bc48f84c9b5d21b005e Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Mon, 2 Jan 2023 19:29:13 -0500 Subject: [PATCH 01/33] Add `keypair` to `NodeMaker.create_mutable_file` Previously `NodeMaker` always took responsibility for generating a keypair to use. Now the caller may supply one. --- src/allmydata/crypto/rsa.py | 14 +++++--------- src/allmydata/nodemaker.py | 19 +++++++++---------- src/allmydata/test/mutable/test_filenode.py | 11 +++++++++++ src/allmydata/test/test_dirnode.py | 3 ++- src/allmydata/test/web/test_web.py | 5 ++++- 5 files changed, 31 insertions(+), 21 deletions(-) diff --git a/src/allmydata/crypto/rsa.py b/src/allmydata/crypto/rsa.py index 95cf01413..7ea4e6c13 100644 --- a/src/allmydata/crypto/rsa.py +++ b/src/allmydata/crypto/rsa.py @@ -9,17 +9,11 @@ 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. - -Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals -from future.utils import PY2 -if PY2: - from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +from __future__ import annotations + +from typing import TypeVar from cryptography.exceptions import InvalidSignature from cryptography.hazmat.backends import default_backend @@ -30,6 +24,8 @@ from cryptography.hazmat.primitives.serialization import load_der_private_key, l from allmydata.crypto.error import BadSignature +PublicKey = TypeVar("PublicKey", bound=rsa.RSAPublicKey) +PrivateKey = TypeVar("PrivateKey", bound=rsa.RSAPrivateKey) # This is the value that was used by `pycryptopp`, and we must continue to use it for # both backwards compatibility and interoperability. diff --git a/src/allmydata/nodemaker.py b/src/allmydata/nodemaker.py index 23ba4b451..1b7ea5f45 100644 --- a/src/allmydata/nodemaker.py +++ b/src/allmydata/nodemaker.py @@ -1,17 +1,12 @@ """ -Ported to Python 3. +Create file nodes of various types. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +from __future__ import annotations import weakref from zope.interface import implementer +from twisted.internet.defer import succeed from allmydata.util.assertutil import precondition from allmydata.interfaces import INodeMaker from allmydata.immutable.literal import LiteralFileNode @@ -22,6 +17,7 @@ from allmydata.mutable.publish import MutableData from allmydata.dirnode import DirectoryNode, pack_children from allmydata.unknown import UnknownNode from allmydata.blacklist import ProhibitedNode +from allmydata.crypto.rsa import PublicKey, PrivateKey from allmydata import uri @@ -126,12 +122,15 @@ class NodeMaker(object): return self._create_dirnode(filenode) return None - def create_mutable_file(self, contents=None, version=None): + def create_mutable_file(self, contents=None, version=None, keypair: tuple[PublicKey, PrivateKey] | None = None): if version is None: version = self.mutable_file_default n = MutableFileNode(self.storage_broker, self.secret_holder, self.default_encoding_parameters, self.history) - d = self.key_generator.generate() + if keypair is None: + d = self.key_generator.generate() + else: + d = succeed(keypair) d.addCallback(n.create_with_keys, contents, version=version) d.addCallback(lambda res: n) return d diff --git a/src/allmydata/test/mutable/test_filenode.py b/src/allmydata/test/mutable/test_filenode.py index 579734433..6c00e4420 100644 --- a/src/allmydata/test/mutable/test_filenode.py +++ b/src/allmydata/test/mutable/test_filenode.py @@ -30,6 +30,7 @@ from allmydata.mutable.publish import MutableData from ..test_download import PausingConsumer, PausingAndStoppingConsumer, \ StoppingConsumer, ImmediatelyStoppingConsumer from .. import common_util as testutil +from ...crypto.rsa import create_signing_keypair from .util import ( FakeStorage, make_nodemaker_with_peers, @@ -65,6 +66,16 @@ class Filenode(AsyncBrokenTestCase, testutil.ShouldFailMixin): d.addCallback(_created) return d + async def test_create_with_keypair(self): + """ + An SDMF can be created using a given keypair. + """ + (priv, pub) = create_signing_keypair(2048) + node = await self.nodemaker.create_mutable_file(keypair=(pub, priv)) + self.assertThat( + (node.get_privkey(), node.get_pubkey()), + Equals((priv, pub)), + ) def test_create_mdmf(self): d = self.nodemaker.create_mutable_file(version=MDMF_VERSION) diff --git a/src/allmydata/test/test_dirnode.py b/src/allmydata/test/test_dirnode.py index 67d331430..2319e3ce1 100644 --- a/src/allmydata/test/test_dirnode.py +++ b/src/allmydata/test/test_dirnode.py @@ -1619,7 +1619,8 @@ class FakeMutableFile(object): # type: ignore # incomplete implementation return defer.succeed(None) class FakeNodeMaker(NodeMaker): - def create_mutable_file(self, contents=b"", keysize=None, version=None): + def create_mutable_file(self, contents=b"", keysize=None, version=None, keypair=None): + assert keypair is None, "FakeNodeMaker does not support externally supplied keypairs" return defer.succeed(FakeMutableFile(contents)) class FakeClient2(_Client): # type: ignore # tahoe-lafs/ticket/3573 diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 03cd6e560..4fc9b37e5 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -102,7 +102,10 @@ class FakeNodeMaker(NodeMaker): self.encoding_params, None, self.all_contents).init_from_cap(cap) def create_mutable_file(self, contents=b"", keysize=None, - version=SDMF_VERSION): + version=SDMF_VERSION, + keypair=None, + ): + assert keypair is None, "FakeNodeMaker does not support externally supplied keypairs" n = FakeMutableFileNode(None, None, self.encoding_params, None, self.all_contents) return n.create(contents, version=version) From 23f2d8b019578b4ae9706b2e143ca1bfe44f57b1 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 3 Jan 2023 10:28:32 -0500 Subject: [PATCH 02/33] add some type annotations to allmydata.crypto.rsa --- src/allmydata/crypto/rsa.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/allmydata/crypto/rsa.py b/src/allmydata/crypto/rsa.py index 7ea4e6c13..a4b2090a0 100644 --- a/src/allmydata/crypto/rsa.py +++ b/src/allmydata/crypto/rsa.py @@ -42,12 +42,12 @@ RSA_PADDING = padding.PSS( -def create_signing_keypair(key_size): +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 int key_size: length of key in bits + :param key_size: length of key in bits :returns: 2-tuple of (private_key, public_key) """ @@ -59,12 +59,12 @@ def create_signing_keypair(key_size): return priv_key, priv_key.public_key() -def create_signing_keypair_from_string(private_key_der): +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 bytes private_key_der: blob as returned from `der_string_from_signing_keypair` + :param private_key_der: blob as returned from `der_string_from_signing_keypair` :returns: 2-tuple of (private_key, public_key) """ @@ -84,7 +84,7 @@ def create_signing_keypair_from_string(private_key_der): return priv_key, priv_key.public_key() -def der_string_from_signing_key(private_key): +def der_string_from_signing_key(private_key: PrivateKey) -> bytes: """ Serializes a given RSA private key to a DER string @@ -101,7 +101,7 @@ def der_string_from_signing_key(private_key): ) -def der_string_from_verifying_key(public_key): +def der_string_from_verifying_key(public_key: PublicKey) -> bytes: """ Serializes a given RSA public key to a DER string. @@ -117,7 +117,7 @@ def der_string_from_verifying_key(public_key): ) -def create_verifying_key_from_string(public_key_der): +def create_verifying_key_from_string(public_key_der: bytes) -> PublicKey: """ Create an RSA verifying key from a previously serialized public key @@ -133,12 +133,12 @@ def create_verifying_key_from_string(public_key_der): return pub_key -def sign_data(private_key, data): +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 bytes data: the bytes to sign + :param data: the bytes to sign :returns: bytes which are a signature of the bytes given as `data`. """ @@ -149,7 +149,7 @@ def sign_data(private_key, data): hashes.SHA256(), ) -def verify_signature(public_key, alleged_signature, data): +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` @@ -169,7 +169,7 @@ def verify_signature(public_key, alleged_signature, data): raise BadSignature() -def _validate_public_key(public_key): +def _validate_public_key(public_key: PublicKey) -> None: """ Internal helper. Checks that `public_key` is a valid cryptography object @@ -180,7 +180,7 @@ def _validate_public_key(public_key): ) -def _validate_private_key(private_key): +def _validate_private_key(private_key: PrivateKey) -> None: """ Internal helper. Checks that `public_key` is a valid cryptography object From f6d9c335261c3f7f9b1ef61fa26e7559a16af141 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 3 Jan 2023 10:28:59 -0500 Subject: [PATCH 03/33] Give slightly better error messages from rsa key validation failure --- src/allmydata/crypto/rsa.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/crypto/rsa.py b/src/allmydata/crypto/rsa.py index a4b2090a0..5acc59ab2 100644 --- a/src/allmydata/crypto/rsa.py +++ b/src/allmydata/crypto/rsa.py @@ -176,7 +176,7 @@ def _validate_public_key(public_key: PublicKey) -> None: """ if not isinstance(public_key, rsa.RSAPublicKey): raise ValueError( - "public_key must be an RSAPublicKey" + f"public_key must be an RSAPublicKey not {type(public_key)}" ) @@ -187,5 +187,5 @@ def _validate_private_key(private_key: PrivateKey) -> None: """ if not isinstance(private_key, rsa.RSAPrivateKey): raise ValueError( - "private_key must be an RSAPrivateKey" + f"private_key must be an RSAPrivateKey not {type(private_key)}" ) From 6b58b6667786ec712c48486d2711d70868c1fa2b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 3 Jan 2023 10:32:03 -0500 Subject: [PATCH 04/33] Clean up some Python 2 remnants --- src/allmydata/web/filenode.py | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/src/allmydata/web/filenode.py b/src/allmydata/web/filenode.py index dd793888e..678078ba3 100644 --- a/src/allmydata/web/filenode.py +++ b/src/allmydata/web/filenode.py @@ -1,23 +1,13 @@ """ Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals +from __future__ import annotations -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, max, min # noqa: F401 - # Use native unicode() as str() to prevent leaking futurebytes in ways that - # break string formattin. - from past.builtins import unicode as str -from past.builtins import long from twisted.web import http, static from twisted.internet import defer from twisted.web.resource import ( - Resource, # note: Resource is an old-style class + Resource, ErrorPage, ) @@ -395,7 +385,7 @@ class FileDownloader(Resource, object): # list of (first,last) inclusive range tuples. filesize = self.filenode.get_size() - assert isinstance(filesize, (int,long)), filesize + assert isinstance(filesize, int), filesize try: # byte-ranges-specifier @@ -408,19 +398,19 @@ class FileDownloader(Resource, object): if first == '': # suffix-byte-range-spec - first = filesize - long(last) + first = filesize - int(last) last = filesize - 1 else: # byte-range-spec # first-byte-pos - first = long(first) + first = int(first) # last-byte-pos if last == '': last = filesize - 1 else: - last = long(last) + last = int(last) if last < first: raise ValueError @@ -456,7 +446,7 @@ class FileDownloader(Resource, object): b'attachment; filename="%s"' % self.filename) filesize = self.filenode.get_size() - assert isinstance(filesize, (int,long)), filesize + assert isinstance(filesize, int), filesize first, size = 0, None contentsize = filesize req.setHeader("accept-ranges", "bytes") From a58d8a567af39f0ab86a2d28794999a84c9b0cf0 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 3 Jan 2023 10:33:37 -0500 Subject: [PATCH 05/33] Clean up some more Python 2 remnants --- src/allmydata/mutable/common.py | 9 +- src/allmydata/mutable/filenode.py | 9 +- src/allmydata/mutable/retrieve.py | 10 +- src/allmydata/mutable/servermap.py | 20 +-- src/allmydata/test/_win_subprocess.py | 210 -------------------------- src/allmydata/test/common.py | 25 +-- 6 files changed, 13 insertions(+), 270 deletions(-) delete mode 100644 src/allmydata/test/_win_subprocess.py diff --git a/src/allmydata/mutable/common.py b/src/allmydata/mutable/common.py index 87951c7b2..a2e482d3c 100644 --- a/src/allmydata/mutable/common.py +++ b/src/allmydata/mutable/common.py @@ -1,14 +1,7 @@ """ Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +from __future__ import annotations MODE_CHECK = "MODE_CHECK" # query all peers MODE_ANYTHING = "MODE_ANYTHING" # one recoverable version diff --git a/src/allmydata/mutable/filenode.py b/src/allmydata/mutable/filenode.py index cd8cb0dc7..99fdcc085 100644 --- a/src/allmydata/mutable/filenode.py +++ b/src/allmydata/mutable/filenode.py @@ -1,14 +1,7 @@ """ Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +from __future__ import annotations import random diff --git a/src/allmydata/mutable/retrieve.py b/src/allmydata/mutable/retrieve.py index 32aaa72e5..efb2c0f85 100644 --- a/src/allmydata/mutable/retrieve.py +++ b/src/allmydata/mutable/retrieve.py @@ -1,15 +1,7 @@ """ Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - # Don't import bytes and str, to prevent API leakage - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, dict, list, object, range, max, min # noqa: F401 +from __future__ import annotations import time diff --git a/src/allmydata/mutable/servermap.py b/src/allmydata/mutable/servermap.py index 211b1fc16..cd220ce0f 100644 --- a/src/allmydata/mutable/servermap.py +++ b/src/allmydata/mutable/servermap.py @@ -1,16 +1,8 @@ """ Ported to Python 3. """ -from __future__ import print_function -from __future__ import absolute_import -from __future__ import division -from __future__ import unicode_literals +from __future__ import annotations -from future.utils import PY2 -if PY2: - # Doesn't import str to prevent API leakage on Python 2 - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, max, min # noqa: F401 -from past.builtins import unicode from six import ensure_str import sys, time, copy @@ -203,8 +195,8 @@ class ServerMap(object): (seqnum, root_hash, IV, segsize, datalength, k, N, prefix, offsets_tuple) = verinfo print("[%s]: sh#%d seq%d-%s %d-of-%d len%d" % - (unicode(server.get_name(), "utf-8"), shnum, - seqnum, unicode(base32.b2a(root_hash)[:4], "utf-8"), k, N, + (str(server.get_name(), "utf-8"), shnum, + seqnum, str(base32.b2a(root_hash)[:4], "utf-8"), k, N, datalength), file=out) if self._problems: print("%d PROBLEMS" % len(self._problems), file=out) @@ -276,7 +268,7 @@ class ServerMap(object): """Take a versionid, return a string that describes it.""" (seqnum, root_hash, IV, segsize, datalength, k, N, prefix, offsets_tuple) = verinfo - return "seq%d-%s" % (seqnum, unicode(base32.b2a(root_hash)[:4], "utf-8")) + return "seq%d-%s" % (seqnum, str(base32.b2a(root_hash)[:4], "utf-8")) def summarize_versions(self): """Return a string describing which versions we know about.""" @@ -824,7 +816,7 @@ class ServermapUpdater(object): def notify_server_corruption(self, server, shnum, reason): - if isinstance(reason, unicode): + if isinstance(reason, str): reason = reason.encode("utf-8") ss = server.get_storage_server() ss.advise_corrupt_share( @@ -879,7 +871,7 @@ class ServermapUpdater(object): # ok, it's a valid verinfo. Add it to the list of validated # versions. self.log(" found valid version %d-%s from %s-sh%d: %d-%d/%d/%d" - % (seqnum, unicode(base32.b2a(root_hash)[:4], "utf-8"), + % (seqnum, str(base32.b2a(root_hash)[:4], "utf-8"), ensure_str(server.get_name()), shnum, k, n, segsize, datalen), parent=lp) diff --git a/src/allmydata/test/_win_subprocess.py b/src/allmydata/test/_win_subprocess.py deleted file mode 100644 index bf9767e73..000000000 --- a/src/allmydata/test/_win_subprocess.py +++ /dev/null @@ -1,210 +0,0 @@ -""" -This module is only necessary on Python 2. Once Python 2 code is dropped, it -can be deleted. -""" - -from future.utils import PY3 -if PY3: - raise RuntimeError("Just use subprocess.Popen") - -# This is necessary to pacify flake8 on Python 3, while we're still supporting -# Python 2. -from past.builtins import unicode - -# -*- coding: utf-8 -*- - -## Copyright (C) 2021 Valentin Lab -## -## Redistribution and use in source and binary forms, with or without -## modification, are permitted provided that the following conditions -## are met: -## -## 1. Redistributions of source code must retain the above copyright -## notice, this list of conditions and the following disclaimer. -## -## 2. Redistributions in binary form must reproduce the above -## copyright notice, this list of conditions and the following -## disclaimer in the documentation and/or other materials provided -## with the distribution. -## -## 3. Neither the name of the copyright holder nor the names of its -## contributors may be used to endorse or promote products derived -## from this software without specific prior written permission. -## -## THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -## "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -## LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS -## FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE -## COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, -## INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES -## (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR -## SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) -## HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, -## STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) -## ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED -## OF THE POSSIBILITY OF SUCH DAMAGE. -## - -## issue: https://bugs.python.org/issue19264 - -# See allmydata/windows/fixups.py -import sys -assert sys.platform == "win32" - -import os -import ctypes -import subprocess -import _subprocess -from ctypes import byref, windll, c_char_p, c_wchar_p, c_void_p, \ - Structure, sizeof, c_wchar, WinError -from ctypes.wintypes import BYTE, WORD, LPWSTR, BOOL, DWORD, LPVOID, \ - HANDLE - - -## -## Types -## - -CREATE_UNICODE_ENVIRONMENT = 0x00000400 -LPCTSTR = c_char_p -LPTSTR = c_wchar_p -LPSECURITY_ATTRIBUTES = c_void_p -LPBYTE = ctypes.POINTER(BYTE) - -class STARTUPINFOW(Structure): - _fields_ = [ - ("cb", DWORD), ("lpReserved", LPWSTR), - ("lpDesktop", LPWSTR), ("lpTitle", LPWSTR), - ("dwX", DWORD), ("dwY", DWORD), - ("dwXSize", DWORD), ("dwYSize", DWORD), - ("dwXCountChars", DWORD), ("dwYCountChars", DWORD), - ("dwFillAtrribute", DWORD), ("dwFlags", DWORD), - ("wShowWindow", WORD), ("cbReserved2", WORD), - ("lpReserved2", LPBYTE), ("hStdInput", HANDLE), - ("hStdOutput", HANDLE), ("hStdError", HANDLE), - ] - -LPSTARTUPINFOW = ctypes.POINTER(STARTUPINFOW) - - -class PROCESS_INFORMATION(Structure): - _fields_ = [ - ("hProcess", HANDLE), ("hThread", HANDLE), - ("dwProcessId", DWORD), ("dwThreadId", DWORD), - ] - -LPPROCESS_INFORMATION = ctypes.POINTER(PROCESS_INFORMATION) - - -class DUMMY_HANDLE(ctypes.c_void_p): - - def __init__(self, *a, **kw): - super(DUMMY_HANDLE, self).__init__(*a, **kw) - self.closed = False - - def Close(self): - if not self.closed: - windll.kernel32.CloseHandle(self) - self.closed = True - - def __int__(self): - return self.value - - -CreateProcessW = windll.kernel32.CreateProcessW -CreateProcessW.argtypes = [ - LPCTSTR, LPTSTR, LPSECURITY_ATTRIBUTES, - LPSECURITY_ATTRIBUTES, BOOL, DWORD, LPVOID, LPCTSTR, - LPSTARTUPINFOW, LPPROCESS_INFORMATION, -] -CreateProcessW.restype = BOOL - - -## -## Patched functions/classes -## - -def CreateProcess(executable, args, _p_attr, _t_attr, - inherit_handles, creation_flags, env, cwd, - startup_info): - """Create a process supporting unicode executable and args for win32 - - Python implementation of CreateProcess using CreateProcessW for Win32 - - """ - - si = STARTUPINFOW( - dwFlags=startup_info.dwFlags, - wShowWindow=startup_info.wShowWindow, - cb=sizeof(STARTUPINFOW), - ## XXXvlab: not sure of the casting here to ints. - hStdInput=int(startup_info.hStdInput), - hStdOutput=int(startup_info.hStdOutput), - hStdError=int(startup_info.hStdError), - ) - - wenv = None - if env is not None: - ## LPCWSTR seems to be c_wchar_p, so let's say CWSTR is c_wchar - env = (unicode("").join([ - unicode("%s=%s\0") % (k, v) - for k, v in env.items()])) + unicode("\0") - wenv = (c_wchar * len(env))() - wenv.value = env - - pi = PROCESS_INFORMATION() - creation_flags |= CREATE_UNICODE_ENVIRONMENT - - if CreateProcessW(executable, args, None, None, - inherit_handles, creation_flags, - wenv, cwd, byref(si), byref(pi)): - return (DUMMY_HANDLE(pi.hProcess), DUMMY_HANDLE(pi.hThread), - pi.dwProcessId, pi.dwThreadId) - raise WinError() - - -class Popen(subprocess.Popen): - """This superseeds Popen and corrects a bug in cPython 2.7 implem""" - - def _execute_child(self, args, executable, preexec_fn, close_fds, - cwd, env, universal_newlines, - startupinfo, creationflags, shell, to_close, - p2cread, p2cwrite, - c2pread, c2pwrite, - errread, errwrite): - """Code from part of _execute_child from Python 2.7 (9fbb65e) - - There are only 2 little changes concerning the construction of - the the final string in shell mode: we preempt the creation of - the command string when shell is True, because original function - will try to encode unicode args which we want to avoid to be able to - sending it as-is to ``CreateProcess``. - - """ - if not isinstance(args, subprocess.types.StringTypes): - args = subprocess.list2cmdline(args) - - if startupinfo is None: - startupinfo = subprocess.STARTUPINFO() - if shell: - startupinfo.dwFlags |= _subprocess.STARTF_USESHOWWINDOW - startupinfo.wShowWindow = _subprocess.SW_HIDE - comspec = os.environ.get("COMSPEC", unicode("cmd.exe")) - args = unicode('{} /c "{}"').format(comspec, args) - if (_subprocess.GetVersion() >= 0x80000000 or - os.path.basename(comspec).lower() == "command.com"): - w9xpopen = self._find_w9xpopen() - args = unicode('"%s" %s') % (w9xpopen, args) - creationflags |= _subprocess.CREATE_NEW_CONSOLE - - cp = _subprocess.CreateProcess - _subprocess.CreateProcess = CreateProcess - try: - super(Popen, self)._execute_child( - args, executable, - preexec_fn, close_fds, cwd, env, universal_newlines, - startupinfo, creationflags, False, to_close, p2cread, - p2cwrite, c2pread, c2pwrite, errread, errwrite, - ) - finally: - _subprocess.CreateProcess = cp diff --git a/src/allmydata/test/common.py b/src/allmydata/test/common.py index b652b2e48..5728b84ad 100644 --- a/src/allmydata/test/common.py +++ b/src/allmydata/test/common.py @@ -1,14 +1,8 @@ """ -Ported to Python 3. +Functionality related to a lot of the test suite. """ -from __future__ import print_function -from __future__ import absolute_import -from __future__ import division -from __future__ import unicode_literals +from __future__ import annotations -from future.utils import PY2, native_str -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 from past.builtins import chr as byteschr __all__ = [ @@ -117,19 +111,8 @@ from .eliotutil import ( ) from .common_util import ShouldFailMixin # noqa: F401 -if sys.platform == "win32" and PY2: - # Python 2.7 doesn't have good options for launching a process with - # non-ASCII in its command line. So use this alternative that does a - # better job. However, only use it on Windows because it doesn't work - # anywhere else. - from ._win_subprocess import ( - Popen, - ) -else: - from subprocess import ( - Popen, - ) from subprocess import ( + Popen, PIPE, ) @@ -298,7 +281,7 @@ class UseNode(object): plugin_config = attr.ib() storage_plugin = attr.ib() basedir = attr.ib(validator=attr.validators.instance_of(FilePath)) - introducer_furl = attr.ib(validator=attr.validators.instance_of(native_str), + introducer_furl = attr.ib(validator=attr.validators.instance_of(str), converter=six.ensure_str) node_config = attr.ib(default=attr.Factory(dict)) From 5bad92cfc533a4a236ffb1bec21acfd611aef40d Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 3 Jan 2023 10:34:39 -0500 Subject: [PATCH 06/33] Another Python 2 remnant cleanup --- src/allmydata/test/web/test_web.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 4fc9b37e5..c220b0a0b 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -1,14 +1,8 @@ """ -Ported to Python 3. +Tests for a bunch of web-related APIs. """ -from __future__ import print_function -from __future__ import absolute_import -from __future__ import division -from __future__ import unicode_literals +from __future__ import annotations -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 from six import ensure_binary import os.path, re, time From c7bb190290ce2b85cb78605599daeb8aefbc072b Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 3 Jan 2023 10:38:18 -0500 Subject: [PATCH 07/33] Factor some SSK "signature" key handling code into a more reusable shape This gives the test suite access to the derivation function so it can re-derive certain values to use as expected results to compare against actual results. --- src/allmydata/mutable/common.py | 33 ++++++++++++++++++++++++++++++ src/allmydata/mutable/filenode.py | 33 +++++++++++------------------- src/allmydata/mutable/retrieve.py | 7 ++++--- src/allmydata/mutable/servermap.py | 7 ++++--- 4 files changed, 53 insertions(+), 27 deletions(-) diff --git a/src/allmydata/mutable/common.py b/src/allmydata/mutable/common.py index a2e482d3c..33a1c2731 100644 --- a/src/allmydata/mutable/common.py +++ b/src/allmydata/mutable/common.py @@ -10,6 +10,9 @@ MODE_WRITE = "MODE_WRITE" # replace all shares, probably.. not for initial MODE_READ = "MODE_READ" MODE_REPAIR = "MODE_REPAIR" # query all peers, get the privkey +from allmydata.crypto import aes, rsa +from allmydata.util import hashutil + class NotWriteableError(Exception): pass @@ -61,3 +64,33 @@ class CorruptShareError(BadShareError): class UnknownVersionError(BadShareError): """The share we received was of a version we don't recognize.""" + + +def encrypt_privkey(writekey: bytes, privkey: rsa.PrivateKey) -> bytes: + """ + For SSK, encrypt a private ("signature") key using the writekey. + """ + encryptor = aes.create_encryptor(writekey) + crypttext = aes.encrypt_data(encryptor, privkey) + return crypttext + +def decrypt_privkey(writekey: bytes, enc_privkey: bytes) -> rsa.PrivateKey: + """ + The inverse of ``encrypt_privkey``. + """ + decryptor = aes.create_decryptor(writekey) + privkey = aes.decrypt_data(decryptor, enc_privkey) + return privkey + +def derive_mutable_keys(keypair: tuple[rsa.PublicKey, rsa.PrivateKey]) -> tuple[bytes, bytes, bytes]: + """ + Derive the SSK writekey, encrypted writekey, and fingerprint from the + public/private ("verification" / "signature") keypair. + """ + pubkey, privkey = keypair + pubkey_s = rsa.der_string_from_verifying_key(pubkey) + privkey_s = rsa.der_string_from_signing_key(privkey) + writekey = hashutil.ssk_writekey_hash(privkey_s) + encprivkey = encrypt_privkey(writekey, privkey_s) + fingerprint = hashutil.ssk_pubkey_fingerprint_hash(pubkey_s) + return writekey, encprivkey, fingerprint diff --git a/src/allmydata/mutable/filenode.py b/src/allmydata/mutable/filenode.py index 99fdcc085..00b31c52b 100644 --- a/src/allmydata/mutable/filenode.py +++ b/src/allmydata/mutable/filenode.py @@ -9,8 +9,6 @@ from zope.interface import implementer from twisted.internet import defer, reactor from foolscap.api import eventually -from allmydata.crypto import aes -from allmydata.crypto import rsa from allmydata.interfaces import IMutableFileNode, ICheckable, ICheckResults, \ NotEnoughSharesError, MDMF_VERSION, SDMF_VERSION, IMutableUploadable, \ IMutableFileVersion, IWriteable @@ -21,8 +19,14 @@ from allmydata.uri import WriteableSSKFileURI, ReadonlySSKFileURI, \ from allmydata.monitor import Monitor from allmydata.mutable.publish import Publish, MutableData,\ TransformingUploadable -from allmydata.mutable.common import MODE_READ, MODE_WRITE, MODE_CHECK, UnrecoverableFileError, \ - UncoordinatedWriteError +from allmydata.mutable.common import ( + MODE_READ, + MODE_WRITE, + MODE_CHECK, + UnrecoverableFileError, + UncoordinatedWriteError, + derive_mutable_keys, +) from allmydata.mutable.servermap import ServerMap, ServermapUpdater from allmydata.mutable.retrieve import Retrieve from allmydata.mutable.checker import MutableChecker, MutableCheckAndRepairer @@ -132,13 +136,10 @@ class MutableFileNode(object): Deferred that fires (with the MutableFileNode instance you should use) when it completes. """ - (pubkey, privkey) = keypair - self._pubkey, self._privkey = pubkey, privkey - pubkey_s = rsa.der_string_from_verifying_key(self._pubkey) - privkey_s = rsa.der_string_from_signing_key(self._privkey) - self._writekey = hashutil.ssk_writekey_hash(privkey_s) - self._encprivkey = self._encrypt_privkey(self._writekey, privkey_s) - self._fingerprint = hashutil.ssk_pubkey_fingerprint_hash(pubkey_s) + self._pubkey, self._privkey = keypair + self._writekey, self._encprivkey, self._fingerprint = derive_mutable_keys( + keypair, + ) if version == MDMF_VERSION: self._uri = WriteableMDMFFileURI(self._writekey, self._fingerprint) self._protocol_version = version @@ -164,16 +165,6 @@ class MutableFileNode(object): (contents, type(contents)) return contents(self) - def _encrypt_privkey(self, writekey, privkey): - encryptor = aes.create_encryptor(writekey) - crypttext = aes.encrypt_data(encryptor, privkey) - return crypttext - - def _decrypt_privkey(self, enc_privkey): - decryptor = aes.create_decryptor(self._writekey) - privkey = aes.decrypt_data(decryptor, enc_privkey) - return privkey - def _populate_pubkey(self, pubkey): self._pubkey = pubkey def _populate_required_shares(self, required_shares): diff --git a/src/allmydata/mutable/retrieve.py b/src/allmydata/mutable/retrieve.py index efb2c0f85..64573a49a 100644 --- a/src/allmydata/mutable/retrieve.py +++ b/src/allmydata/mutable/retrieve.py @@ -24,7 +24,7 @@ from allmydata import hashtree, codec from allmydata.storage.server import si_b2a from allmydata.mutable.common import CorruptShareError, BadShareError, \ - UncoordinatedWriteError + UncoordinatedWriteError, decrypt_privkey from allmydata.mutable.layout import MDMFSlotReadProxy @implementer(IRetrieveStatus) @@ -923,9 +923,10 @@ class Retrieve(object): def _try_to_validate_privkey(self, enc_privkey, reader, server): - alleged_privkey_s = self._node._decrypt_privkey(enc_privkey) + node_writekey = self._node.get_writekey() + alleged_privkey_s = decrypt_privkey(node_writekey, enc_privkey) alleged_writekey = hashutil.ssk_writekey_hash(alleged_privkey_s) - if alleged_writekey != self._node.get_writekey(): + if alleged_writekey != node_writekey: self.log("invalid privkey from %s shnum %d" % (reader, reader.shnum), level=log.WEIRD, umid="YIw4tA") diff --git a/src/allmydata/mutable/servermap.py b/src/allmydata/mutable/servermap.py index cd220ce0f..99aa85d24 100644 --- a/src/allmydata/mutable/servermap.py +++ b/src/allmydata/mutable/servermap.py @@ -21,7 +21,7 @@ from allmydata.storage.server import si_b2a from allmydata.interfaces import IServermapUpdaterStatus from allmydata.mutable.common import MODE_CHECK, MODE_ANYTHING, MODE_WRITE, \ - MODE_READ, MODE_REPAIR, CorruptShareError + MODE_READ, MODE_REPAIR, CorruptShareError, decrypt_privkey from allmydata.mutable.layout import SIGNED_PREFIX_LENGTH, MDMFSlotReadProxy @implementer(IServermapUpdaterStatus) @@ -943,9 +943,10 @@ class ServermapUpdater(object): writekey stored in my node. If it is valid, then I set the privkey and encprivkey properties of the node. """ - alleged_privkey_s = self._node._decrypt_privkey(enc_privkey) + node_writekey = self._node.get_writekey() + alleged_privkey_s = decrypt_privkey(node_writekey, enc_privkey) alleged_writekey = hashutil.ssk_writekey_hash(alleged_privkey_s) - if alleged_writekey != self._node.get_writekey(): + if alleged_writekey != node_writekey: self.log("invalid privkey from %r shnum %d" % (server.get_name(), shnum), parent=lp, level=log.WEIRD, umid="aJVccw") From 3423bfb351b717281a9088404f2a090534179fc4 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 3 Jan 2023 11:31:29 -0500 Subject: [PATCH 08/33] Expose the pre-constructed keypair functionality to the HTTP API --- src/allmydata/client.py | 34 ++++++++++++++++++-- src/allmydata/test/common.py | 51 ++++++++++++++++++++++++------ src/allmydata/test/web/test_web.py | 51 ++++++++++++++++++++++++++---- src/allmydata/web/filenode.py | 21 ++++++++++-- 4 files changed, 138 insertions(+), 19 deletions(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 1a158a1aa..a8238e4ee 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -32,6 +32,7 @@ from allmydata.storage.server import StorageServer, FoolscapStorageServer from allmydata import storage_client from allmydata.immutable.upload import Uploader from allmydata.immutable.offloaded import Helper +from allmydata.mutable.filenode import MutableFileNode from allmydata.introducer.client import IntroducerClient from allmydata.util import ( hashutil, base32, pollmixin, log, idlib, @@ -1086,9 +1087,38 @@ class _Client(node.Node, pollmixin.PollMixin): def create_immutable_dirnode(self, children, convergence=None): return self.nodemaker.create_immutable_directory(children, convergence) - def create_mutable_file(self, contents=None, version=None): + def create_mutable_file( + self, + contents: bytes | None = None, + version: int | None = None, + *, + unique_keypair: tuple[rsa.PublicKey, rsa.PrivateKey] | None = None, + ) -> MutableFileNode: + """ + Create *and upload* a new mutable object. + + :param contents: If given, the initial contents for the new object. + + :param version: If given, the mutable file format for the new object + (otherwise a format will be chosen automatically). + + :param unique_keypair: **Warning** This valuely independently + determines the identity of the mutable object to create. There + cannot be two different mutable objects that share a keypair. + They will merge into one object (with undefined contents). + + It is not common to pass a non-None value for this parameter. If + None is given then a new random keypair will be generated. + + If non-None, the given public/private keypair will be used for the + new object. + + :return: A Deferred which will fire with a representation of the new + mutable object after it has been uploaded. + """ return self.nodemaker.create_mutable_file(contents, - version=version) + version=version, + keypair=unique_keypair) def upload(self, uploadable, reactor=None): uploader = self.getServiceNamed("uploader") diff --git a/src/allmydata/test/common.py b/src/allmydata/test/common.py index 5728b84ad..37d390da5 100644 --- a/src/allmydata/test/common.py +++ b/src/allmydata/test/common.py @@ -105,6 +105,7 @@ from allmydata.scripts.common import ( from ..crypto import ( ed25519, + rsa, ) from .eliotutil import ( EliotLoggedRunTest, @@ -622,15 +623,28 @@ class FakeMutableFileNode(object): # type: ignore # incomplete implementation MUTABLE_SIZELIMIT = 10000 - def __init__(self, storage_broker, secret_holder, - default_encoding_parameters, history, all_contents): + _public_key: rsa.PublicKey | None + _private_key: rsa.PrivateKey | None + + def __init__(self, + storage_broker, + secret_holder, + default_encoding_parameters, + history, + all_contents, + keypair: tuple[rsa.PublicKey, rsa.PrivateKey] | None + ): self.all_contents = all_contents self.file_types = {} # storage index => MDMF_VERSION or SDMF_VERSION - self.init_from_cap(make_mutable_file_cap()) + self.init_from_cap(make_mutable_file_cap(keypair)) self._k = default_encoding_parameters['k'] self._segsize = default_encoding_parameters['max_segment_size'] - def create(self, contents, key_generator=None, keysize=None, - version=SDMF_VERSION): + if keypair is None: + self._public_key = self._private_key = None + else: + self._public_key, self._private_key = keypair + + def create(self, contents, version=SDMF_VERSION): if version == MDMF_VERSION and \ isinstance(self.my_uri, (uri.ReadonlySSKFileURI, uri.WriteableSSKFileURI)): @@ -826,9 +840,28 @@ class FakeMutableFileNode(object): # type: ignore # incomplete implementation return defer.succeed(consumer) -def make_mutable_file_cap(): - return uri.WriteableSSKFileURI(writekey=os.urandom(16), - fingerprint=os.urandom(32)) +def make_mutable_file_cap( + keypair: tuple[rsa.PublicKey, rsa.PrivateKey] | None = None, +) -> uri.WriteableSSKFileURI: + """ + Create a local representation of a mutable object. + + :param keypair: If None, a random keypair will be generated for the new + object. Otherwise, this is the keypair for that object. + """ + if keypair is None: + writekey = os.urandom(16) + fingerprint = os.urandom(32) + else: + pubkey, privkey = keypair + pubkey_s = rsa.der_string_from_verifying_key(pubkey) + privkey_s = rsa.der_string_from_signing_key(privkey) + writekey = hashutil.ssk_writekey_hash(privkey_s) + fingerprint = hashutil.ssk_pubkey_fingerprint_hash(pubkey_s) + + return uri.WriteableSSKFileURI( + writekey=writekey, fingerprint=fingerprint, + ) def make_mdmf_mutable_file_cap(): return uri.WriteableMDMFFileURI(writekey=os.urandom(16), @@ -858,7 +891,7 @@ def create_mutable_filenode(contents, mdmf=False, all_contents=None): encoding_params['max_segment_size'] = 128*1024 filenode = FakeMutableFileNode(None, None, encoding_params, None, - all_contents) + all_contents, None) filenode.init_from_cap(cap) if mdmf: filenode.create(MutableData(contents), version=MDMF_VERSION) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index c220b0a0b..bb1f27322 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -8,6 +8,7 @@ from six import ensure_binary import os.path, re, time import treq from urllib.parse import quote as urlquote, unquote as urlunquote +from base64 import urlsafe_b64encode from bs4 import BeautifulSoup @@ -32,6 +33,7 @@ from allmydata.util import fileutil, base32, hashutil, jsonbytes as json from allmydata.util.consumer import download_to_data from allmydata.util.encodingutil import to_bytes from ...util.connection_status import ConnectionStatus +from ...crypto.rsa import PublicKey, PrivateKey, create_signing_keypair, der_string_from_signing_key from ..common import ( EMPTY_CLIENT_CONFIG, FakeCHKFileNode, @@ -59,6 +61,7 @@ from allmydata.interfaces import ( MustBeReadonlyError, ) from allmydata.mutable import servermap, publish, retrieve +from allmydata.mutable.common import derive_mutable_keys from .. import common_util as testutil from ..common_util import TimezoneMixin from ..common_web import ( @@ -94,14 +97,19 @@ class FakeNodeMaker(NodeMaker): def _create_mutable(self, cap): return FakeMutableFileNode(None, None, self.encoding_params, None, - self.all_contents).init_from_cap(cap) - def create_mutable_file(self, contents=b"", keysize=None, - version=SDMF_VERSION, - keypair=None, + self.all_contents, None).init_from_cap(cap) + def create_mutable_file(self, + contents=None, + version=None, + keypair: tuple[PublicKey, PrivateKey] | None=None, ): - assert keypair is None, "FakeNodeMaker does not support externally supplied keypairs" + if contents is None: + contents = b"" + if version is None: + version = SDMF_VERSION + n = FakeMutableFileNode(None, None, self.encoding_params, None, - self.all_contents) + self.all_contents, keypair) return n.create(contents, version=version) class FakeUploader(service.Service): @@ -2865,6 +2873,37 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi "Unknown format: foo", method="post", data=body, headers=headers) + async def test_POST_upload_keypair(self) -> None: + """ + A *POST* creating a new mutable object may include a *private-key* + query argument giving a urlsafe-base64-encoded RSA private key to use + as the "signature key". The given signature key is used, rather than + a new one being generated. + """ + format = "sdmf" + priv, pub = create_signing_keypair(2048) + encoded_privkey = urlsafe_b64encode(der_string_from_signing_key(priv)).decode("ascii") + filename = "predetermined-sdmf" + actual_cap = uri.from_string(await self.POST( + self.public_url + + f"/foo?t=upload&format={format}&private-key={encoded_privkey}", + file=(filename, self.NEWFILE_CONTENTS * 100), + )) + # Ideally we would inspect the private ("signature") and public + # ("verification") keys but they are not made easily accessible here + # (ostensibly because we have a FakeMutableFileNode instead of a real + # one). + # + # So, instead, re-compute the writekey and fingerprint and compare + # those against the capability string. + expected_writekey, _, expected_fingerprint = derive_mutable_keys((pub, priv)) + self.assertEqual( + (expected_writekey, expected_fingerprint), + (actual_cap.writekey, actual_cap.fingerprint), + ) + + + def test_POST_upload_format(self): def _check_upload(ign, format, uri_prefix, fn=None): filename = format + ".txt" diff --git a/src/allmydata/web/filenode.py b/src/allmydata/web/filenode.py index 678078ba3..1b0db5045 100644 --- a/src/allmydata/web/filenode.py +++ b/src/allmydata/web/filenode.py @@ -3,8 +3,10 @@ Ported to Python 3. """ from __future__ import annotations +from base64 import urlsafe_b64decode from twisted.web import http, static +from twisted.web.iweb import IRequest from twisted.internet import defer from twisted.web.resource import ( Resource, @@ -45,6 +47,19 @@ from allmydata.web.check_results import ( ) from allmydata.web.info import MoreInfo from allmydata.util import jsonbytes as json +from allmydata.crypto.rsa import PrivateKey, PublicKey, create_signing_keypair_from_string + + +def get_keypair(request: IRequest) -> tuple[PublicKey, PrivateKey] | None: + """ + Load a keypair from a urlsafe-base64-encoded RSA private key in the + **private-key** argument of the given request, if there is one. + """ + privkey_der = get_arg(request, "private-key", None) + if privkey_der is None: + return None + privkey, pubkey = create_signing_keypair_from_string(urlsafe_b64decode(privkey_der)) + return pubkey, privkey class ReplaceMeMixin(object): @@ -54,7 +69,8 @@ class ReplaceMeMixin(object): mutable_type = get_mutable_type(file_format) if mutable_type is not None: data = MutableFileHandle(req.content) - d = client.create_mutable_file(data, version=mutable_type) + keypair = get_keypair(req) + d = client.create_mutable_file(data, version=mutable_type, unique_keypair=keypair) def _uploaded(newnode): d2 = self.parentnode.set_node(self.name, newnode, overwrite=replace) @@ -96,7 +112,8 @@ class ReplaceMeMixin(object): if file_format in ("SDMF", "MDMF"): mutable_type = get_mutable_type(file_format) uploadable = MutableFileHandle(contents.file) - d = client.create_mutable_file(uploadable, version=mutable_type) + keypair = get_keypair(req) + d = client.create_mutable_file(uploadable, version=mutable_type, unique_keypair=keypair) def _uploaded(newnode): d2 = self.parentnode.set_node(self.name, newnode, overwrite=replace) From e236cc95a56ed01dafdc874515ee6ec2727eca83 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 6 Jan 2023 15:36:08 -0500 Subject: [PATCH 09/33] Move get_keypair to a shared location --- src/allmydata/web/common.py | 24 ++++++++++++++---------- src/allmydata/web/filenode.py | 17 +---------------- 2 files changed, 15 insertions(+), 26 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index bf89044a3..2d5fe6297 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -1,15 +1,7 @@ """ Ported to Python 3. """ -from __future__ import division -from __future__ import absolute_import -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, max, min # noqa: F401 - from past.builtins import unicode as str # prevent leaking newbytes/newstr into code that can't handle it +from __future__ import annotations from six import ensure_str @@ -21,6 +13,7 @@ except ImportError: import time import json from functools import wraps +from base64 import urlsafe_b64decode from hyperlink import ( DecodedURL, @@ -94,7 +87,7 @@ from allmydata.util.encodingutil import ( to_bytes, ) from allmydata.util import abbreviate - +from allmydata.crypto.rsa import PrivateKey, PublicKey, create_signing_keypair_from_string class WebError(Exception): def __init__(self, text, code=http.BAD_REQUEST): @@ -833,3 +826,14 @@ def abbreviate_time(data): if s >= 0.001: return u"%.1fms" % (1000*s) return u"%.0fus" % (1000000*s) + +def get_keypair(request: IRequest) -> tuple[PublicKey, PrivateKey] | None: + """ + Load a keypair from a urlsafe-base64-encoded RSA private key in the + **private-key** argument of the given request, if there is one. + """ + privkey_der = get_arg(request, "private-key", None) + if privkey_der is None: + return None + privkey, pubkey = create_signing_keypair_from_string(urlsafe_b64decode(privkey_der)) + return pubkey, privkey diff --git a/src/allmydata/web/filenode.py b/src/allmydata/web/filenode.py index 1b0db5045..52ef48e1e 100644 --- a/src/allmydata/web/filenode.py +++ b/src/allmydata/web/filenode.py @@ -3,8 +3,6 @@ Ported to Python 3. """ from __future__ import annotations -from base64 import urlsafe_b64decode - from twisted.web import http, static from twisted.web.iweb import IRequest from twisted.internet import defer @@ -26,6 +24,7 @@ from allmydata.blacklist import ( ) from allmydata.web.common import ( + get_keypair, boolean_of_arg, exception_to_child, get_arg, @@ -47,20 +46,6 @@ from allmydata.web.check_results import ( ) from allmydata.web.info import MoreInfo from allmydata.util import jsonbytes as json -from allmydata.crypto.rsa import PrivateKey, PublicKey, create_signing_keypair_from_string - - -def get_keypair(request: IRequest) -> tuple[PublicKey, PrivateKey] | None: - """ - Load a keypair from a urlsafe-base64-encoded RSA private key in the - **private-key** argument of the given request, if there is one. - """ - privkey_der = get_arg(request, "private-key", None) - if privkey_der is None: - return None - privkey, pubkey = create_signing_keypair_from_string(urlsafe_b64decode(privkey_der)) - return pubkey, privkey - class ReplaceMeMixin(object): def replace_me_with_a_child(self, req, client, replace): From 3ff9c45e95be2a8144ff70e35a1e7e41ea67ded6 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 6 Jan 2023 15:40:48 -0500 Subject: [PATCH 10/33] expose the private-key feature in the `tahoe put` cli --- src/allmydata/scripts/cli.py | 15 ++++++- src/allmydata/scripts/tahoe_put.py | 41 +++++++++++------ src/allmydata/test/cli/test_put.py | 72 ++++++++++++++++++++++++++---- src/allmydata/web/filenode.py | 1 - src/allmydata/web/unlinked.py | 13 ++---- 5 files changed, 108 insertions(+), 34 deletions(-) diff --git a/src/allmydata/scripts/cli.py b/src/allmydata/scripts/cli.py index 55975b8c5..aa7644e18 100644 --- a/src/allmydata/scripts/cli.py +++ b/src/allmydata/scripts/cli.py @@ -180,10 +180,21 @@ class GetOptions(FileStoreOptions): class PutOptions(FileStoreOptions): optFlags = [ ("mutable", "m", "Create a mutable file instead of an immutable one (like --format=SDMF)"), - ] + ] + optParameters = [ ("format", None, None, "Create a file with the given format: SDMF and MDMF for mutable, CHK (default) for immutable. (case-insensitive)"), - ] + + ("private-key-path", None, None, + "***Warning*** " + "It is possible to use this option to spoil the normal security properties of mutable objects. " + "It is also possible to corrupt or destroy data with this option. " + "For mutables only, " + "this gives a file containing a PEM-encoded 2048 bit RSA private key to use as the signature key for the mutable. " + "The private key must be handled at least as strictly as the resulting capability string. " + "A single private key must not be used for more than one mutable." + ), + ] def parseArgs(self, arg1=None, arg2=None): # see Examples below diff --git a/src/allmydata/scripts/tahoe_put.py b/src/allmydata/scripts/tahoe_put.py index 1ea45e8ea..fd746a43d 100644 --- a/src/allmydata/scripts/tahoe_put.py +++ b/src/allmydata/scripts/tahoe_put.py @@ -1,23 +1,31 @@ """ -Ported to Python 3. +Implement the ``tahoe put`` command. """ -from __future__ import unicode_literals -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function - -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +from __future__ import annotations from io import BytesIO from urllib.parse import quote as url_quote +from base64 import urlsafe_b64encode +from cryptography.hazmat.primitives.serialization import load_pem_private_key + +from twisted.python.filepath import FilePath + +from allmydata.crypto.rsa import der_string_from_signing_key from allmydata.scripts.common_http import do_http, format_http_success, format_http_error from allmydata.scripts.common import get_alias, DEFAULT_ALIAS, escape_path, \ UnknownAliasError from allmydata.util.encodingutil import quote_output +def load_private_key(path: str) -> str: + """ + Load a private key from a file and return it in a format appropriate + to include in the HTTP request. + """ + privkey = load_pem_private_key(FilePath(path).getContent(), password=None) + derbytes = der_string_from_signing_key(privkey) + return urlsafe_b64encode(derbytes).decode("ascii") + def put(options): """ @param verbosity: 0, 1, or 2, meaning quiet, verbose, or very verbose @@ -29,6 +37,10 @@ def put(options): from_file = options.from_file to_file = options.to_file mutable = options['mutable'] + if options["private-key-path"] is None: + private_key = None + else: + private_key = load_private_key(options["private-key-path"]) format = options['format'] if options['quiet']: verbosity = 0 @@ -79,6 +91,12 @@ def put(options): queryargs = [] if mutable: queryargs.append("mutable=true") + if private_key is not None: + queryargs.append(f"private-key={private_key}") + else: + if private_key is not None: + raise Exception("Can only supply a private key for mutables.") + if format: queryargs.append("format=%s" % format) if queryargs: @@ -92,10 +110,7 @@ def put(options): if verbosity > 0: print("waiting for file data on stdin..", file=stderr) # We're uploading arbitrary files, so this had better be bytes: - if PY2: - stdinb = stdin - else: - stdinb = stdin.buffer + stdinb = stdin.buffer data = stdinb.read() infileobj = BytesIO(data) diff --git a/src/allmydata/test/cli/test_put.py b/src/allmydata/test/cli/test_put.py index 03306ab71..6f33a14fd 100644 --- a/src/allmydata/test/cli/test_put.py +++ b/src/allmydata/test/cli/test_put.py @@ -1,19 +1,17 @@ """ -Ported to Python 3. +Tests for the ``tahoe put`` CLI tool. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +from __future__ import annotations +from typing import Callable, Awaitable, TypeVar import os.path from twisted.trial import unittest from twisted.python import usage +from twisted.python.filepath import FilePath +from cryptography.hazmat.primitives.serialization import load_pem_private_key + +from allmydata.uri import from_string from allmydata.util import fileutil from allmydata.scripts.common import get_aliases from allmydata.scripts import cli @@ -22,6 +20,9 @@ from ..common_util import skip_if_cannot_represent_filename from allmydata.util.encodingutil import get_io_encoding from allmydata.util.fileutil import abspath_expanduser_unicode from .common import CLITestMixin +from allmydata.mutable.common import derive_mutable_keys + +T = TypeVar("T") class Put(GridTestMixin, CLITestMixin, unittest.TestCase): @@ -215,6 +216,59 @@ class Put(GridTestMixin, CLITestMixin, unittest.TestCase): return d + async def test_unlinked_mutable_specified_private_key(self) -> None: + """ + A new unlinked mutable can be created using a specified private + key. + """ + self.basedir = "cli/Put/unlinked-mutable-with-key" + await self._test_mutable_specified_key( + lambda do_cli, pempath, datapath: do_cli( + "put", "--mutable", "--private-key-path", pempath.path, + stdin=datapath.getContent(), + ), + ) + + async def test_linked_mutable_specified_private_key(self) -> None: + """ + A new linked mutable can be created using a specified private key. + """ + self.basedir = "cli/Put/linked-mutable-with-key" + await self._test_mutable_specified_key( + lambda do_cli, pempath, datapath: do_cli( + "put", "--mutable", "--private-key-path", pempath.path, datapath.path, + ), + ) + + async def _test_mutable_specified_key( + self, + run: Callable[[Callable[..., T], FilePath, FilePath], Awaitable[T]], + ) -> None: + """ + A helper for testing mutable creation. + + :param run: A function to do the creation. It is called with + ``self.do_cli`` and the path to a private key PEM file and a data + file. It returns whatever ``do_cli`` returns. + """ + self.set_up_grid(oneshare=True) + + pempath = FilePath(__file__).parent().sibling("data").child("openssl-rsa-2048.txt") + datapath = FilePath(self.basedir).child("data") + datapath.setContent(b"Hello world" * 1024) + + (rc, out, err) = await run(self.do_cli, pempath, datapath) + self.assertEqual(rc, 0, (out, err)) + cap = from_string(out.strip()) + # The capability is derived from the key we specified. + privkey = load_pem_private_key(pempath.getContent(), password=None) + pubkey = privkey.public_key() + writekey, _, fingerprint = derive_mutable_keys((pubkey, privkey)) + self.assertEqual( + (writekey, fingerprint), + (cap.writekey, cap.fingerprint), + ) + def test_mutable(self): # echo DATA1 | tahoe put --mutable - uploaded.txt # echo DATA2 | tahoe put - uploaded.txt # should modify-in-place diff --git a/src/allmydata/web/filenode.py b/src/allmydata/web/filenode.py index 52ef48e1e..680ca3331 100644 --- a/src/allmydata/web/filenode.py +++ b/src/allmydata/web/filenode.py @@ -4,7 +4,6 @@ Ported to Python 3. from __future__ import annotations from twisted.web import http, static -from twisted.web.iweb import IRequest from twisted.internet import defer from twisted.web.resource import ( Resource, diff --git a/src/allmydata/web/unlinked.py b/src/allmydata/web/unlinked.py index 425622496..2c7be6f30 100644 --- a/src/allmydata/web/unlinked.py +++ b/src/allmydata/web/unlinked.py @@ -1,14 +1,7 @@ """ Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +from __future__ import annotations from urllib.parse import quote as urlquote @@ -25,6 +18,7 @@ from twisted.web.template import ( from allmydata.immutable.upload import FileHandle from allmydata.mutable.publish import MutableFileHandle from allmydata.web.common import ( + get_keypair, get_arg, boolean_of_arg, convert_children_json, @@ -48,7 +42,8 @@ def PUTUnlinkedSSK(req, client, version): # SDMF: files are small, and we can only upload data req.content.seek(0) data = MutableFileHandle(req.content) - d = client.create_mutable_file(data, version=version) + keypair = get_keypair(req) + d = client.create_mutable_file(data, version=version, unique_keypair=keypair) d.addCallback(lambda n: n.get_uri()) return d From 1d125b7be80f4ac3300fffd6e343d41df6debde5 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 6 Jan 2023 15:51:36 -0500 Subject: [PATCH 11/33] news fragment --- newsfragments/3962.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/3962.feature diff --git a/newsfragments/3962.feature b/newsfragments/3962.feature new file mode 100644 index 000000000..86cf62781 --- /dev/null +++ b/newsfragments/3962.feature @@ -0,0 +1 @@ +Mutable objects can now be created with a pre-determined "signature key" using the ``tahoe put`` CLI or the HTTP API. This enables deterministic creation of mutable capabilities. This feature must be used with care to preserve the normal security and reliability properties. \ No newline at end of file From e829b891b300dba118a18490cfc74e356b36915c Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 6 Jan 2023 15:51:59 -0500 Subject: [PATCH 12/33] important data file ... --- src/allmydata/test/data/openssl-rsa-2048.txt | 28 ++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 src/allmydata/test/data/openssl-rsa-2048.txt diff --git a/src/allmydata/test/data/openssl-rsa-2048.txt b/src/allmydata/test/data/openssl-rsa-2048.txt new file mode 100644 index 000000000..8f989f42c --- /dev/null +++ b/src/allmydata/test/data/openssl-rsa-2048.txt @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDF1MeXulDWFO05 +YXCh8aqNc1dS1ddJRzsti4BOWuDOepUc0oCaSIcC5aR7XJ+vhX7a02mTIwvLcuEH +8sxx0BJU4jCDpRI6aAqaKJxwZx1e6AcVFJDl7vzymhvWhqHuKh0jTvwM2zONWTwV +V8m2PbDdxu0Prwdx+Mt2sDT6xHEhJj5fI/GUDUEdkhLJF6DQSulFRqqd0qP7qcI9 +fSHZbM7MywfzqFUe8J1+tk4fBh2v7gNzN1INpzh2mDtLPAtxr4ZPtEb/0D0U4PsP +CniOHP0U8sF3VY0+K5qoCQr92cLRJvT/vLpQGVNUTFdFrtbqDoFxUCyEH4FUqRDX +2mVrPo2xAgMBAAECggEAA0Ev1y5/1NTPbgytBeIIH3d+v9hwKDbHecVoMwnOVeFJ +BZpONrOToovhAc1NXH2wj4SvwYWfpJ1HR9piDAuLeKlnuUu4ffzfE0gQok4E+v4r +2yg9ZcYBs/NOetAYVwbq960tiv/adFRr71E0WqbfS3fBx8q2L3Ujkkhd98PudUhQ +izbrTvkT7q00OPCWGwgWepMlLEowUWwZehGI0MlbONg7SbRraZZmG586Iy0tpC3e +AM7wC1/ORzFqcRgTIxXizQ5RHL7S0OQPLhbEJbuwPonNjze3p0EP4wNBELZTaVOd +xeA22Py4Bh/d1q3aEgbwR7tLyA8YfEzshTaY6oV8AQKBgQD0uFo8pyWk0AWXfjzn +jV4yYyPWy8pJA6YfAJAST8m7B/JeYgGlfHxTlNZiB40DsJq08tOZv3HAubgMpFIa +reuDxPqo6/Quwdy4Syu+AFhY48KIuwuoegG/L+5qcQLE69r1w71ZV6wUvLmXYX2I +Y6nYz+OdpD1JrMIr6Js60XURsQKBgQDO8yWl7ufIDKMbQpbs0PgUQsH4FtzGcP4J +j/7/8GfhKYt6rPsrojPHUbAi1+25xBVOuhm0Zx2ku2t+xPIMJoS+15EcER1Z2iHZ +Zci9UGpJpUxGcUhG7ETF1HZv0xKHcEOl9eIIOcAP9Vd9DqnGk85gy6ti6MHe/5Tn +IMD36OQ8AQKBgQDwqE7NMM67KnslRNaeG47T3F0FQbm3XehCuqnz6BUJYcI+gQD/ +fdFB3K+LDcPmKgmqAtaGbxdtoPXXMM0xQXHHTrH15rxmMu1dK0dj/TDkkW7gSZko +YHtRSdCbSnGfuBXG9GxD7QzkA8g7j3sE4oXIGoDLqRVAW61DwubMy+jlsQKBgGNB ++Zepi1/Gt+BWQt8YpzPIhRIBnShMf3uEphCJdLlo3K4dE2btKBp8UpeTq0CDDJky +5ytAndYp0jf+K/2p59dEuyOUDdjPp5aGnA446JGkB35tzPW/Uoj0C049FVEChl+u +HBhH4peE285uXv2QXNbOOMh6zKmxOfDVI9iDyhwBAoGBAIXq2Ar0zDXXaL3ncEKo +pXt9BZ8OpJo2pvB1t2VPePOwEQ0wdT+H62fKNY47NiF9+LyS541/ps5Qhv6AmiKJ +Z7I0Vb6+sxQljYH/LNW+wc2T/pIAi/7sNcmnlBtZfoVwt99bk2CyoRALPLWHYCkh +c7Tty2bZzDZy6aCX+FGRt5N/ +-----END PRIVATE KEY----- From 2dc6466ef5c260e7cb5d46cfcbfc456399f29b71 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 6 Jan 2023 17:12:59 -0500 Subject: [PATCH 13/33] fix some errors reported by mypy --- src/allmydata/crypto/rsa.py | 6 +++--- src/allmydata/mutable/common.py | 2 +- src/allmydata/test/cli/test_put.py | 4 ++-- src/allmydata/test/common.py | 2 +- src/allmydata/test/web/test_web.py | 1 + src/allmydata/web/common.py | 19 ++++++++++++------- 6 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/allmydata/crypto/rsa.py b/src/allmydata/crypto/rsa.py index 5acc59ab2..f16b1b95f 100644 --- a/src/allmydata/crypto/rsa.py +++ b/src/allmydata/crypto/rsa.py @@ -13,7 +13,7 @@ on any of their methods. from __future__ import annotations -from typing import TypeVar +from typing_extensions import TypeAlias from cryptography.exceptions import InvalidSignature from cryptography.hazmat.backends import default_backend @@ -24,8 +24,8 @@ from cryptography.hazmat.primitives.serialization import load_der_private_key, l from allmydata.crypto.error import BadSignature -PublicKey = TypeVar("PublicKey", bound=rsa.RSAPublicKey) -PrivateKey = TypeVar("PrivateKey", bound=rsa.RSAPrivateKey) +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. diff --git a/src/allmydata/mutable/common.py b/src/allmydata/mutable/common.py index 33a1c2731..a498ab02a 100644 --- a/src/allmydata/mutable/common.py +++ b/src/allmydata/mutable/common.py @@ -66,7 +66,7 @@ class UnknownVersionError(BadShareError): """The share we received was of a version we don't recognize.""" -def encrypt_privkey(writekey: bytes, privkey: rsa.PrivateKey) -> bytes: +def encrypt_privkey(writekey: bytes, privkey: bytes) -> bytes: """ For SSK, encrypt a private ("signature") key using the writekey. """ diff --git a/src/allmydata/test/cli/test_put.py b/src/allmydata/test/cli/test_put.py index 6f33a14fd..98407bb7e 100644 --- a/src/allmydata/test/cli/test_put.py +++ b/src/allmydata/test/cli/test_put.py @@ -3,7 +3,7 @@ Tests for the ``tahoe put`` CLI tool. """ from __future__ import annotations -from typing import Callable, Awaitable, TypeVar +from typing import Callable, Awaitable, TypeVar, Any import os.path from twisted.trial import unittest from twisted.python import usage @@ -242,7 +242,7 @@ class Put(GridTestMixin, CLITestMixin, unittest.TestCase): async def _test_mutable_specified_key( self, - run: Callable[[Callable[..., T], FilePath, FilePath], Awaitable[T]], + run: Callable[[Any, FilePath, FilePath], Awaitable[tuple[int, bytes, bytes]]], ) -> None: """ A helper for testing mutable creation. diff --git a/src/allmydata/test/common.py b/src/allmydata/test/common.py index 37d390da5..db2921e86 100644 --- a/src/allmydata/test/common.py +++ b/src/allmydata/test/common.py @@ -635,7 +635,7 @@ class FakeMutableFileNode(object): # type: ignore # incomplete implementation keypair: tuple[rsa.PublicKey, rsa.PrivateKey] | None ): self.all_contents = all_contents - self.file_types = {} # storage index => MDMF_VERSION or SDMF_VERSION + self.file_types: dict[bytes, int] = {} # storage index => MDMF_VERSION or SDMF_VERSION self.init_from_cap(make_mutable_file_cap(keypair)) self._k = default_encoding_parameters['k'] self._segsize = default_encoding_parameters['max_segment_size'] diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index bb1f27322..7793c023c 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -90,6 +90,7 @@ class FakeNodeMaker(NodeMaker): 'happy': 7, 'max_segment_size':128*1024 # 1024=KiB } + all_contents: dict[bytes, object] def _create_lit(self, cap): return FakeCHKFileNode(cap, self.all_contents) def _create_immutable(self, cap): diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 2d5fe6297..25e9e51f3 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -6,7 +6,7 @@ from __future__ import annotations from six import ensure_str try: - from typing import Optional, Union, Tuple, Any + from typing import Optional, Union, Tuple, Any, TypeVar except ImportError: pass @@ -706,8 +706,9 @@ def url_for_string(req, url_string): ) return url +T = TypeVar("T") -def get_arg(req, argname, default=None, multiple=False): # type: (IRequest, Union[bytes,str], Any, bool) -> Union[bytes,Tuple[bytes],Any] +def get_arg(req: IRequest, argname: str | bytes, default: T = None, multiple: bool = False) -> Union[bytes, tuple[bytes, ...], T]: """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 @@ -719,13 +720,17 @@ def get_arg(req, argname, default=None, multiple=False): # type: (IRequest, Uni :return: Either bytes or tuple of bytes. """ if isinstance(argname, str): - argname = argname.encode("utf-8") + argname_bytes = argname.encode("utf-8") + else: + argname_bytes = argname + if isinstance(default, str): default = default.encode("utf-8") + results = [] - if argname in req.args: - results.extend(req.args[argname]) - argname_unicode = str(argname, "utf-8") + if argname_bytes in req.args: + results.extend(req.args[argname_bytes]) + argname_unicode = str(argname_bytes, "utf-8") if req.fields and argname_unicode in req.fields: value = req.fields[argname_unicode].value if isinstance(value, str): @@ -832,7 +837,7 @@ def get_keypair(request: IRequest) -> tuple[PublicKey, PrivateKey] | None: Load a keypair from a urlsafe-base64-encoded RSA private key in the **private-key** argument of the given request, if there is one. """ - privkey_der = get_arg(request, "private-key", None) + privkey_der = get_arg(request, "private-key", default=None, multiple=False) if privkey_der is None: return None privkey, pubkey = create_signing_keypair_from_string(urlsafe_b64decode(privkey_der)) From a806b2fabaf985a28d9872ea2db292a30c91810f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 6 Jan 2023 18:11:47 -0500 Subject: [PATCH 14/33] Fix some more mypy errors --- src/allmydata/crypto/rsa.py | 22 +++++++++++++++------- src/allmydata/web/common.py | 16 +++++++++++----- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/allmydata/crypto/rsa.py b/src/allmydata/crypto/rsa.py index 3554ec557..f1ba0d10a 100644 --- a/src/allmydata/crypto/rsa.py +++ b/src/allmydata/crypto/rsa.py @@ -14,6 +14,7 @@ on any of their methods. from __future__ import annotations from typing_extensions import TypeAlias +from typing import Callable from functools import partial @@ -70,22 +71,29 @@ def create_signing_keypair_from_string(private_key_der: bytes) -> tuple[PrivateK :returns: 2-tuple of (private_key, public_key) """ - load = partial( - load_der_private_key, + _load = partial( + load_der_public_key, private_key_der, password=None, backend=default_backend(), ) + def load_with_validation() -> PrivateKey: + return _load() + + def load_without_validation() -> PrivateKey: + return _load(unsafe_skip_rsa_key_validation=True) + + # 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: - # 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. - unsafe_priv_key = load(unsafe_skip_rsa_key_validation=True) + unsafe_priv_key = load() except TypeError: # cryptography<39 does not support this parameter, so just load the # key with validation... - unsafe_priv_key = load() + unsafe_priv_key = load_without_validation() # But avoid *reloading* it since that will run the expensive # validation *again*. load = lambda: unsafe_priv_key diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 25e9e51f3..8f81aec94 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -6,7 +6,7 @@ from __future__ import annotations from six import ensure_str try: - from typing import Optional, Union, Tuple, Any, TypeVar + from typing import Optional, Union, Tuple, Any, TypeVar, Literal, overload except ImportError: pass @@ -708,7 +708,13 @@ def url_for_string(req, url_string): T = TypeVar("T") -def get_arg(req: IRequest, argname: str | bytes, default: T = None, multiple: bool = False) -> Union[bytes, tuple[bytes, ...], T]: +@overload +def get_arg(req: IRequest, argname: str | bytes, default: T = None, *, multiple: Literal[False] = False) -> bytes: ... + +@overload +def get_arg(req: IRequest, argname: str | bytes, default: T = None, *, multiple: Literal[True]) -> tuple[bytes, ...]: ... + +def get_arg(req: IRequest, argname: str | bytes, default: T | None = 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 @@ -724,9 +730,6 @@ def get_arg(req: IRequest, argname: str | bytes, default: T = None, multiple: bo else: argname_bytes = argname - if isinstance(default, str): - default = default.encode("utf-8") - results = [] if argname_bytes in req.args: results.extend(req.args[argname_bytes]) @@ -740,6 +743,9 @@ def get_arg(req: IRequest, argname: str | bytes, default: T = None, multiple: bo return tuple(results) if results: return results[0] + + if isinstance(default, str): + return default.encode("utf-8") return default From c9e23dea13a51114f47c8c64ed1caf34d57cfcdc Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 6 Jan 2023 20:59:48 -0500 Subject: [PATCH 15/33] we should always be able to get these and we always need overload now --- src/allmydata/web/common.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 8f81aec94..b55a49d4d 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -5,10 +5,7 @@ from __future__ import annotations from six import ensure_str -try: - from typing import Optional, Union, Tuple, Any, TypeVar, Literal, overload -except ImportError: - pass +from typing import Optional, Union, TypeVar, Literal, overload import time import json From 85234b07a0ce4f12be3676562a0ed7a24f650d27 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 6 Jan 2023 21:00:04 -0500 Subject: [PATCH 16/33] load the right kind of key! --- src/allmydata/crypto/rsa.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/crypto/rsa.py b/src/allmydata/crypto/rsa.py index f1ba0d10a..b8de52c4f 100644 --- a/src/allmydata/crypto/rsa.py +++ b/src/allmydata/crypto/rsa.py @@ -72,7 +72,7 @@ def create_signing_keypair_from_string(private_key_der: bytes) -> tuple[PrivateK :returns: 2-tuple of (private_key, public_key) """ _load = partial( - load_der_public_key, + load_der_private_key, private_key_der, password=None, backend=default_backend(), From 8c56ccad725ebb6c9ae745a60d6258ed0f8b5b2f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 6 Jan 2023 21:00:10 -0500 Subject: [PATCH 17/33] fall back to *with* validation, not without --- src/allmydata/crypto/rsa.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/crypto/rsa.py b/src/allmydata/crypto/rsa.py index b8de52c4f..c21b522cd 100644 --- a/src/allmydata/crypto/rsa.py +++ b/src/allmydata/crypto/rsa.py @@ -93,7 +93,7 @@ def create_signing_keypair_from_string(private_key_der: bytes) -> tuple[PrivateK except TypeError: # cryptography<39 does not support this parameter, so just load the # key with validation... - unsafe_priv_key = load_without_validation() + unsafe_priv_key = load_with_validation() # But avoid *reloading* it since that will run the expensive # validation *again*. load = lambda: unsafe_priv_key From e893d06cb35fd7c32a8682b3f538086c663f30de Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 6 Jan 2023 21:00:21 -0500 Subject: [PATCH 18/33] RSAPrivateKey certainly does have this method I don't know why mypy fails to see it. --- src/allmydata/crypto/rsa.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/crypto/rsa.py b/src/allmydata/crypto/rsa.py index c21b522cd..3ad893dbf 100644 --- a/src/allmydata/crypto/rsa.py +++ b/src/allmydata/crypto/rsa.py @@ -123,7 +123,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( + return private_key.private_bytes( # type: ignore[attr-defined] encoding=Encoding.DER, format=PrivateFormat.PKCS8, encryption_algorithm=NoEncryption(), From 3ce5ee6f0304131a5cc4ebbbed2237842084f3d2 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Sat, 7 Jan 2023 07:17:40 -0500 Subject: [PATCH 19/33] get Literal from somewhere it is more likely to be --- src/allmydata/web/common.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index b55a49d4d..470170e7d 100644 --- a/src/allmydata/web/common.py +++ b/src/allmydata/web/common.py @@ -5,7 +5,8 @@ from __future__ import annotations from six import ensure_str -from typing import Optional, Union, TypeVar, Literal, overload +from typing import Optional, Union, TypeVar, overload +from typing_extensions import Literal import time import json From 98624f3d6a32882d5fb2283fa61690226e81299e Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 12 Jan 2023 09:53:07 -0500 Subject: [PATCH 20/33] Attempt to workaround for 3960. --- .github/workflows/ci.yml | 9 +++++++++ newsfragments/3960.minor | 0 2 files changed, 9 insertions(+) create mode 100644 newsfragments/3960.minor diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 80b312008..2a8f5246e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -86,8 +86,17 @@ jobs: run: python misc/build_helpers/show-tool-versions.py - name: Run tox for corresponding Python version + if: ${{ !contains(matrix.os, 'windows') }} run: python -m tox + # On Windows, a non-blocking pipe might respond (when emulating Unix-y + # API) with ENOSPC to indicate buffer full. Trial doesn't handle this + # well. So, we pipe the output through Get-Content (similar to piping + # through cat) to make buffer handling someone else's problem. + - name: Run tox for corresponding Python version + if: ${{ contains(matrix.os, 'windows') }} + run: python -m tox | Get-Content + - name: Upload eliot.log uses: actions/upload-artifact@v3 with: diff --git a/newsfragments/3960.minor b/newsfragments/3960.minor new file mode 100644 index 000000000..e69de29bb From e142f051b497124cb8828106242d14cb46643431 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 12 Jan 2023 10:01:09 -0500 Subject: [PATCH 21/33] Cats solve problems, right? --- .github/workflows/ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2a8f5246e..1b8ab0d57 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -91,11 +91,11 @@ jobs: # On Windows, a non-blocking pipe might respond (when emulating Unix-y # API) with ENOSPC to indicate buffer full. Trial doesn't handle this - # well. So, we pipe the output through Get-Content (similar to piping - # through cat) to make buffer handling someone else's problem. + # well. So, we pipe the output through cat to make buffer handling someone + # else's problem. - name: Run tox for corresponding Python version if: ${{ contains(matrix.os, 'windows') }} - run: python -m tox | Get-Content + run: python -m tox | cat - name: Upload eliot.log uses: actions/upload-artifact@v3 From dd89ca6d4f2738f92534f0b33da2a365b9f30016 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 12 Jan 2023 10:36:39 -0500 Subject: [PATCH 22/33] Another approach. --- .github/workflows/ci.yml | 8 ++++--- misc/windows-enospc/passthrough.py | 38 ++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 misc/windows-enospc/passthrough.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1b8ab0d57..1eb12bdd5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -91,11 +91,13 @@ jobs: # On Windows, a non-blocking pipe might respond (when emulating Unix-y # API) with ENOSPC to indicate buffer full. Trial doesn't handle this - # well. So, we pipe the output through cat to make buffer handling someone - # else's problem. + # well. So, we pipe the output through pipethrough that will hopefully be + # able to do the right thing by using Windows APIs. - name: Run tox for corresponding Python version if: ${{ contains(matrix.os, 'windows') }} - run: python -m tox | cat + run: | + pip install twisted + python -m tox | python misc/windows-enospc/passthrough.py - name: Upload eliot.log uses: actions/upload-artifact@v3 diff --git a/misc/windows-enospc/passthrough.py b/misc/windows-enospc/passthrough.py new file mode 100644 index 000000000..6be3d7dbe --- /dev/null +++ b/misc/windows-enospc/passthrough.py @@ -0,0 +1,38 @@ +""" +Writing to non-blocking pipe can result in ENOSPC when using Unix APIs on +Windows. So, this program passes through data from stdin to stdout, using +Windows APIs instead of Unix-y APIs. +""" + +import sys + +from twisted.internet.stdio import StandardIO +from twisted.internet import reactor +from twisted.internet.protocol import Protocol +from twisted.internet.interfaces import IHalfCloseableProtocol +from twisted.internet.error import ReactorNotRunning +from zope.interface import implementer + +@implementer(IHalfCloseableProtocol) +class Passthrough(Protocol): + def readConnectionLost(self): + self.transport.loseConnection() + + def writeConnectionLost(self): + try: + reactor.stop() + except ReactorNotRunning: + pass + + def dataReceived(self, data): + self.transport.write(data) + + def connectionLost(self, reason): + try: + reactor.stop() + except ReactorNotRunning: + pass + + +std = StandardIO(Passthrough()) +reactor.run() From e997bb9c398d191c28c7a82840a2df3bbbb657e9 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 12 Jan 2023 10:39:50 -0500 Subject: [PATCH 23/33] Also need pywin32. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1eb12bdd5..0f8040e96 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -96,7 +96,7 @@ jobs: - name: Run tox for corresponding Python version if: ${{ contains(matrix.os, 'windows') }} run: | - pip install twisted + pip install twisted pywin32 python -m tox | python misc/windows-enospc/passthrough.py - name: Upload eliot.log From ee5ad549fed7150d2a6782ca4409b27c5821d546 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 12 Jan 2023 11:12:00 -0500 Subject: [PATCH 24/33] Clarify. --- .github/workflows/ci.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0f8040e96..011bfc1ea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -91,8 +91,9 @@ jobs: # On Windows, a non-blocking pipe might respond (when emulating Unix-y # API) with ENOSPC to indicate buffer full. Trial doesn't handle this - # well. So, we pipe the output through pipethrough that will hopefully be - # able to do the right thing by using Windows APIs. + # well, so it breaks test runs. To attempt to solve this, we pipe the + # output through pipethrough that will hopefully be able to do the right + # thing by using Windows APIs. - name: Run tox for corresponding Python version if: ${{ contains(matrix.os, 'windows') }} run: | From a765d8a35b8d0bdbe6163585dcdd96f4d6f7eddf Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Thu, 12 Jan 2023 11:18:05 -0500 Subject: [PATCH 25/33] Unused. --- misc/windows-enospc/passthrough.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/misc/windows-enospc/passthrough.py b/misc/windows-enospc/passthrough.py index 6be3d7dbe..1d4cd48bb 100644 --- a/misc/windows-enospc/passthrough.py +++ b/misc/windows-enospc/passthrough.py @@ -4,8 +4,6 @@ Windows. So, this program passes through data from stdin to stdout, using Windows APIs instead of Unix-y APIs. """ -import sys - from twisted.internet.stdio import StandardIO from twisted.internet import reactor from twisted.internet.protocol import Protocol From 2490f0f58ac3b170cf768d7e40cabebbb699c4db Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 12 Jan 2023 15:33:37 -0500 Subject: [PATCH 26/33] some minor rationalization of the return type --- src/allmydata/web/common.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/allmydata/web/common.py b/src/allmydata/web/common.py index 470170e7d..c49354217 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) -> bytes: ... +def get_arg(req: IRequest, argname: str | bytes, default: T = None, *, multiple: Literal[False] = False) -> T | bytes: ... @overload -def get_arg(req: IRequest, argname: str | bytes, default: T = None, *, multiple: Literal[True]) -> tuple[bytes, ...]: ... +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: T | None = None, *, multiple: bool = False) -> None | T | bytes | tuple[bytes, ...]: +def get_arg(req: IRequest, argname: str | bytes, default: 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 From 2d23e2e6401e5c696002e9eefadd30193a39201f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 12 Jan 2023 15:37:07 -0500 Subject: [PATCH 27/33] some doc improvements --- src/allmydata/client.py | 16 +++++++++------- src/allmydata/scripts/cli.py | 1 + 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index a8238e4ee..73672f30a 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -1102,16 +1102,18 @@ class _Client(node.Node, pollmixin.PollMixin): :param version: If given, the mutable file format for the new object (otherwise a format will be chosen automatically). - :param unique_keypair: **Warning** This valuely independently - determines the identity of the mutable object to create. There - cannot be two different mutable objects that share a keypair. - They will merge into one object (with undefined contents). + :param unique_keypair: **Warning** This value independently determines + the identity of the mutable object to create. There cannot be two + different mutable objects that share a keypair. They will merge + into one object (with undefined contents). - It is not common to pass a non-None value for this parameter. If - None is given then a new random keypair will be generated. + It is common to pass a None value (or not pass a valuye) for this + parameter. In these cases, a new random keypair will be + generated. If non-None, the given public/private keypair will be used for the - new object. + new object. The expected use-case is for implementing compliance + tests. :return: A Deferred which will fire with a representation of the new mutable object after it has been uploaded. diff --git a/src/allmydata/scripts/cli.py b/src/allmydata/scripts/cli.py index aa7644e18..579b37906 100644 --- a/src/allmydata/scripts/cli.py +++ b/src/allmydata/scripts/cli.py @@ -189,6 +189,7 @@ class PutOptions(FileStoreOptions): "***Warning*** " "It is possible to use this option to spoil the normal security properties of mutable objects. " "It is also possible to corrupt or destroy data with this option. " + "Most users will not need this option and can ignore it. " "For mutables only, " "this gives a file containing a PEM-encoded 2048 bit RSA private key to use as the signature key for the mutable. " "The private key must be handled at least as strictly as the resulting capability string. " From e6ef45d3371321e5a4667e43a3d53223bdc153c1 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 12 Jan 2023 15:37:12 -0500 Subject: [PATCH 28/33] test that we can also download the mutable --- src/allmydata/test/cli/test_put.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/allmydata/test/cli/test_put.py b/src/allmydata/test/cli/test_put.py index 98407bb7e..e8f83188a 100644 --- a/src/allmydata/test/cli/test_put.py +++ b/src/allmydata/test/cli/test_put.py @@ -268,6 +268,11 @@ class Put(GridTestMixin, CLITestMixin, unittest.TestCase): (writekey, fingerprint), (cap.writekey, cap.fingerprint), ) + # Also the capability we were given actually refers to the data we + # uploaded. + (rc, out, err) = await self.do_cli("get", out.strip()) + self.assertEqual(rc, 0, (out, err)) + self.assertEqual(out, datapath.getContent().decode("ascii")) def test_mutable(self): # echo DATA1 | tahoe put --mutable - uploaded.txt From 47ec418f7afcc745e7b0376e9485812351efab1a Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 12 Jan 2023 15:43:54 -0500 Subject: [PATCH 29/33] Test that we can also download the mutable data via the web interface --- src/allmydata/test/web/test_web.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/web/test_web.py b/src/allmydata/test/web/test_web.py index 7793c023c..4c828817a 100644 --- a/src/allmydata/test/web/test_web.py +++ b/src/allmydata/test/web/test_web.py @@ -2885,10 +2885,11 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi priv, pub = create_signing_keypair(2048) encoded_privkey = urlsafe_b64encode(der_string_from_signing_key(priv)).decode("ascii") filename = "predetermined-sdmf" + expected_content = self.NEWFILE_CONTENTS * 100 actual_cap = uri.from_string(await self.POST( self.public_url + f"/foo?t=upload&format={format}&private-key={encoded_privkey}", - file=(filename, self.NEWFILE_CONTENTS * 100), + file=(filename, expected_content), )) # Ideally we would inspect the private ("signature") and public # ("verification") keys but they are not made easily accessible here @@ -2903,7 +2904,10 @@ class Web(WebMixin, WebErrorMixin, testutil.StallMixin, testutil.ReallyEqualMixi (actual_cap.writekey, actual_cap.fingerprint), ) - + # And the capability we got can be used to download the data we + # uploaded. + downloaded_content = await self.GET(f"/uri/{actual_cap.to_string().decode('ascii')}") + self.assertEqual(expected_content, downloaded_content) def test_POST_upload_format(self): def _check_upload(ign, format, uri_prefix, fn=None): From c856f1aa298a10ea91707fcce91899931a8924c0 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 12 Jan 2023 16:16:55 -0500 Subject: [PATCH 30/33] Censor private key values in the HTTP log, too. --- src/allmydata/test/web/test_webish.py | 10 +++ src/allmydata/webish.py | 98 +++++++++++++++------------ 2 files changed, 64 insertions(+), 44 deletions(-) diff --git a/src/allmydata/test/web/test_webish.py b/src/allmydata/test/web/test_webish.py index 4a77d21ae..050f77d1c 100644 --- a/src/allmydata/test/web/test_webish.py +++ b/src/allmydata/test/web/test_webish.py @@ -202,6 +202,16 @@ class TahoeLAFSSiteTests(SyncTestCase): ), ) + def test_private_key_censoring(self): + """ + The log event for a request including a **private-key** query + argument has the private key value censored. + """ + self._test_censoring( + b"/uri?uri=URI:CHK:aaa:bbb&private-key=AAAAaaaabbbb==", + b"/uri?uri=[CENSORED]&private-key=[CENSORED]", + ) + def test_uri_censoring(self): """ The log event for a request for **/uri/** has the capability value diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index 519b3e1f0..1b2b8192a 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -1,18 +1,12 @@ """ -Ported to Python 3. +General web server-related utilities. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +from __future__ import annotations from six import ensure_str import re, time, tempfile +from urllib.parse import parse_qsl, urlencode from cgi import ( FieldStorage, @@ -45,40 +39,37 @@ from .web.storage_plugins import ( ) -if PY2: - FileUploadFieldStorage = FieldStorage -else: - class FileUploadFieldStorage(FieldStorage): - """ - Do terrible things to ensure files are still bytes. +class FileUploadFieldStorage(FieldStorage): + """ + Do terrible things to ensure files are still bytes. - On Python 2, uploaded files were always bytes. On Python 3, there's a - heuristic: if the filename is set on a field, it's assumed to be a file - upload and therefore bytes. If no filename is set, it's Unicode. + On Python 2, uploaded files were always bytes. On Python 3, there's a + heuristic: if the filename is set on a field, it's assumed to be a file + upload and therefore bytes. If no filename is set, it's Unicode. - Unfortunately, we always want it to be bytes, and Tahoe-LAFS also - enables setting the filename not via the MIME filename, but via a - separate field called "name". + Unfortunately, we always want it to be bytes, and Tahoe-LAFS also + enables setting the filename not via the MIME filename, but via a + separate field called "name". - Thus we need to do this ridiculous workaround. Mypy doesn't like it - either, thus the ``# type: ignore`` below. + Thus we need to do this ridiculous workaround. Mypy doesn't like it + either, thus the ``# type: ignore`` below. - Source for idea: - https://mail.python.org/pipermail/python-dev/2017-February/147402.html - """ - @property # type: ignore - def filename(self): - if self.name == "file" and not self._mime_filename: - # We use the file field to upload files, see directory.py's - # _POST_upload. Lack of _mime_filename means we need to trick - # FieldStorage into thinking there is a filename so it'll - # return bytes. - return "unknown-filename" - return self._mime_filename + Source for idea: + https://mail.python.org/pipermail/python-dev/2017-February/147402.html + """ + @property # type: ignore + def filename(self): + if self.name == "file" and not self._mime_filename: + # We use the file field to upload files, see directory.py's + # _POST_upload. Lack of _mime_filename means we need to trick + # FieldStorage into thinking there is a filename so it'll + # return bytes. + return "unknown-filename" + return self._mime_filename - @filename.setter - def filename(self, value): - self._mime_filename = value + @filename.setter + def filename(self, value): + self._mime_filename = value class TahoeLAFSRequest(Request, object): @@ -180,12 +171,7 @@ def _logFormatter(logDateTime, request): queryargs = b"" else: path, queryargs = x - # there is a form handler which redirects POST /uri?uri=FOO into - # GET /uri/FOO so folks can paste in non-HTTP-prefixed uris. Make - # sure we censor these too. - if queryargs.startswith(b"uri="): - queryargs = b"uri=[CENSORED]" - queryargs = b"?" + queryargs + queryargs = b"?" + censor(queryargs) if path.startswith(b"/uri/"): path = b"/uri/[CENSORED]" elif path.startswith(b"/file/"): @@ -207,6 +193,30 @@ def _logFormatter(logDateTime, request): ) +def censor(queryargs: bytes) -> bytes: + """ + Replace potentially sensitive values in query arguments with a + constant string. + """ + args = parse_qsl(queryargs.decode("ascii"), keep_blank_values=True, encoding="utf8") + result = [] + for k, v in args: + if k == "uri": + # there is a form handler which redirects POST /uri?uri=FOO into + # GET /uri/FOO so folks can paste in non-HTTP-prefixed uris. Make + # sure we censor these. + v = "[CENSORED]" + elif k == "private-key": + # Likewise, sometimes a private key is supplied with mutable + # creation. + v = "[CENSORED]" + + result.append((k, v)) + + # Customize safe to try to leave our markers intact. + return urlencode(result, safe="[]").encode("ascii") + + class TahoeLAFSSite(Site, object): """ The HTTP protocol factory used by Tahoe-LAFS. From 1a807a0232996447c47dd095a13498488e16d541 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Thu, 12 Jan 2023 16:32:32 -0500 Subject: [PATCH 31/33] mollify the type checker --- src/allmydata/crypto/rsa.py | 9 +++++++-- src/allmydata/scripts/tahoe_put.py | 3 ++- src/allmydata/test/cli/test_put.py | 2 ++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/allmydata/crypto/rsa.py b/src/allmydata/crypto/rsa.py index 3ad893dbf..e579a3d2a 100644 --- a/src/allmydata/crypto/rsa.py +++ b/src/allmydata/crypto/rsa.py @@ -79,10 +79,14 @@ def create_signing_keypair_from_string(private_key_der: bytes) -> tuple[PrivateK ) def load_with_validation() -> PrivateKey: - return _load() + k = _load() + assert isinstance(k, PrivateKey) + return k def load_without_validation() -> PrivateKey: - return _load(unsafe_skip_rsa_key_validation=True) + 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 @@ -159,6 +163,7 @@ def create_verifying_key_from_string(public_key_der: bytes) -> PublicKey: public_key_der, backend=default_backend(), ) + assert isinstance(pub_key, PublicKey) return pub_key diff --git a/src/allmydata/scripts/tahoe_put.py b/src/allmydata/scripts/tahoe_put.py index fd746a43d..c04b6b4bc 100644 --- a/src/allmydata/scripts/tahoe_put.py +++ b/src/allmydata/scripts/tahoe_put.py @@ -11,7 +11,7 @@ from cryptography.hazmat.primitives.serialization import load_pem_private_key from twisted.python.filepath import FilePath -from allmydata.crypto.rsa import der_string_from_signing_key +from allmydata.crypto.rsa import PrivateKey, der_string_from_signing_key from allmydata.scripts.common_http import do_http, format_http_success, format_http_error from allmydata.scripts.common import get_alias, DEFAULT_ALIAS, escape_path, \ UnknownAliasError @@ -23,6 +23,7 @@ def load_private_key(path: str) -> str: to include in the HTTP request. """ privkey = load_pem_private_key(FilePath(path).getContent(), password=None) + assert isinstance(privkey, PrivateKey) derbytes = der_string_from_signing_key(privkey) return urlsafe_b64encode(derbytes).decode("ascii") diff --git a/src/allmydata/test/cli/test_put.py b/src/allmydata/test/cli/test_put.py index e8f83188a..c5f32a553 100644 --- a/src/allmydata/test/cli/test_put.py +++ b/src/allmydata/test/cli/test_put.py @@ -11,6 +11,7 @@ from twisted.python.filepath import FilePath from cryptography.hazmat.primitives.serialization import load_pem_private_key +from allmydata.crypto.rsa import PrivateKey from allmydata.uri import from_string from allmydata.util import fileutil from allmydata.scripts.common import get_aliases @@ -262,6 +263,7 @@ class Put(GridTestMixin, CLITestMixin, unittest.TestCase): cap = from_string(out.strip()) # The capability is derived from the key we specified. privkey = load_pem_private_key(pempath.getContent(), password=None) + assert isinstance(privkey, PrivateKey) pubkey = privkey.public_key() writekey, _, fingerprint = derive_mutable_keys((pubkey, privkey)) self.assertEqual( From 0eee22cccf8af1a95b176c2a91b4de91539f9411 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 13 Jan 2023 09:53:38 -0500 Subject: [PATCH 32/33] Pin older charset_normalizer. --- newsfragments/3966.bugfix | 1 + setup.py | 7 +++++++ 2 files changed, 8 insertions(+) create mode 100644 newsfragments/3966.bugfix diff --git a/newsfragments/3966.bugfix b/newsfragments/3966.bugfix new file mode 100644 index 000000000..ead94c47c --- /dev/null +++ b/newsfragments/3966.bugfix @@ -0,0 +1 @@ +Fix incompatibility with newer versions of the transitive charset_normalizer dependency when using PyInstaller. \ No newline at end of file diff --git a/setup.py b/setup.py index 3681dc441..a9b42d522 100644 --- a/setup.py +++ b/setup.py @@ -148,6 +148,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 e64c6b02e6370290a9c6d9ca17257b2e736f693a Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 13 Jan 2023 10:29:22 -0500 Subject: [PATCH 33/33] Fix a typo --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 011bfc1ea..4d67f09bd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -92,7 +92,7 @@ jobs: # On Windows, a non-blocking pipe might respond (when emulating Unix-y # API) with ENOSPC to indicate buffer full. Trial doesn't handle this # well, so it breaks test runs. To attempt to solve this, we pipe the - # output through pipethrough that will hopefully be able to do the right + # output through passthrough.py that will hopefully be able to do the right # thing by using Windows APIs. - name: Run tox for corresponding Python version if: ${{ contains(matrix.os, 'windows') }}