fix: preserve credential iteration state during GET RESPONSE chaining - #55
Merged
BryanJacobs merged 1 commit intoMar 20, 2026
Merged
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
When enumerating resident credentials via ENUMERATE_CREDS_BEGIN followed
by one or more ENUMERATE_CREDS_NEXT calls, the credential iteration
pointer could be silently cleared mid-enumeration if the BEGIN response
required more than one GET RESPONSE exchange to be fully delivered.
Root cause
----------
streamOutgoingContinuation() is called for each GET RESPONSE APDU
(INS=0xC0) the platform sends to retrieve a chained response. When a
chunk is sent and further data remains, the method signals this by
calling throwException() with SW=61xx. However, it used the single-
argument overload of throwException(), which defaults to
clearIteration=true and unconditionally calls
transientStorage.clearIterationPointers() before throwing the ISO
exception.
This meant that every intermediate GET RESPONSE that still had bytes
remaining would destroy the credential iteration pointer that had just
been set by handleEnumerateCreds(). A concrete example with a ~523-byte
ENUMERATE_CREDS_BEGIN response:
1. handleEnumerateCreds() sets credIterationPointer = 1
2. doSendResponse() sends 256 bytes, calls setupChainedResponse()
with clearIteration=false → pointer preserved (= 1)
3. Platform sends GET RESPONSE BryanJacobs#1 → streamOutgoingContinuation()
sends 256 bytes, 11 remain → throwException(SW=610b)
[single-arg → clearIteration=true] → pointer CLEARED (= 0)
4. Platform sends GET RESPONSE BryanJacobs#2 → streamOutgoingContinuation()
sends final 11 bytes → SW=9000
5. Platform sends ENUMERATE_CREDS_NEXT → credPtr == 0 →
OPERATION_DENIED (0x27)
This caused silent data loss: a client receiving OPERATION_DENIED on
ENUMERATE_CREDS_NEXT would break out of its enumeration loop early,
dropping all subsequent credentials for the current RP. Only credentials
returned by BEGIN were visible; all NEXT calls would fail.
The bug did not affect clients using extended-length APDUs,
because doSendResponse() detects isExtendedAPDU
and streams the entire response in a single burst via
streamOutgoingContinuation(chaining=false), which never calls
throwException() at all.
Fix
---
Change both throwException() calls in the chaining branch of
streamOutgoingContinuation() to use the two-argument overload with
clearIteration=false. GET RESPONSE is a continuation of an already-
initiated command response, not a new command; it must never disturb
enumeration state.
- throwException(ISO7816.SW_BYTES_REMAINING_00, false)
- throwException((short)(ISO7816.SW_BYTES_REMAINING_00 + remaining), false)
The first call (remaining >= 256) was already using false in some
versions; both are made explicit here for consistency and correctness.
Copy link
Copy Markdown
Contributor
Author
|
For the context: I am working on a modified version of Yubikey Manager adapted to support smartcard devices with the FIDO2 applet. When listing credentials using ykman fido credentials list, I get the fido2.ctap.CtapError: CTAP error: 0x27 - OPERATION_DENIED error. Using this patch seems to correct this issue and correctly list all the resident passkeys stored in the card. |
Sorry, something went wrong.
Copy link
Copy Markdown
Owner
|
Good find. This looks right to me. Thanks! |
Sorry, something went wrong.
Headcrabed
added a commit
to Headcrabed/pin_plus_firmware
that referenced
this pull request
Mar 21, 2026
This commit is ported from upstream repo: BryanJacobs/FIDO2Applet#55 Original commit description: When enumerating resident credentials via ENUMERATE_CREDS_BEGIN followed by one or more ENUMERATE_CREDS_NEXT calls, the credential iteration pointer could be silently cleared mid-enumeration if the BEGIN response required more than one GET RESPONSE exchange to be fully delivered. Root cause ---------- streamOutgoingContinuation() is called for each GET RESPONSE APDU (INS=0xC0) the platform sends to retrieve a chained response. When a chunk is sent and further data remains, the method signals this by calling throwException() with SW=61xx. However, it used the single- argument overload of throwException(), which defaults to clearIteration=true and unconditionally calls transientStorage.clearIterationPointers() before throwing the ISO exception. This meant that every intermediate GET RESPONSE that still had bytes remaining would destroy the credential iteration pointer that had just been set by handleEnumerateCreds(). A concrete example with a ~523-byte ENUMERATE_CREDS_BEGIN response: 1. handleEnumerateCreds() sets credIterationPointer = 1 2. doSendResponse() sends 256 bytes, calls setupChainedResponse() with clearIteration=false → pointer preserved (= 1) 3. Platform sends GET RESPONSE token2#1 → streamOutgoingContinuation() sends 256 bytes, 11 remain → throwException(SW=610b) [single-arg → clearIteration=true] → pointer CLEARED (= 0) 4. Platform sends GET RESPONSE token2#2 → streamOutgoingContinuation() sends final 11 bytes → SW=9000 5. Platform sends ENUMERATE_CREDS_NEXT → credPtr == 0 → OPERATION_DENIED (0x27) This caused silent data loss: a client receiving OPERATION_DENIED on ENUMERATE_CREDS_NEXT would break out of its enumeration loop early, dropping all subsequent credentials for the current RP. Only credentials returned by BEGIN were visible; all NEXT calls would fail. The bug did not affect clients using extended-length APDUs, because doSendResponse() detects isExtendedAPDU and streams the entire response in a single burst via streamOutgoingContinuation(chaining=false), which never calls throwException() at all. Fix --- Change both throwException() calls in the chaining branch of streamOutgoingContinuation() to use the two-argument overload with clearIteration=false. GET RESPONSE is a continuation of an already- initiated command response, not a new command; it must never disturb enumeration state. throwException(ISO7816.SW_BYTES_REMAINING_00, false) throwException((short)(ISO7816.SW_BYTES_REMAINING_00 + remaining), false) The first call (remaining >= 256) was already using false in some versions; both are made explicit here for consistency and correctness. Original author: Toporin https://github.com/Toporin Signed-off-by: Shengyu Qu <wiagn233@outlook.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When enumerating resident credentials via ENUMERATE_CREDS_BEGIN followed by one or more ENUMERATE_CREDS_NEXT calls, the credential iteration pointer could be silently cleared mid-enumeration if the BEGIN response required more than one GET RESPONSE exchange to be fully delivered.
Root cause
streamOutgoingContinuation() is called for each GET RESPONSE APDU (INS=0xC0) the platform sends to retrieve a chained response. When a chunk is sent and further data remains, the method signals this by calling throwException() with SW=61xx. However, it used the single- argument overload of throwException(), which defaults to clearIteration=true and unconditionally calls transientStorage.clearIterationPointers() before throwing the ISO exception.
This meant that every intermediate GET RESPONSE that still had bytes remaining would destroy the credential iteration pointer that had just been set by handleEnumerateCreds(). A concrete example with a ~523-byte ENUMERATE_CREDS_BEGIN response:
This caused silent data loss: a client receiving OPERATION_DENIED on ENUMERATE_CREDS_NEXT would break out of its enumeration loop early, dropping all subsequent credentials for the current RP. Only credentials returned by BEGIN were visible; all NEXT calls would fail.
The bug did not affect clients using extended-length APDUs, because doSendResponse() detects isExtendedAPDU
and streams the entire response in a single burst via streamOutgoingContinuation(chaining=false), which never calls throwException() at all.
Fix
Change both throwException() calls in the chaining branch of streamOutgoingContinuation() to use the two-argument overload with clearIteration=false. GET RESPONSE is a continuation of an already- initiated command response, not a new command; it must never disturb enumeration state.