From 81b8243e1dbd063d76e608041e27ee1fffa5a5ae Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Thu, 25 Jan 2024 19:53:24 +0000 Subject: [PATCH 1/3] Enforce SAML assertion ID uniqueness and notValidOnOrAfter attribute --- .../Commands/SamlClearExpiredNonces.php | 44 +++++++++++++++++++ app/Console/Kernel.php | 1 + app/Http/Controllers/Auth/LoginController.php | 18 ++++++-- app/Models/SamlNonce.php | 15 +++++++ app/Services/Saml.php | 2 + ...4_01_24_145544_create_saml_nonce_table.php | 33 ++++++++++++++ 6 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 app/Console/Commands/SamlClearExpiredNonces.php create mode 100644 app/Models/SamlNonce.php create mode 100644 database/migrations/2024_01_24_145544_create_saml_nonce_table.php diff --git a/app/Console/Commands/SamlClearExpiredNonces.php b/app/Console/Commands/SamlClearExpiredNonces.php new file mode 100644 index 0000000000..ec20bc37a9 --- /dev/null +++ b/app/Console/Commands/SamlClearExpiredNonces.php @@ -0,0 +1,44 @@ +delete(); + return 0; + } +} diff --git a/app/Console/Kernel.php b/app/Console/Kernel.php index 0b80d2eccd..8d512f303b 100644 --- a/app/Console/Kernel.php +++ b/app/Console/Kernel.php @@ -25,6 +25,7 @@ class Kernel extends ConsoleKernel $schedule->command('backup:clean')->daily(); $schedule->command('snipeit:upcoming-audits')->daily(); $schedule->command('auth:clear-resets')->everyFifteenMinutes(); + $schedule->command('saml:clear_expired_nonces')->weekly(); } /** diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 100eed12b9..896ca11ff5 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -3,6 +3,7 @@ namespace App\Http\Controllers\Auth; use App\Http\Controllers\Controller; +use App\Models\SamlNonce; use App\Models\Setting; use App\Models\User; use App\Models\Ldap; @@ -109,7 +110,14 @@ class LoginController extends Controller try { $user = $saml->samlLogin($samlData); - + $notValidAfter = new \Carbon\Carbon(@$samlData['assertionNotOnOrAfter']); + if(\Carbon::now()->greaterThanOrEqualTo($notValidAfter)) { + abort(400,"Expired SAML Assertion"); + } + if(SamlNonce::where('nonce', @$samlData['nonce'])->count() > 0) { + abort(400,"Assertion has already been used"); + } + Log::debug("okay, fine, this is a new nonce then. Good for you."); if (!is_null($user)) { Auth::login($user); } else { @@ -123,10 +131,14 @@ class LoginController extends Controller $user->last_login = \Carbon::now(); $user->saveQuietly(); } - + $s = new SamlNonce(); + $s->nonce = @$samlData['nonce']; + $s->not_valid_after = $notValidAfter; + $s->save(); + } catch (\Exception $e) { \Log::debug('There was an error authenticating the SAML user: '.$e->getMessage()); - throw new \Exception($e->getMessage()); + throw $e; } // Fallthrough with better logging diff --git a/app/Models/SamlNonce.php b/app/Models/SamlNonce.php new file mode 100644 index 0000000000..6eb05352d8 --- /dev/null +++ b/app/Models/SamlNonce.php @@ -0,0 +1,15 @@ + $auth->getNameIdSPNameQualifier(), 'sessionIndex' => $auth->getSessionIndex(), 'sessionExpiration' => $auth->getSessionExpiration(), + 'nonce' => $auth->getLastAssertionId(), + 'assertionNotOnOrAfter' => $auth->getLastAssertionNotOnOrAfter(), ]; } diff --git a/database/migrations/2024_01_24_145544_create_saml_nonce_table.php b/database/migrations/2024_01_24_145544_create_saml_nonce_table.php new file mode 100644 index 0000000000..92c3a16358 --- /dev/null +++ b/database/migrations/2024_01_24_145544_create_saml_nonce_table.php @@ -0,0 +1,33 @@ +id(); + $table->string('nonce')->index(); + $table->datetime('not_valid_after')->index(); + //$table->timestamps(); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::dropIfExists('saml_nonces'); + } +} From b39b575fecb05720ef6e6fcb4fd38693e07b3124 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Thu, 25 Jan 2024 20:07:18 +0000 Subject: [PATCH 2/3] Add table-check; add command-description for Artisan command --- app/Console/Commands/SamlClearExpiredNonces.php | 2 +- .../2024_01_24_145544_create_saml_nonce_table.php | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/app/Console/Commands/SamlClearExpiredNonces.php b/app/Console/Commands/SamlClearExpiredNonces.php index ec20bc37a9..f03b55095e 100644 --- a/app/Console/Commands/SamlClearExpiredNonces.php +++ b/app/Console/Commands/SamlClearExpiredNonces.php @@ -19,7 +19,7 @@ class SamlClearExpiredNonces extends Command * * @var string */ - protected $description = 'Command description'; + protected $description = 'Clears out expired SAML assertions from the saml_nonces table'; /** * Create a new command instance. diff --git a/database/migrations/2024_01_24_145544_create_saml_nonce_table.php b/database/migrations/2024_01_24_145544_create_saml_nonce_table.php index 92c3a16358..f12615160e 100644 --- a/database/migrations/2024_01_24_145544_create_saml_nonce_table.php +++ b/database/migrations/2024_01_24_145544_create_saml_nonce_table.php @@ -13,12 +13,14 @@ class CreateSamlNonceTable extends Migration */ public function up() { - Schema::create('saml_nonces', function (Blueprint $table) { - $table->id(); - $table->string('nonce')->index(); - $table->datetime('not_valid_after')->index(); - //$table->timestamps(); - }); + if (! Schema::hasTable('saml_nonces') ) { + Schema::create('saml_nonces', function (Blueprint $table) { + $table->id(); + $table->string('nonce')->index(); + $table->datetime('not_valid_after')->index(); + //$table->timestamps(); + }); + } } /** From d3815ddce7e775de2dc85ef9f5be882c532152ea Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Thu, 25 Jan 2024 20:13:49 +0000 Subject: [PATCH 3/3] Remove commented-out timestamps on the saml_nonces table --- .../migrations/2024_01_24_145544_create_saml_nonce_table.php | 1 - 1 file changed, 1 deletion(-) diff --git a/database/migrations/2024_01_24_145544_create_saml_nonce_table.php b/database/migrations/2024_01_24_145544_create_saml_nonce_table.php index f12615160e..f6305288ee 100644 --- a/database/migrations/2024_01_24_145544_create_saml_nonce_table.php +++ b/database/migrations/2024_01_24_145544_create_saml_nonce_table.php @@ -18,7 +18,6 @@ class CreateSamlNonceTable extends Migration $table->id(); $table->string('nonce')->index(); $table->datetime('not_valid_after')->index(); - //$table->timestamps(); }); } }