From 80999907e817b5bb3b97874ff74ea0c0e21db441 Mon Sep 17 00:00:00 2001
From: Charles Severance <csev@umich.edu>
Date: Wed, 15 May 2024 13:27:54 -0400
Subject: [PATCH] Fix error where keys were not being rotated

---
 src/Core/Keyset.php | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/src/Core/Keyset.php b/src/Core/Keyset.php
index c69705ea..c2eca3e0 100644
--- a/src/Core/Keyset.php
+++ b/src/Core/Keyset.php
@@ -10,15 +10,14 @@
  * Helper class for Using Tsugi's internal global outgoing keyset
  */
 
-// TODO: Make a configuration option and truncate the table in a MYSQL and PostgreSQL friendly way
-// Also make an admin UI to manage this - but for now there will be reasonble data in this table to build code upon
-
 // TODO: Add a private key encryption regime like Sakai
 
 class Keyset {
 
      public static $ttl = 10*60;
      public static $expire = 5*60;
+     // public static $expire = 0; // Debug only
+     public static $verbose = false;  // Debug only
 
      // Auto populate and/or rotate the lti_keyset data
      public static function maintain() {
@@ -27,9 +26,12 @@ public static function maintain() {
         $now = time();
         $apc_check = U::appCacheGet('keyset_last_check', 0);
 
+        $privkey = U::appCacheGet('keyset_privkey', null);
+        $kid = U::appCacheGet('keyset_kid', null);
+
         $delta = abs($now-$apc_check);
-        if ( $apc_check > 0 && $delta < self::$expire ) {
-            // error_log("Keyset::maintain Last key rotation check seconds=".$delta);
+        if ( is_string($kid) && is_string($privkey) && $apc_check > 0 && $delta < self::$expire ) {
+            if ( self::$verbose ) error_log("Keyset::maintain Last key rotation check seconds=".$delta);
             return;
         }
 
@@ -43,7 +45,7 @@ public static function maintain() {
             $now = new \DateTime($rows[0]['now']);
             $create = new \DateTime($rows[0]['created_at']);
             $delta = $now->diff($create, true);
-            $days = $delta->d;
+            $days = $delta->days;
         }
 
         if ( $days == -1 || $days >= 30) {
@@ -56,12 +58,16 @@ public static function maintain() {
                 VALUES (:title, :pubkey, :privkey)
             ";
             $values = array(
-                ":title" => 'From lti/database.php',
+                ":title" => 'From Keyset:maintain()',
                 ":pubkey" => $publicKey,
                 ":privkey" => $privateKey,
             );
             $stmt = $PDOX->queryReturnError($sql, $values);
 
+            if ( $stmt->rowCount() > 0 ) {
+                error_log("KeySet::maintain table cleanup rows=".$stmt->rowCount());
+            }
+
             $kid = LTIX::getKidForKey($publicKey);
             error_log("Keyset::maintain Key rotated days=".$days." new kid=".$kid);
 
@@ -69,12 +75,17 @@ public static function maintain() {
                 error_log("Keyset::maintain Unable to insert new key into keyset\n");
                 return;
             }
+
+            // Clean up very old records
+            $stmt = $PDOX->queryDie("DELETE FROM {$CFG->dbprefix}lti_keyset WHERE
+                 created_at < (CURDATE() - INTERVAL 1 MONTH);");
+
         } else {
-            error_log("Keyset::maintain No key rotation necessary days=".$days);
+            if ( self::$verbose ) error_log("Keyset::maintain No key rotation necessary days=".$days);
         }
      }
 
-     // Get the private key and kid
+     // Get the private key and kid - call by reference
      public static function getSigning(&$privkey, &$kid) {
         global $PDOX, $CFG;
 
@@ -89,7 +100,7 @@ public static function getSigning(&$privkey, &$kid) {
         // No more than once per expiration period
         $delta = abs($now-$last_load);
         if ( is_string($kid) && is_string($privkey) && $delta < self::$expire ) {
-            // error_log("Keyset::getSigning cache hit seconds=".$delta);
+            if ( self::$verbose ) error_log("Keyset::getSigning cache hit seconds=".$delta);
             return;
         }
 
@@ -99,7 +110,7 @@ public static function getSigning(&$privkey, &$kid) {
         $privkey = LTIX::decrypt_secret($row['privkey']);
         $pubkey = $row['pubkey'];
         $kid = LTIX::getKidForKey($pubkey);
-        error_log("Keyset::getSigning loaded key from database now=".$now." kid=".$kid);
+        if ( self::$verbose ) error_log("Keyset::getSigning loaded key from database now=".$now." kid=".$kid);
 
         // Save for later
         if ( is_string($kid) && is_string($privkey)) {
@@ -116,8 +127,10 @@ public static function getSigning(&$privkey, &$kid) {
     public static function getCurrentKeys() {
         global $PDOX, $CFG;
 
-        // TODO: Once we are convident this table exists switch to allRowsDie()
-        $stmt = $PDOX->queryReturnError(
+        // Make sure we have a key and it is recent
+        self::maintain();
+
+        $stmt = $PDOX->queryDie(
         "SELECT pubkey FROM {$CFG->dbprefix}lti_keyset
             WHERE deleted = 0 AND pubkey IS NOT NULL AND privkey IS NOT NULL
             ORDER BY created_at DESC LIMIT 3"