From 81b8243e1dbd063d76e608041e27ee1fffa5a5ae Mon Sep 17 00:00:00 2001
From: Brady Wetherington <bwetherington@grokability.com>
Date: Thu, 25 Jan 2024 19:53:24 +0000
Subject: [PATCH] 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 @@
+<?php
+
+namespace App\Console\Commands;
+
+use Illuminate\Console\Command;
+use App\Models\SamlNonce;
+
+class SamlClearExpiredNonces extends Command
+{
+    /**
+     * The name and signature of the console command.
+     *
+     * @var string
+     */
+    protected $signature = 'saml:clear_expired_nonces';
+
+    /**
+     * The console command description.
+     *
+     * @var string
+     */
+    protected $description = 'Command description';
+
+    /**
+     * Create a new command instance.
+     *
+     * @return void
+     */
+    public function __construct()
+    {
+        parent::__construct();
+    }
+
+    /**
+     * Execute the console command.
+     *
+     * @return int
+     */
+    public function handle()
+    {
+        SamlNonce::where('not_valid_after','<=',now())->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 @@
+<?php
+
+namespace App\Models;
+
+use Illuminate\Database\Eloquent\Factories\HasFactory;
+use Illuminate\Database\Eloquent\Model;
+
+class SamlNonce extends Model
+{
+    use HasFactory;
+
+    protected $fillable = ['nonce','not_on_or_after'];
+
+    public $timestamps = false;
+}
diff --git a/app/Services/Saml.php b/app/Services/Saml.php
index f80b1c1fb9..b34501366a 100644
--- a/app/Services/Saml.php
+++ b/app/Services/Saml.php
@@ -394,6 +394,8 @@ class Saml
             'nameIdSPNameQualifier' => $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 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+class CreateSamlNonceTable extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        Schema::create('saml_nonces', function (Blueprint $table) {
+            $table->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');
+    }
+}