[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#702908: PTS: upload signature parsing patch



Hi,

attached is an updated patch. I sanitized the body pseudo-header
parsing, especially when combined with an OpenPGP signature.

On 03/13/2013 08:29 AM, Raphael Hertzog wrote:
>>> In the news.xml file, I replaced the "from" attribute of the news item
>>> with more fine grained "from_address" and "from_realname". However, I
>>> think existing entries will be kept, so the XSL-templates need to be
>>> able to parse both. At least that's how I've implemented it. If a
>>> complete rewrite of all news.xml files is feasible, the XSLTs could be
>>> simplified quite a bit.
>>
>> I guess this is needed for the developer.php links, fair enough.
> 
> I don't understand your comment. Markus, it's certainly possible to update
> all news.xml once with a new structure.
> 
> $ bin/list_packages.sh | bin/update_news.py

Okay, so it doesn't just parse the mails, but stores them for later
processing as well. Perfect.

> So if you can simplify, please do it.

Done.

> I noticed in particular that you used <xsl:when> a few times where a simple
> <xsl:if> would have been more appropriate.

I didn't find any place where <xsl:if> would have been sufficient.

However, I guess that's also a bit a matter of taste. I prefer
<xsl:choose> whenever I need <xsl:otherwise> (i.e. an else clause).
AFAIK there's no such thing for an <xsl:if>. With that, you'd have to
repeat the test with a negation. While a bit less clutter with xml tags,
I think that's more prone to error.

> I also saw a "print" that you
> probably left while debugging (print "signature uid: …).

Removed.

Regards

Markus
Index: xsl/news2rss.xsl
===================================================================
--- xsl/news2rss.xsl	(revision 2953)
+++ xsl/news2rss.xsl	(working copy)
@@ -60,9 +60,25 @@
     <item>
       <title>
 	<xsl:value-of select="." />
-	<xsl:text> (</xsl:text>
-	<xsl:value-of select="@from" />
-	<xsl:text>)</xsl:text>
+        <xsl:if test="@from_address or @from_realname">
+          <xsl:text> (</xsl:text>
+          <xsl:choose>
+            <xsl:when test="@from_address and @sign_address and @from_address != @sign_address">
+              <xsl:choose>
+                <xsl:when test="@from_realname"><xsl:value-of select="@from_realname"/></xsl:when>
+                <xsl:otherwise><xsl:value-of select="@from_address"/></xsl:otherwise>
+              </xsl:choose>
+              <xsl:text> - signed by: </xsl:text>
+              <xsl:choose>
+                <xsl:when test="@sign_realname"><xsl:value-of select="@sign_realname"/></xsl:when>
+                <xsl:otherwise><xsl:value-of select="@sign_address"/></xsl:otherwise>
+              </xsl:choose>
+            </xsl:when>
+            <xsl:when test="@from_realname"><xsl:value-of select="@from_realname"/></xsl:when>
+            <xsl:otherwise><xsl:value-of select="@from_address"/></xsl:otherwise>
+          </xsl:choose>
+          <xsl:text>)</xsl:text>
+        </xsl:if>
       </title>
       <xsl:variable name="id">
 	<xsl:value-of select="$ptsurl" />
@@ -81,9 +97,26 @@
         <xsl:value-of select="@date" />
         <xsl:text>] </xsl:text>
         <xsl:value-of select="." />
-        <xsl:text> (</xsl:text>
-        <xsl:value-of select="@from" />
-        <xsl:text>)&lt;/a&gt;</xsl:text>
+        <xsl:if test="@from_address or @from_realname">
+          <xsl:text> (</xsl:text>
+          <xsl:choose>
+            <xsl:when test="@from_address and @sign_address and @from_address != @sign_address">
+              <xsl:choose>
+                <xsl:when test="@from_realname"><xsl:value-of select="@from_realname"/></xsl:when>
+                <xsl:otherwise><xsl:value-of select="@from_address"/></xsl:otherwise>
+              </xsl:choose>
+              <xsl:text> - signed by: </xsl:text>
+              <xsl:choose>
+                <xsl:when test="@sign_realname"><xsl:value-of select="@sign_realname"/></xsl:when>
+                <xsl:otherwise><xsl:value-of select="@sign_address"/></xsl:otherwise>
+              </xsl:choose>
+            </xsl:when>
+            <xsl:when test="@from_realname"><xsl:value-of select="@from_realname"/></xsl:when>
+            <xsl:otherwise><xsl:value-of select="@from_address"/></xsl:otherwise>
+          </xsl:choose>
+          <xsl:text>)</xsl:text>
+        </xsl:if>
+        <xsl:text>&lt;/a&gt;</xsl:text>
       </description>
     </item>
   </xsl:template>
Index: xsl/pts.xsl
===================================================================
--- xsl/pts.xsl	(revision 2953)
+++ xsl/pts.xsl	(working copy)
@@ -75,6 +75,38 @@
 <xsl:variable name="security-mirror">http://security.debian.org/</xsl:variable>
 <xsl:variable name="backports-mirror">http://http.debian.net/debian-backports</xsl:variable>
 
+<xsl:template name="name_email_link">
+  <!-- may be called with realname, address or both -->
+  <xsl:param name="realname"/>
+  <xsl:param name="address"/>
+  <xsl:param name="title"/>
+
+  <xsl:choose>
+    <xsl:when test="string-length($address) > 0">
+      <xsl:element name="a">
+        <xsl:attribute name="href">
+          <xsl:text>http://qa.debian.org/developer.php?login=</xsl:text>
+          <xsl:call-template name="escape-name">
+            <xsl:with-param name="text"><xsl:value-of select="$address"/></xsl:with-param>
+          </xsl:call-template>
+        </xsl:attribute>
+        <span class="name">
+          <xsl:if test="string-length($title) > 0">
+            <xsl:attribute name="title"><xsl:value-of select="$title"/></xsl:attribute>
+          </xsl:if>
+          <xsl:choose>
+            <xsl:when test="string-length($realname) > 0"><xsl:value-of select="$realname"/></xsl:when>
+            <xsl:otherwise><xsl:value-of select="$address"/></xsl:otherwise>
+          </xsl:choose>
+        </span>
+      </xsl:element>
+    </xsl:when>
+    <xsl:otherwise>
+      <xsl:value-of select="$realname"/>
+    </xsl:otherwise>
+  </xsl:choose>
+</xsl:template>
+
 <xsl:template name="outputitem">
   <xsl:choose>
     <xsl:when test="@url">
@@ -83,10 +115,34 @@
 	<xsl:value-of select="@date"/>
         <xsl:text>] </xsl:text>
       </xsl:if><a href="{@url}">
-      <xsl:value-of select="text()"/></a><xsl:if test="@from">
+      <xsl:value-of select="text()"/></a>
+
+      <xsl:if test="@from_address">
         <xsl:text> (</xsl:text>
-	<xsl:value-of select="@from"/>
-        <xsl:text>)</xsl:text></xsl:if></li>
+        <xsl:choose>
+          <xsl:when test="@from_address and @sign_address and @from_address != @sign_address">
+            <xsl:text></xsl:text><xsl:call-template name="name_email_link">
+              <xsl:with-param name="title">email sender</xsl:with-param>
+              <xsl:with-param name="realname"><xsl:value-of select="@from_realname"/></xsl:with-param>
+              <xsl:with-param name="address"><xsl:value-of select="@from_address"/></xsl:with-param>
+            </xsl:call-template>
+            <xsl:text> - </xsl:text>
+            <xsl:text>signed by: </xsl:text><xsl:call-template name="name_email_link">
+              <xsl:with-param name="title">signer</xsl:with-param>
+              <xsl:with-param name="realname"><xsl:value-of select="@sign_realname"/></xsl:with-param>
+              <xsl:with-param name="address"><xsl:value-of select="@sign_address"/></xsl:with-param>
+            </xsl:call-template>
+          </xsl:when>
+          <xsl:otherwise>
+            <xsl:call-template name="name_email_link">
+              <xsl:with-param name="realname"><xsl:value-of select="@from_realname"/></xsl:with-param>
+              <xsl:with-param name="address"><xsl:value-of select="@from_address"/></xsl:with-param>
+            </xsl:call-template>
+          </xsl:otherwise>
+        </xsl:choose>
+        <xsl:text>)</xsl:text>
+      </xsl:if>
+      </li>
     </xsl:when>
     <xsl:when test="@type='raw'">
       <li>
@@ -208,13 +264,6 @@
   </xsl:if>
 </xsl:template>
 
-<xsl:template name="maintainer-email">
-  <xsl:param name="email" />
-  <a class="email" href="mailto:{$email}";>
-    <img alt="[email]" src="../common/email.png" title="email" />
-  </a>
-</xsl:template>
-
 <xsl:template name="general-information">
   <div class="block info">
     <a name="general" />
@@ -245,17 +294,11 @@
 
       <dt title="Maintainer and Uploaders">maint</dt>
       <dd class="maintainer">
-	<xsl:element name="a">
-	  <xsl:attribute name="href">
-	    <xsl:text>http://qa.debian.org/developer.php?login=</xsl:text>
-	    <xsl:call-template name="escape-name">
-	      <xsl:with-param name="text"><xsl:value-of select="maintainer/email"/></xsl:with-param>
-	    </xsl:call-template>
-	  </xsl:attribute>
-	  <span class="name" title="maintainer">
-	    <xsl:value-of select="maintainer/name"/>
-	  </span>
-	</xsl:element>
+        <xsl:call-template name="name_email_link">
+          <xsl:with-param name="title">maintainer</xsl:with-param>
+          <xsl:with-param name="realname"><xsl:value-of select="maintainer/name"/></xsl:with-param>
+          <xsl:with-param name="address"><xsl:value-of select="maintainer/email"/></xsl:with-param>
+        </xsl:call-template>
 	<xsl:if test="$hasother and $other/dms/item/@email=maintainer/email">
 	  (<small>dm</small>)
 	</xsl:if>
@@ -264,19 +307,11 @@
 	    <xsl:text>, </xsl:text>
 	    <span class="uploader">
 	      <small>
-	      <xsl:element name="a">
-		<xsl:attribute name="href">
-		  <xsl:text>http://qa.debian.org/developer.php?login=</xsl:text>
-		  <xsl:call-template name="escape-name">
-		    <xsl:with-param name="text">
-		      <xsl:value-of select="email"/>
-		    </xsl:with-param>
-		  </xsl:call-template>
-		</xsl:attribute>
-		<span class="name" title="uploader">
-		  <xsl:value-of select="name"/>
-		</span>
-	      </xsl:element>
+              <xsl:call-template name="name_email_link">
+                <xsl:with-param name="title">uploader</xsl:with-param>
+                <xsl:with-param name="realname"><xsl:value-of select="name"/></xsl:with-param>
+                <xsl:with-param name="address"><xsl:value-of select="email"/></xsl:with-param>
+              </xsl:call-template>
 	      <xsl:text> (u</xsl:text>
 	      <xsl:if test="$hasother and $other/dms/item/@email=email">
 		<xsl:text>, dm</xsl:text>
Index: bin/update_news.py
===================================================================
--- bin/update_news.py	(revision 2953)
+++ bin/update_news.py	(working copy)
@@ -79,8 +79,12 @@
         sub_elt.setAttribute("date", info["date"])
     sub_elt.setAttribute("rfc822date",
                          timestamp_to_rfc822date(info["timestamp"]))
-    if info.has_key("from_name"):
-        sub_elt.setAttribute("from", info["from_name"])
+    # Copy these attributes as-is
+    for attname in ("from_realname", "from_address",
+		    "sign_realname", "sign_address",
+		    "sign_valid", "sign_fingerprint", "sign_missing_pubkey"):
+	if info.has_key(attname):
+	    sub_elt.setAttribute(attname, info[attname])
     elt.appendChild(sub_elt)
 
 
Index: bin/common.py
===================================================================
--- bin/common.py	(revision 2953)
+++ bin/common.py	(working copy)
@@ -7,8 +7,12 @@
 # This file is distributed under the terms of the General Public License
 # version 2 or (at your option) any later version.
 
-import hashlib, os, os.path, re, rfc822, time, email, sys
+import hashlib, os, os.path, re, rfc822, time, email, sys, gpgme
 from email import Utils, Header
+try:
+    from io import BytesIO
+except ImportError:
+    from StringIO import StringIO as BytesIO
 
 from config import root
 
@@ -45,16 +49,30 @@
                 break
     else:
         body = msg.get_payload(None, 1)
-    lines = body.split("\n", 3)
-    for i in range(3):
-        res = re.match(r"^(\w+): (\S+)(.*)", lines[i])
-        if res:
-            field = res.group(1).lower()
-            info[field] = unicode(res.group(2), "UTF-8", "replace")
-            if field == "subject":
-                info[field] = info[field] + unicode(res.group(3), "UTF-8", "replace")
+
+    # Analize first few lines for OpenPGP signature or additional pseudo-
+    # header fields.
+    lines = body.split("\n", 10)
+    i = 0
+    isSigned = False
+    while i < len(lines):
+        if lines[i].startswith("-----BEGIN PGP"):
+            isSigned = True
+            i += 1
+            # Skip all OpenPGP Armor Headers, which are also of the form
+            # Key Colon Space Value Newline.
+            while lines[i].strip() != "" and i < len(lines):
+                i += 1
         else:
-            break
+            res = re.match(r"^(\w+): (\S+)(.*)", lines[i])
+            if res:
+                field = res.group(1).lower()
+                info[field] = unicode(res.group(2), "UTF-8", "replace")
+                if field == "subject":
+                    info[field] = info[field] + unicode(res.group(3), "UTF-8", "replace")
+            else:
+                break
+        i += 1
 
     if not info.has_key("subject"):
         subject = decode_header(msg.get("subject"))
@@ -93,12 +111,70 @@
         frm = msg.get("From")
         if msg.has_key("X-PTS-From"): frm = msg.get("X-PTS-From")
         (realname, address) = rfc822.parseaddr(frm)
+
         if realname:
-            frm = realname
-        else:
-            frm = address
-        frm = decode_header(frm)
-        info["from_name"] = frm
+            info["from_realname"] = decode_header(realname)
+        info["from_address"] = decode_header(address)
+
+    if isSigned:
+        ctx = gpgme.Context()
+        plain = BytesIO()
+        result = ctx.verify(BytesIO(body), None, plain)
+        for sig in result:
+            # All flags returned by GPGME
+            valid = bool(sig.summary & gpgme.SIGSUM_VALID)
+            green = bool(sig.summary & gpgme.SIGSUM_GREEN)
+            key_revoked = bool(sig.summary & gpgme.SIGSUM_KEY_REVOKED)
+            key_expired = bool(sig.summary & gpgme.SIGSUM_KEY_EXPIRED)
+            key_expired = bool(sig.summary & gpgme.SIGSUM_SIG_EXPIRED)
+            key_missing = bool(sig.summary & gpgme.SIGSUM_KEY_MISSING)
+            crl_missing = bool(sig.summary & gpgme.SIGSUM_CRL_MISSING)
+            crl_too_old = bool(sig.summary & gpgme.SIGSUM_CRL_TOO_OLD)
+            bad_policy = bool(sig.summary & gpgme.SIGSUM_BAD_POLICY)
+            sys_error = bool(sig.summary & gpgme.SIGSUM_SYS_ERROR)
+
+            if key_missing:
+                # Missing public key of the signer.  We cannot check
+                # validity.
+                info['sign_fingerprint'] = sig.fpr
+                info['sign_missing_pubkey'] = "true"
+            elif sys_error:
+                continue
+            else:
+                # We found a signature and successfully parsed it and the
+                # accompaining public key.  We pass on a simple boolean for
+                # validity of the signature, but the main piece of
+                # information the PTS needs to show is *who* signed.
+                try:
+                    info['sign_fingerprint'] = sig.fpr
+                    key = ctx.get_key(sig.fpr)
+
+                    best = None
+                    for uid in key.uids:
+                        if not best:
+                            best = (uid.name, uid.email)
+                            # We continue the loop, because we prefer a
+                            # @debian.org address.
+
+                        if uid.email.endswith("@debian.org"):
+                            best = (uid.name, uid.email)
+                            break
+
+                    if best:
+                        info['sign_realname'] = best[0]
+                        info['sign_address'] = best[1]
+                        if valid and green:
+                            info['sign_valid'] = "true"
+                        else:
+                            info['sign_valid'] = "false"
+
+                    # Simply use the first valid signature found
+                    break
+                except:
+                    # Catch all exceptions. At the worst, we do not display
+                    # signature info, so that's not too critical.
+                    continue
+
     return info
 
 def hash_name(pkg):

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: