Skip to content

Commit

Permalink
Fix the Keyset rotation code
Browse files Browse the repository at this point in the history
  • Loading branch information
csev committed May 15, 2024
1 parent 6cdfe85 commit e68ce1f
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 31 deletions.
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ Mostly because PHP is evolving from version 7.0 to version 9.0 in ways that brea
dependencies, we are maintaining several long term branches for those stuck on older versions
of PHP - we maintain these as branches.

* Branch: master - Requires PHP 8.2 or greater - probably wqould work on 8.1 but not 8.0
* Branch: master - Requires PHP 8.2 or greater - probably would work on 8.1 but not 8.0

* Branch: php-80-x - If you are running PHP 8.0 or a late version of PHP 7.x this is a good branch

* Branch: php-74-x - This branch a little earlier than php-80-x and should run on late versions of
PHP 7.x and PHP 8.0
* Branch: php-74-x - This branch is a little earlier than php-80-x and should
run on late versions of PHP 7.x and PHP 8.0

* Branch: php-72-x - If you have a system that is lower than PHP 7.4, this is a safe branch

Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"react/dns" : ">=1.12.0",
"react/socket" : ">=1.15.0",

"tsugi/lib": "dev-master#25d7c67051c37e4121700e121415d19ab7dc7435",
"tsugi/lib": "dev-master#80999907e817b5bb3b97874ff74ea0c0e21db441",
"koseu/lib": "dev-master#70c7ac1ca413c2dd541e078ebe07719405621b1b"
},
"config": {
Expand Down
10 changes: 5 additions & 5 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion config-dist.php
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ function __the_end(){
// then git is not setup in your path
// (Control Panel > System and Security > System > Advanced System Settings > Environment Variables)
// (3) Then here in "config.php":
// $CFG->git_command = 'git'
// $CFG->git_command = 'git';

// In order to run git from the a PHP script, we may need a setuid version
// of git - example commands if you are not root:
Expand Down
2 changes: 1 addition & 1 deletion settings/key/auto_common.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@
$tool->product_family_code = "tsugi.org";
$tool->target_link_uri = $CFG->wwwroot . '/lti/store/';

$pieces = parse_url($CFG->apphome);
$pieces = parse_url(is_string($CFG->apphome) ? $CFG->apphome : $CFG->apphome);
if ( U::get($pieces, 'host') ) $tool->domain = U::get($pieces, 'host');

if ( isset($CFG->servicedesc) && $CFG->servicedesc ) {
Expand Down
8 changes: 4 additions & 4 deletions vendor/composer/installed.json
Original file line number Diff line number Diff line change
Expand Up @@ -7708,12 +7708,12 @@
"source": {
"type": "git",
"url": "https://github.com/tsugiproject/tsugi-php.git",
"reference": "25d7c67051c37e4121700e121415d19ab7dc7435"
"reference": "80999907e817b5bb3b97874ff74ea0c0e21db441"
},
"dist": {
"type": "zip",
"url": "https://api.github.com/repos/tsugiproject/tsugi-php/zipball/25d7c67051c37e4121700e121415d19ab7dc7435",
"reference": "25d7c67051c37e4121700e121415d19ab7dc7435",
"url": "https://api.github.com/repos/tsugiproject/tsugi-php/zipball/80999907e817b5bb3b97874ff74ea0c0e21db441",
"reference": "80999907e817b5bb3b97874ff74ea0c0e21db441",
"shasum": ""
},
"require": {
Expand All @@ -7727,7 +7727,7 @@
"phpunit/php-timer": "v5.0.3",
"phpunit/phpunit": "9.*"
},
"time": "2024-05-15T15:05:31+00:00",
"time": "2024-05-15T17:27:54+00:00",
"default-branch": true,
"type": "library",
"installation-source": "dist",
Expand Down
6 changes: 3 additions & 3 deletions vendor/composer/installed.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
'name' => '__root__',
'pretty_version' => 'dev-master',
'version' => 'dev-master',
'reference' => '6de2585bae0a3f3ff57f012326c01fc85978dcb8',
'reference' => '6cdfe85802dd137411183d441d81f9221ad6a434',
'type' => 'library',
'install_path' => __DIR__ . '/../../',
'aliases' => array(),
Expand All @@ -13,7 +13,7 @@
'__root__' => array(
'pretty_version' => 'dev-master',
'version' => 'dev-master',
'reference' => '6de2585bae0a3f3ff57f012326c01fc85978dcb8',
'reference' => '6cdfe85802dd137411183d441d81f9221ad6a434',
'type' => 'library',
'install_path' => __DIR__ . '/../../',
'aliases' => array(),
Expand Down Expand Up @@ -1078,7 +1078,7 @@
'tsugi/lib' => array(
'pretty_version' => 'dev-master',
'version' => 'dev-master',
'reference' => '25d7c67051c37e4121700e121415d19ab7dc7435',
'reference' => '80999907e817b5bb3b97874ff74ea0c0e21db441',
'type' => 'library',
'install_path' => __DIR__ . '/../tsugi/lib',
'aliases' => array(
Expand Down
39 changes: 26 additions & 13 deletions vendor/tsugi/lib/src/Core/Keyset.php
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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;
}

Expand All @@ -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) {
Expand All @@ -56,25 +58,34 @@ 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);

if ( ! $stmt->success ) {
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;

Expand All @@ -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;
}

Expand All @@ -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)) {
Expand All @@ -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"
Expand Down

0 comments on commit e68ce1f

Please sign in to comment.