Script 'mail_helper' called by obssrc Hello community, here is the log from the commit of package python-patatt for openSUSE:Factory checked in at 2021-06-11 00:18:44 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/python-patatt (Old) and /work/SRC/openSUSE:Factory/.python-patatt.new.32437 (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "python-patatt" Fri Jun 11 00:18:44 2021 rev:3 rq:898975 version:0.4.6 Changes: -------- --- /work/SRC/openSUSE:Factory/python-patatt/python-patatt.changes 2021-06-04 00:34:07.760969068 +0200 +++ /work/SRC/openSUSE:Factory/.python-patatt.new.32437/python-patatt.changes 2021-06-11 00:19:18.769383542 +0200 @@ -1,0 +2,10 @@ +Thu Jun 10 04:23:09 UTC 2021 - Jiri Slaby <[email protected]> + +- update to 0.4.6 + * Catch NoKeyError before ValidationError + * Throw a NoKeyError when no matching PGP key + * Make instructions for automatic signing more reliable + * Handle MIME encoded-word & other header manglings + * Add "frequently seen commentary" + +------------------------------------------------------------------- Old: ---- patatt-0.4.4.tar.asc patatt-0.4.4.tar.gz New: ---- patatt-0.4.6.tar.asc patatt-0.4.6.tar.gz ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ python-patatt.spec ++++++ --- /var/tmp/diff_new_pack.emU1eQ/_old 2021-06-11 00:19:19.241384362 +0200 +++ /var/tmp/diff_new_pack.emU1eQ/_new 2021-06-11 00:19:19.241384362 +0200 @@ -19,7 +19,7 @@ %define skip_python2 1 %{?!python_module:%define python_module() python-%{**} python3-%{**}} Name: python-patatt -Version: 0.4.4 +Version: 0.4.6 Release: 0 Summary: Cryptographic patch attestation for the masses License: MIT-0 ++++++ patatt-0.4.4.tar.gz -> patatt-0.4.6.tar.gz ++++++ diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/patatt-0.4.4/README.rst new/patatt-0.4.6/README.rst --- old/patatt-0.4.4/README.rst 2021-05-28 16:10:52.000000000 +0200 +++ new/patatt-0.4.6/README.rst 2021-06-09 21:00:01.000000000 +0200 @@ -184,8 +184,8 @@ If everything is working well, you can start automatically signing all outgoing patches sent via git-send-email:: - $ echo 'patatt sign --hook "${1}"' > .git/hooks/sendemail-validate - $ chmod a+x .git/hooks/sendemail-validate + $ echo 'patatt sign --hook "${1}"' > "$(git rev-parse --git-dir)/hooks/sendemail-validate" + $ chmod a+x "$(git rev-parse --git-dir)/hooks/sendemail-validate" PGP vs ed25519 keys considerations ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -400,3 +400,96 @@ certificate of contribution and should include a Signed-off-by: line. Please read the DCO file for full legal definition of what that implies. +Frequently seen commentary +-------------------------- +Why is this library even needed? Why not... + +Why not simply PGP-sign all patches? +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +PGP-signing patches causes important problems for reviewers. If a patch +is inline-signed, then this not only adds textual headers/footers, but +adds additional escaping in the protected body, converting all '^-' +sequences into '^- -', which corrupts patches. + +MIME-signing is better, but has several other downsides: + +- messages are now sent as multipart mime structures, which causes some + tooling to no longer properly handle the patch content +- the signature attachments may be stripped/quarantined by email + gateways that don't properly recognize OpenPGP mime signatures +- the From/Subject headers are rarely included into protected content, + even though they are crucial parts of what ends up going into a git + commit + +These considerations have resulted in many projects specifically +requesting that patches should NOT be sent PGP-signed. + +Why not just rely on proper code review? +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Code review is a crucial step of the development process and patatt does +not aim to replace it. However, there are several areas where the +process can be abused by malicious parties in the absence of end-to-end +cryptographic attestation: + +1. A maintainer who struggles with code review volume may delegate parts + of their duties to a submaintainer. If that person submits aggregated + patch series to the maintainer after performing that work, there must + be a mechanism to ensure that none of the reviewed patches have been + modified between when they were reviewed by the trusted submaintainer + and when the upstream developer applies them to their tree. Up to + now, the only mechanism to ensure this was via signed pull requests + -- with patatt this is now also possible with regular patch series. + +2. It is important to ensure that what developer reviews is what + actually ends up being applied to their git tree. Linux development + process consists of collecting follow-up trailers (Tested-by, + Reviewed-by, etc), so various tooling exists to aggregate these + trailers and create the collated patch series containing all + follow-up tags (see b4, patchwork, etc). Patatt signing provides a + mechanism to ensure that what that developer reviewed and approved + and what they applied to their tree is the exact same code and hasn't + been maliciously modified in-between review and "git am" (e.g. by + archival services such as lore.kernel.org, mail hosting providers, + someone with access to the developer's inbox, etc). + +3. An attacker may attempt to impersonate a well-known developer by + submitting malicious code, perhaps with the hope that it receives + less scrutiny and is accepted without rigorous code review. Even if + this attempt is unsuccessful (and it most likely would be), this may + cause unnecessary reputation damage to the person being impersonated. + Cryptographic signatures (and lack thereof) will help the developer + quickly establish that the attack was performed without their + involvement. + +Why not just rely on DKIM? +~~~~~~~~~~~~~~~~~~~~~~~~~~ +DKIM standard is great, but there are several places where it falls a +bit short when it comes to patch attestation: + +1. The signing is done by the mail gateways that may or may not be + properly checking that the "From:" header matches the identity of the + authenticated user. For example, a service that allows free account + registration may not check that [email protected] sends outgoing + email with "[email protected]" in the "From:" field, which would allow + Alice to impersonate Bob and have the messages arrive with a valid + DKIM signature. + +2. DKIM is usually seen as merely a spam reduction mechanism, so there's + usually little incentive for infrastructure administrators to be too + strict about how they handle the private keys used for DKIM signing. + Most likely, they are just stored on disk without a passphrase and + accessible by the SMTP daemon. + +3. DKIM's "relaxed" canonicalization standard for message bodies + replaces all multiple whitespace characters with a single space + before the body hash is signed. This poses significant problems for + patches where whitespace is syntactically significant (Python, + Makefiles, etc). A "return True" with a different indent will pass + DKIM signature check and may introduce a serious security + vulnerability. + +4. DKIM doesn't prevent typosquatting attacks. For example, an attacker + attempting to impersonate [email protected] may send an + email from [email protected] or any other + similar-looking address or domain, with valid DKIM signatures in + every case. diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/patatt-0.4.4/patatt/__init__.py new/patatt-0.4.6/patatt/__init__.py --- old/patatt-0.4.4/patatt/__init__.py 2021-05-28 16:10:52.000000000 +0200 +++ new/patatt-0.4.6/patatt/__init__.py 2021-06-09 21:00:01.000000000 +0200 @@ -47,7 +47,7 @@ KEYCACHE = dict() # My version -__VERSION__ = '0.4.4' +__VERSION__ = '0.4.6' MAX_SUPPORTED_FORMAT_VERSION = 1 @@ -69,6 +69,12 @@ self.errors = errors +class NoKeyError(ValidationError): + def __init__(self, message: str, errors: Optional[list] = None): + super().__init__(message) + self.errors = errors + + class BodyValidationError(ValidationError): def __init__(self, message: str, errors: Optional[list] = None): super().__init__(message, errors) @@ -91,7 +97,7 @@ def from_bytes(self, hval: bytes) -> None: self.hval = DevsigHeader._dkim_canonicalize_header(hval) - hval = re.sub(rb'\s*', b'', hval) + hval = re.sub(rb'\s*', b'', self.hval) for chunk in hval.split(b';'): parts = chunk.split(b'=', 1) if len(parts) < 2: @@ -346,6 +352,8 @@ ecode, out, err = gpg_run_command(vrfyargs, stdin=bsigdata) if ecode > 0: + if err.find(b'[GNUPG:] NO_PUBKEY '): + raise NoKeyError('No matching key found') raise ValidationError('Failed to validate PGP signature') good, valid, trusted, signkey, signtime = DevsigHeader._check_gpg_status(err) @@ -392,6 +400,15 @@ @staticmethod def _dkim_canonicalize_header(hval: bytes) -> bytes: + # Handle MIME encoded-word syntax or other types of header encoding if + # present. The decode_header() function requires a str argument (not + # bytes) so we must decode our bytes first, this is easy as RFC2822 (sec + # 2.2) says header fields must be composed of US-ASCII characters. The + # resulting string is re-encoded to allow further processing. + if b'?q?' in hval: + hval = hval.decode('ascii', errors='ignore') + hval = str(email.header.make_header(email.header.decode_header(hval))) + hval = hval.encode('utf-8') # We only do relaxed for headers # o Unfold all header field continuation lines as described in # [RFC5322]; in particular, lines with terminators embedded in @@ -941,13 +958,15 @@ # Default keyring used keysrc = '(default keyring)/%s' % signkey attestations.append((RES_VALID, i, signtime, keysrc, algo, errors)) + except NoKeyError: + # Not in default keyring + errors.append('%s/%s no matching openpgp key found' % (i, s)) + attestations.append((RES_NOKEY, i, t, None, algo, errors)) except ValidationError: if keysrc is None: - # Not in default keyring - errors.append('%s/%s no matching openpgp key found' % (i, s)) - attestations.append((RES_NOKEY, i, t, None, algo, errors)) - continue - errors.append('failed to validate using %s' % keysrc) + errors.append('failed to validate using default keyring') + else: + errors.append('failed to validate using %s' % keysrc) attestations.append((RES_BADSIG, i, t, keysrc, algo, errors)) return attestations
