This is an automated email from the ASF dual-hosted git repository.

sbp pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tooling-atr-experiments.git


The following commit(s) were added to refs/heads/main by this push:
     new a6a81c2  Improve the public signing key model
a6a81c2 is described below

commit a6a81c20945bbfc3bbe8ab7eacdea8ec1520ca0e
Author: Sean B. Palmer <[email protected]>
AuthorDate: Tue Feb 18 17:15:54 2025 +0200

    Improve the public signing key model
---
 atr/db/models.py                                   | 23 ++++--
 atr/routes.py                                      | 89 ++++++++++++++--------
 atr/static/css/atr.css                             | 32 ++++++++
 atr/templates/secret/update-pmcs.html              |  4 +-
 atr/templates/user-keys-add.html                   | 54 +++++++++----
 ...al_schema.py => 512e973a9ce4_initial_schema.py} |  8 +-
 6 files changed, 152 insertions(+), 58 deletions(-)

diff --git a/atr/db/models.py b/atr/db/models.py
index ef44270..6b27006 100644
--- a/atr/db/models.py
+++ b/atr/db/models.py
@@ -37,14 +37,27 @@ class UserRole(str, Enum):
 
 class PMCKeyLink(SQLModel, table=True):
     pmc_id: int = Field(foreign_key="pmc.id", primary_key=True)
-    key_user_id: str = Field(foreign_key="publicsigningkey.user_id", 
primary_key=True)
+    key_fingerprint: str = Field(foreign_key="publicsigningkey.fingerprint", 
primary_key=True)
 
 
 class PublicSigningKey(SQLModel, table=True):
-    user_id: str = Field(primary_key=True)
-    public_key: str
-    key_type: str
-    expiration: datetime.datetime
+    # The fingerprint must be stored as lowercase hex
+    fingerprint: str = Field(primary_key=True, unique=True)
+    # The algorithm is an RFC 4880 algorithm ID
+    algorithm: int
+    # Key length in bits
+    length: int
+    # Creation date
+    created: datetime.datetime
+    # Expiration date
+    expires: datetime.datetime | None
+    # The UID declared in the key
+    declared_uid: str | None
+    # The UID used by Apache
+    apache_uid: str
+    # The ASCII armored key
+    ascii_armored_key: str
+    # The PMCs that use this key
     pmcs: list["PMC"] = Relationship(back_populates="public_signing_keys", 
link_model=PMCKeyLink)
 
 
diff --git a/atr/routes.py b/atr/routes.py
index cc2fbf2..25fca27 100644
--- a/atr/routes.py
+++ b/atr/routes.py
@@ -25,9 +25,8 @@ import secrets
 import shutil
 import tempfile
 from contextlib import asynccontextmanager
-from io import BufferedReader
 from pathlib import Path
-from typing import Any, cast
+from typing import Any, BinaryIO, cast
 
 import aiofiles
 import aiofiles.os
@@ -61,6 +60,31 @@ from .util import compute_sha512, get_release_storage_dir
 if APP is ...:
     raise ValueError("APP is not set")
 
+# |         1 | RSA (Encrypt or Sign) [HAC]                        |
+# |         2 | RSA Encrypt-Only [HAC]                             |
+# |         3 | RSA Sign-Only [HAC]                                |
+# |        16 | Elgamal (Encrypt-Only) [ELGAMAL] [HAC]             |
+# |        17 | DSA (Digital Signature Algorithm) [FIPS186] [HAC]  |
+# |        18 | ECDH public key algorithm                          |
+# |        19 | ECDSA public key algorithm [FIPS186]               |
+# |        20 | Reserved (formerly Elgamal Encrypt or Sign)        |
+# |        21 | Reserved for Diffie-Hellman                        |
+# |           | (X9.42, as defined for IETF-S/MIME)                |
+# |        22 | EdDSA [I-D.irtf-cfrg-eddsa]                        |
+# - https://lists.gnupg.org/pipermail/gnupg-devel/2017-April/032762.html
+# TODO: (Obviously we should move this, but where to?)
+algorithms = {
+    1: "RSA",
+    2: "RSA",
+    3: "RSA",
+    16: "Elgamal",
+    17: "DSA",
+    18: "ECDH",
+    19: "ECDSA",
+    21: "Diffie-Hellman",
+    22: "EdDSA",
+}
+
 
 @APP.route("/")
 async def root() -> str:
@@ -203,7 +227,7 @@ async def root_release_signatures_verify(release_key: str) 
-> str:
 
         # For now, for debugging, we'll just get all keys in the database
         statement = select(PublicSigningKey)
-        all_public_keys = (await db_session.execute(statement)).scalars().all()
+        all_signing_keys = (await 
db_session.execute(statement)).scalars().all()
 
         statement = (
             select(Release)
@@ -214,8 +238,8 @@ async def root_release_signatures_verify(release_key: str) 
-> str:
         if not release:
             raise ASFQuartException("Release not found", errorcode=404)
 
-        # Get all public keys associated with the PMC
-        pmc_keys = [key.public_key for key in all_public_keys]
+        # Get all ASCII-armored keys associated with the PMC
+        ascii_armored_keys = [key.ascii_armored_key for key in 
all_signing_keys]
 
         # Verify each package's signature
         verification_results = []
@@ -233,7 +257,7 @@ async def root_release_signatures_verify(release_key: str) 
-> str:
                 result["error"] = "Package signature file not found"
             else:
                 # Verify the signature
-                result = await verify_gpg_signature(artifact_path, 
signature_path, pmc_keys)
+                result = await verify_gpg_signature(artifact_path, 
signature_path, ascii_armored_keys)
                 result["file"] = package.file
 
             verification_results.append(result)
@@ -327,7 +351,7 @@ async def root_user_keys_add() -> str:
 
     # Get all existing keys for the user
     async with get_session() as db_session:
-        statement = select(PublicSigningKey).where(PublicSigningKey.user_id == 
session.uid)
+        statement = select(PublicSigningKey).where(PublicSigningKey.apache_uid 
== session.uid)
         user_keys = (await db_session.execute(statement)).scalars().all()
 
     if request.method == "POST":
@@ -345,6 +369,7 @@ async def root_user_keys_add() -> str:
         error=error,
         key_info=key_info,
         user_keys=user_keys,
+        algorithms=algorithms,
     )
 
 
@@ -359,9 +384,8 @@ async def root_user_keys_delete() -> str:
     async with get_session() as db_session:
         async with db_session.begin():
             # Get all keys for the user
-            # TODO: Might be clearer if user_id were "asf_id"
-            # But then we'd also want session.uid to be session.asf_id instead
-            statement = 
select(PublicSigningKey).where(PublicSigningKey.user_id == session.uid)
+            # TODO: Use session.apache_uid instead of session.uid?
+            statement = 
select(PublicSigningKey).where(PublicSigningKey.apache_uid == session.uid)
             keys = (await db_session.execute(statement)).scalars().all()
             count = len(keys)
 
@@ -456,7 +480,8 @@ async def user_keys_add(session: ClientSession, public_key: 
str) -> tuple[str, d
         if not import_result.fingerprints:
             return ("Invalid public key format", None)
 
-        fingerprint = import_result.fingerprints[0]
+        fingerprint = import_result.fingerprints[0].lower()
+        # APP.logger.info("Import result: %s", vars(import_result))
         # Get key details
         # We could probably use import_result instead
         # But this way it shows that they've really been imported
@@ -465,7 +490,7 @@ async def user_keys_add(session: ClientSession, public_key: 
str) -> tuple[str, d
     # Then we have the properties listed here:
     # https://gnupg.readthedocs.io/en/latest/#listing-keys
     # Note that "fingerprint" is not listed there, but we have it anyway...
-    key = next((k for k in keys if k["fingerprint"] == fingerprint), None)
+    key = next((k for k in keys if k["fingerprint"].lower() == fingerprint), 
None)
     if not key:
         return ("Failed to import key", None)
     if (key.get("algo") == "1") and (int(key.get("length", "0")) < 2048):
@@ -482,7 +507,7 @@ async def user_keys_add_session(
     session: ClientSession, public_key: str, key: dict, db_session: 
AsyncSession
 ) -> tuple[str, dict | None]:
     # Check if key already exists
-    statement = select(PublicSigningKey).where(PublicSigningKey.user_id == 
session.uid)
+    statement = select(PublicSigningKey).where(PublicSigningKey.apache_uid == 
session.uid)
 
     # # If uncommented, this will prevent a user from adding a second key
     # existing_key = (await db_session.execute(statement)).scalar_one_or_none()
@@ -492,15 +517,18 @@ async def user_keys_add_session(
     if not session.uid:
         return ("You must be signed in to add a key", None)
 
+    uids = key.get("uids")
     async with db_session.begin():
         # Create new key record
         key_record = PublicSigningKey(
-            user_id=session.uid,
-            public_key=public_key,
-            key_type=key.get("type", "unknown"),
-            expiration=datetime.datetime.fromtimestamp(int(key["expires"]))
-            if key.get("expires")
-            else datetime.datetime.max,
+            fingerprint=key["fingerprint"].lower(),
+            algorithm=int(key["algo"]),
+            length=int(key.get("length", "0")),
+            created=datetime.datetime.fromtimestamp(int(key["date"])),
+            expires=datetime.datetime.fromtimestamp(int(key["expires"])) if 
key.get("expires") else None,
+            declared_uid=uids[0] if uids else None,
+            apache_uid=session.uid,
+            ascii_armored_key=public_key,
         )
         db_session.add(key_record)
 
@@ -508,8 +536,8 @@ async def user_keys_add_session(
         for pmc_name in session.committees:
             statement = select(PMC).where(PMC.project_name == pmc_name)
             pmc = (await db_session.execute(statement)).scalar_one_or_none()
-            if pmc and pmc.id and session.uid:
-                link = PMCKeyLink(pmc_id=pmc.id, key_user_id=session.uid)
+            if pmc and pmc.id:
+                link = PMCKeyLink(pmc_id=pmc.id, 
key_fingerprint=key_record.fingerprint)
                 db_session.add(link)
             else:
                 # TODO: Log? Add to "error"?
@@ -519,7 +547,7 @@ async def user_keys_add_session(
         "",
         {
             "key_id": key["keyid"],
-            "fingerprint": key["fingerprint"],
+            "fingerprint": key["fingerprint"].lower(),
             "user_id": key["uids"][0] if key.get("uids") else "Unknown",
             "creation_date": datetime.datetime.fromtimestamp(int(key["date"])),
             "expiration_date": 
datetime.datetime.fromtimestamp(int(key["expires"])) if key.get("expires") else 
None,
@@ -546,26 +574,25 @@ async def verify_gpg_signature(artifact_path: Path, 
signature_path: Path, public
 
 
 async def verify_gpg_signature_file(
-    sig_file: BufferedReader, artifact_path: Path, public_keys: list[str]
+    sig_file: BinaryIO, artifact_path: Path, ascii_armored_keys: list[str]
 ) -> dict[str, Any]:
-    # Run the blocking GPG verification in a thread
+    """Verify a GPG signature for a file."""
     async with ephemeral_gpg_home() as gpg_home:
         gpg = gnupg.GPG(gnupghome=gpg_home)
 
-        # Import all PMC public keys
-        for key in public_keys:
+        # Import all PMC public signing keys
+        for key in ascii_armored_keys:
             import_result = await asyncio.to_thread(gpg.import_keys, key)
             if not import_result.fingerprints:
                 # TODO: Log warning about invalid key?
                 continue
-
         verified = await asyncio.to_thread(gpg.verify_file, sig_file, 
str(artifact_path))
 
     # Collect all available information for debugging
     debug_info = {
         "key_id": verified.key_id or "Not available",
-        "fingerprint": verified.fingerprint or "Not available",
-        "pubkey_fingerprint": verified.pubkey_fingerprint or "Not available",
+        "fingerprint": verified.fingerprint.lower() or "Not available",
+        "pubkey_fingerprint": verified.pubkey_fingerprint.lower() or "Not 
available",
         "creation_date": verified.creation_date or "Not available",
         "timestamp": verified.timestamp or "Not available",
         "username": verified.username or "Not available",
@@ -574,7 +601,7 @@ async def verify_gpg_signature_file(
         "trust_level": verified.trust_level if hasattr(verified, 
"trust_level") else "Not available",
         "trust_text": verified.trust_text if hasattr(verified, "trust_text") 
else "Not available",
         "stderr": verified.stderr if hasattr(verified, "stderr") else "Not 
available",
-        "num_pmc_keys": len(public_keys),
+        "num_pmc_keys": len(ascii_armored_keys),
     }
 
     if not verified:
@@ -590,7 +617,7 @@ async def verify_gpg_signature_file(
         "key_id": verified.key_id,
         "timestamp": verified.timestamp,
         "username": verified.username or "Unknown",
-        "email": verified.pubkey_fingerprint or "Unknown",
+        "email": verified.pubkey_fingerprint.lower() or "Unknown",
         "status": "Valid signature",
         "debug_info": debug_info,
     }
diff --git a/atr/static/css/atr.css b/atr/static/css/atr.css
index bb72ad1..3defa7a 100644
--- a/atr/static/css/atr.css
+++ b/atr/static/css/atr.css
@@ -61,6 +61,38 @@ label {
     cursor: pointer;
 }
 
+table {
+    width: 100%;
+    border-collapse: collapse;
+}
+
+table th {
+    text-align: left;
+    padding: 0.5rem;
+    font-weight: 500;
+    width: 30%;
+    color: #333333;
+}
+
+table td {
+    padding: 0.5rem;
+
+    /* Almost all of our tables have data in them */
+
+    /* Not sure if we should keep it this way, but it seems pretty good */
+    font-family: ui-monospace, "SFMono-Regular", "Menlo", "Monaco", 
"Consolas", monospace;
+    font-size: 0.9em;
+}
+
+table tr {
+    /* This doesn't always work; not clear why */
+    border-bottom: 1px solid #c1c2c3;
+}
+
+table tr:last-child {
+    border-bottom: none;
+}
+
 aside h1 {
     font-size: 3rem;
     line-height: 1.1;
diff --git a/atr/templates/secret/update-pmcs.html 
b/atr/templates/secret/update-pmcs.html
index 044e2a9..48bc217 100644
--- a/atr/templates/secret/update-pmcs.html
+++ b/atr/templates/secret/update-pmcs.html
@@ -30,7 +30,7 @@
           background: #047;
       }
 
-      .warning {
+      div.warning {
           margin: 1.5rem 0;
           padding: 1rem;
           background: #fff3cd;
@@ -39,7 +39,7 @@
           color: #856404;
       }
 
-      .warning strong {
+      div.warning strong {
           color: #533f03;
       }
   </style>
diff --git a/atr/templates/user-keys-add.html b/atr/templates/user-keys-add.html
index a219685..bf78133 100644
--- a/atr/templates/user-keys-add.html
+++ b/atr/templates/user-keys-add.html
@@ -74,16 +74,25 @@
 
       .keys-grid {
           display: grid;
-          grid-template-columns: repeat(auto-fill, minmax(300px, 1fr));
-          gap: 1rem;
-          margin-top: 1rem;
+          /* This just messes up resizing */
+          /* grid-template-columns: repeat(auto-fill, minmax(800px, 1fr)); */
+          gap: 1.5rem;
       }
 
       .key-card {
-          padding: 1rem;
           background: white;
-          border: 1px solid #dee2e6;
+          border: 1px solid #d1d2d3;
           border-radius: 4px;
+          overflow: hidden;
+          padding: 1rem;
+      }
+
+      .key-card table {
+          margin: 0;
+      }
+
+      .key-card td {
+          word-break: break-all;
       }
 
       .key-card h3 {
@@ -173,17 +182,30 @@
       <div class="keys-grid">
         {% for key in user_keys %}
           <div class="key-card">
-            <h3>Key Details</h3>
-            <dl>
-              <dt>Key Type</dt>
-              <dd>
-                {{ key.key_type }}
-              </dd>
-              <dt>Expires</dt>
-              <dd>
-                {{ key.expiration.strftime("%Y-%m-%d %H:%M:%S") if 
key.expiration else 'Never' }}
-              </dd>
-            </dl>
+            <table>
+              <tbody>
+                <tr>
+                  <th>Fingerprint</th>
+                  <td>{{ key.fingerprint }}</td>
+                </tr>
+                <tr>
+                  <th>Key Type</th>
+                  <td>{{ algorithms[key.algorithm] }} ({{ key.length }} 
bits)</td>
+                </tr>
+                <tr>
+                  <th>Created</th>
+                  <td>{{ key.created.strftime("%Y-%m-%d %H:%M:%S") }}</td>
+                </tr>
+                <tr>
+                  <th>Expires</th>
+                  <td>{{ key.expires.strftime("%Y-%m-%d %H:%M:%S") if 
key.expires else 'Never' }}</td>
+                </tr>
+                <tr>
+                  <th>User ID</th>
+                  <td>{{ key.declared_uid or 'Not specified' }}</td>
+                </tr>
+              </tbody>
+            </table>
           </div>
         {% endfor %}
       </div>
diff --git a/migrations/versions/15182b58521a_initial_schema.py 
b/migrations/versions/512e973a9ce4_initial_schema.py
similarity index 81%
rename from migrations/versions/15182b58521a_initial_schema.py
rename to migrations/versions/512e973a9ce4_initial_schema.py
index 6db694f..c9f6768 100644
--- a/migrations/versions/15182b58521a_initial_schema.py
+++ b/migrations/versions/512e973a9ce4_initial_schema.py
@@ -1,15 +1,15 @@
-"""initial schema
+"""initial_schema
 
-Revision ID: 15182b58521a
+Revision ID: 512e973a9ce4
 Revises:
-Create Date: 2025-02-11 20:09:20.011634
+Create Date: 2025-02-18 16:37:04.346002
 
 """
 
 from collections.abc import Sequence
 
 # revision identifiers, used by Alembic.
-revision: str = "15182b58521a"
+revision: str = "512e973a9ce4"
 down_revision: str | None = None
 branch_labels: str | Sequence[str] | None = None
 depends_on: str | Sequence[str] | None = None


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to