fix: accept OneTimeUse, and let the replay record honour it

Makefile

@@ -37,10 +37,10 @@ test: build deps/
 clean:
 	rm -rf *.so *.o xmlsec1-$(XMLSEC_VER)*
 
-saml.o: src/*.c
+saml.o: src/*.c src/*.h
 	$(CC) -c $(CFLAGS_ALL) -o saml.o src/saml.c
 
-lua_saml.o: src/lua_saml.c
+lua_saml.o: src/lua_saml.c src/*.h
 	$(CC) -c $(CFLAGS_ALL) -I$(LUA_INCDIR) -Isrc/ -o $@ $<
 
 saml.so: lua_saml.o saml.o

README.md

@@ -141,12 +141,18 @@ the delivery window is minutes and the assertion window at most an hour, and the
 alternative is a record nothing reclaims. The limit an operator can move is
 `replay_ttl`; the day cap is fixed.
 
-**Two things it deliberately does not do.** An assertion carrying `<saml:OneTimeUse/>`
-is still refused outright, so an IdP asking for exactly this protection cannot log in
-even with the option on; that is tracked separately and the two do not meet yet. And
-re-submitting a response that already logged in is refused, which is what a browser
-does when it loses the redirect that ends a login. Returning to the application starts
-a fresh login, and the IdP will not ask for a password again.
+**This is what `<saml:OneTimeUse/>` asks for.** An IdP stamps that condition on an
+assertion to ask the SP to keep exactly this record. SAML Core 2.5.1.5 makes the
+condition always valid, a condition on use rather than on validity, so the login goes
+through with or without the option. With it, the assertion is single-use within the
+bounds above. Without it, the login is accepted and a line at `warn` level names `replay_dict`,
+so an IdP that asks for this is the signal to set it; a deployment logging at `error`
+or above does not see it.
+
+**One thing it deliberately does not do.** Re-submitting a response that already logged
+in is refused, which is what a browser does when it loses the redirect that ends a
+login. Returning to the application starts a fresh login, and the IdP will not ask for
+a password again.
 
 #### Seeding the worker
 

lua/resty/saml.lua

@@ -405,7 +405,7 @@ end
 
 -- Every top-level assertion the verified signature left in the document is one
 -- the readers draw identity from, so every one of them has to hold up.
-local function assertions_acceptable(opts, assertions, expected, now)
+local function assertions_acceptable(opts, assertions, expected, now, replay_dict)
     local skew = opts.clock_skew or DEFAULT_CLOCK_SKEW
     local accepted = opts.sp_audiences or { opts.sp_issuer }
 
@@ -419,6 +419,16 @@ local function assertions_acceptable(opts, assertions, expected, now)
                 assertion.unknown_condition
         end
 
+        -- Core 2.5.1.5: OneTimeUse is always valid, and asks the SP to keep a
+        -- record of the assertions it has spent. replay_dict is that record;
+        -- without it the IdP's request goes unmet, and the operator is told
+        -- what to configure rather than the user refused. The handle is the
+        -- one the last gate enforces on, so the two cannot disagree
+        if assertion.one_time_use and not replay_dict then
+            ngx.log(ngx.WARN, "assertion ", loggable(assertion.id),
+                " carries OneTimeUse, which this SP cannot enforce without replay_dict")
+        end
+
         local ok, err = time_bounds_ok(assertion.not_before, assertion.not_on_or_after, now, skew)
         if not ok then
             return false, where .. err
@@ -599,6 +609,15 @@ local function spend_assertions(dict, opts, assertions, expected, now)
             ttl = MAX_REPLAY_TTL
         end
 
+        -- the record is bounded where acceptance is not, so past it the
+        -- assertion is accepted again. An IdP that asked for single use is
+        -- told, since it is the IdP's window that made the record fall short
+        if assertion.one_time_use and (usable_until == nil or ttl == MAX_REPLAY_TTL) then
+            ngx.log(ngx.WARN, "assertion ", loggable(assertion.id),
+                " carries OneTimeUse but stays acceptable past its record, which lapses in ",
+                ttl, " seconds")
+        end
+
         local key = replay_key(opts, assertion)
         local added, add_err = dict:safe_add(key, true, ttl)
         if added then
@@ -613,7 +632,8 @@ local function spend_assertions(dict, opts, assertions, expected, now)
         else
             ngx.log(ngx.ERR, "could not remember assertion ", loggable(assertion.id), " in ",
                 opts.replay_dict, ": ", add_err,
-                ", this login is not covered by replay tracking")
+                ", this login is not covered by replay tracking",
+                assertion.one_time_use and " though it carries OneTimeUse" or "")
         end
     end
 
@@ -705,7 +725,8 @@ local function login_callback(self, opts)
     end
 
     local now = ngx.time()
-    local acceptable, reason = assertions_acceptable(opts, assertions, expected, now)
+    local acceptable, reason = assertions_acceptable(opts, assertions, expected, now,
+        self.replay_dict)
     if not acceptable then
         ngx.log(ngx.ERR, "response from IdP rejected: ", loggable(reason))
         ngx.exit(ngx.HTTP_UNAUTHORIZED)

src/lua_saml.c

@@ -698,6 +698,7 @@ static int doc_assertions(lua_State* L) {
     set_str_field(L, "id", a->id);
     set_str_field(L, "issuer", a->issuer);
     set_bool_field(L, "has_conditions", a->has_conditions);
+    set_bool_field(L, "one_time_use", a->one_time_use);
     set_str_field(L, "not_before", a->not_before);
     set_str_field(L, "not_on_or_after", a->not_on_or_after);
     set_str_field(L, "unknown_condition", a->unknown_condition);

src/saml.h

@@ -59,6 +59,7 @@ typedef struct {
   xmlChar* id;
   xmlChar* issuer;
   int has_conditions;
+  int one_time_use;
   xmlChar* not_before;
   xmlChar* not_on_or_after;
   xmlChar* unknown_condition;

src/xml.c

@@ -396,18 +396,19 @@ static size_t count_assertion_el(xmlNode* parent, const char* name) {
 }
 
 
-// Conditions this SP can actually satisfy. SAML Core 2.5.1 makes an assertion
+// Conditions this SP understands. SAML Core 2.5.1 makes an assertion
 // carrying any other one Indeterminate rather than valid, so everything else is
 // reported for the caller to refuse.
 //
-// ProxyRestriction is here because it binds an IdP issuing on behalf of another
-// IdP and asks nothing of the SP consuming the assertion. OneTimeUse is not,
-// because honouring it means remembering which assertions have been spent, and
-// Core 2.5.1.5 tells a party that cannot keep that record to treat the
-// assertion as invalid.
+// ProxyRestriction binds an IdP issuing on behalf of another IdP and asks
+// nothing of the SP consuming the assertion. OneTimeUse is always valid by
+// Core 2.5.1.5, a condition on use rather than on validity: it asks the SP to
+// keep a record of the assertions it has spent, which the caller has or has
+// not, so it is reported as a flag.
 static int is_known_condition(xmlNode* node) {
   return is_assertion_el(node, "AudienceRestriction") ||
-    is_assertion_el(node, "ProxyRestriction");
+    is_assertion_el(node, "ProxyRestriction") ||
+    is_assertion_el(node, "OneTimeUse");
 }
 
 
@@ -510,14 +511,18 @@ static int read_assertion(xmlDoc* doc, xmlNode* node, saml_assertion_t* a) {
     }
 
     for (xmlNode* child = conditions->children; child != NULL; child = child->next) {
-      if (child->type == XML_ELEMENT_NODE && !is_known_condition(child)) {
+      if (is_assertion_el(child, "OneTimeUse")) {
+        a->one_time_use = 1;
+      }
+      if (child->type == XML_ELEMENT_NODE && !is_known_condition(child) &&
+          a->unknown_condition == NULL) {
         // the caller refuses the assertion on this name, so losing it would
-        // let the condition through rather than fail the read
+        // let the condition through rather than fail the read. The scan goes
+        // on so what else the assertion carries is read whatever the order
         a->unknown_condition = xmlStrdup(child->name);
         if (a->unknown_condition == NULL) {
           return -1;
         }
-        break;
       }
     }
 

t/assertion-conditions.t

@@ -574,9 +574,10 @@ offers no subject confirmation this SP can satisfy
             ngx.say(login_with("plain", saml_response({
                 conditions = conditions({ body = "<saml:ProxyRestriction Count=\"1\"/>" }),
             })))
-            -- OneTimeUse asks this SP to remember which assertions it has spent
+            -- OneTimeUse is always valid (Core 2.5.1.5); with no replay_dict it
+            -- asks for a record this SP does not keep, which is said, not refused
             ngx.say(login_with("plain", saml_response({
-                conditions = conditions({ body = "<saml:OneTimeUse/>" }),
+                id = "single", conditions = conditions({ body = "<saml:OneTimeUse/>" }),
             })))
             -- and a condition it has never heard of asks who knows what
             ngx.say(login_with("plain", saml_response({
@@ -589,10 +590,10 @@ offers no subject confirmation this SP can satisfy
     }
 --- response_body
 302 /
-401 nil
+302 /
 401 nil
 --- error_log eval
-[qr/carries a condition this SP cannot satisfy: OneTimeUse/,
+[qr/\[warn\] .* assertion single carries OneTimeUse, which this SP cannot enforce without replay_dict/,
 qr/carries a condition this SP cannot satisfy: Condition/]
 
 
@@ -631,7 +632,7 @@ response from IdP is addressed to http://evil.example.com/acs
         content_by_lua_block {
             local xml = sign_doc(response(
                 assertion({ id = "a1", conditions = conditions({ not_on_or_after = "2026-07-21T00:00:00Z",
-                    body = audience("sp") }) }) ..
+                    body = audience("sp") .. "<saml:OneTimeUse/>" }) }) ..
                 assertion({ id = "a2", name_id = "second@example.com",
                     confirmations = confirmation({ recipient = ACS }) })))
             local doc, err = parse(xml)
@@ -641,14 +642,15 @@ response from IdP is addressed to http://evil.example.com/acs
                 ngx.say(a.id, " conditions=", tostring(a.has_conditions),
                     " expires=", tostring(a.not_on_or_after),
                     " audiences=", #a.audience_restrictions,
-                    " confirmations=", #a.subject_confirmations)
+                    " confirmations=", #a.subject_confirmations,
+                    " one_time_use=", tostring(a.one_time_use))
             end
             ngx.say("destination: ", tostring(saml.doc_destination(doc)))
         }
     }
 --- response_body
-a1 conditions=true expires=2026-07-21T00:00:00Z audiences=1 confirmations=0
-a2 conditions=false expires=nil audiences=0 confirmations=1
+a1 conditions=true expires=2026-07-21T00:00:00Z audiences=1 confirmations=0 one_time_use=true
+a2 conditions=false expires=nil audiences=0 confirmations=1 one_time_use=false
 destination: nil
 
 
@@ -1386,3 +1388,135 @@ earlier: true
 --- response_body
 302 /
 dated one decides: true
+
+
+
+=== TEST 48: with a record, an OneTimeUse assertion is treated like any other
+--- config
+    location /t {
+        content_by_lua_block {
+            ngx.shared.saml_replay:flush_all()
+            local xml = saml_response({
+                id = "stamped",
+                conditions = conditions({ not_on_or_after = at(600), body = "<saml:OneTimeUse/>" }),
+            })
+            ngx.say(login_with("replay", xml))
+            -- remembered until acceptance ends plus clock_skew, as any other
+            local ttl = ngx.shared.saml_replay:ttl(replay_key("stamped"))
+            ngx.say("recorded: ", ttl > 650 and ttl <= 660)
+            ngx.say(login_with("replay", xml))
+        }
+    }
+--- response_body
+302 /
+recorded: true
+401 nil
+--- error_log
+assertion stamped has been presented already
+--- no_error_log
+[crit]
+[alert]
+[emerg]
+OneTimeUse
+
+
+
+=== TEST 49: a full dict says when the untracked login asked for single use
+--- config
+    location /t {
+        content_by_lua_block {
+            local dict = ngx.shared.saml_replay_full
+            dict:flush_all()
+            dict:flush_expired()
+            local filler = string.rep("x", 256)
+            local i, ok = 0, true
+            while ok do
+                ok = dict:safe_set("filler-" .. i, filler, 600)
+                if ok then i = i + 1 end
+                if i > 5000 then break end
+            end
+            local j = 0
+            while dict:safe_add("small-" .. j, true, 600) do
+                j = j + 1
+                if j > 5000 then break end
+            end
+            ngx.say(login_with("replay_full", saml_response({
+                id = "untracked-stamped",
+                conditions = conditions({ body = "<saml:OneTimeUse/>" }),
+            })))
+        }
+    }
+--- response_body
+302 /
+--- error_log
+in saml_replay_full: no memory, this login is not covered by replay tracking though it carries OneTimeUse
+
+
+
+=== TEST 50: an OneTimeUse assertion that outlives its record says so
+--- config
+    location /t {
+        content_by_lua_block {
+            ngx.shared.saml_replay:flush_all()
+            -- nothing bounds it, so the record falls back to replay_ttl
+            ngx.say(login_with("replay", saml_response({
+                id = "stamped-unbounded",
+                conditions = conditions({ body = "<saml:OneTimeUse/>" }),
+            })))
+            -- valid for years, so the record is capped at a day
+            ngx.say(login_with("replay", saml_response({
+                id = "stamped-forever",
+                conditions = conditions({ not_on_or_after = "9999-12-31T23:59:59Z",
+                    body = "<saml:OneTimeUse/>" }),
+            })))
+        }
+    }
+--- response_body
+302 /
+302 /
+--- error_log eval
+[qr/\[warn\] .* assertion stamped-unbounded carries OneTimeUse but stays acceptable past its record, which lapses in 600 seconds/,
+qr/\[warn\] .* assertion stamped-forever carries OneTimeUse but stays acceptable past its record, which lapses in 86400 seconds/]
+
+
+
+=== TEST 51: OneTimeUse is read wherever it sits among the conditions
+--- config
+    location /t {
+        content_by_lua_block {
+            local unknown = '<saml:Condition xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" ' ..
+                'xsi:type="saml:AudienceRestrictionType"><saml:Audience>sp</saml:Audience></saml:Condition>'
+            for _, body in ipairs({ "<saml:OneTimeUse/>" .. unknown, unknown .. "<saml:OneTimeUse/>" }) do
+                local doc, err = parse(sign_doc(response(assertion({
+                    id = "ordered", conditions = conditions({ body = body }),
+                }))))
+                if err then ngx.say("err: ", err) return end
+                local a = saml.doc_assertions(doc)[1]
+                ngx.say("one_time_use=", tostring(a.one_time_use),
+                    " unknown_condition=", tostring(a.unknown_condition))
+            end
+        }
+    }
+--- response_body
+one_time_use=true unknown_condition=Condition
+one_time_use=true unknown_condition=Condition
+
+
+
+=== TEST 52: without a record, an OneTimeUse assertion is accepted again
+--- config
+    location /t {
+        content_by_lua_block {
+            local xml = saml_response({
+                id = "stamped-untracked",
+                conditions = conditions({ not_on_or_after = at(600), body = "<saml:OneTimeUse/>" }),
+            })
+            ngx.say(login_with("plain", xml))
+            ngx.say(login_with("plain", xml))
+        }
+    }
+--- response_body
+302 /
+302 /
+--- error_log eval
+qr/\[warn\] .* assertion stamped-untracked carries OneTimeUse, which this SP cannot enforce without replay_dict/