Skip to content

Add initial certificate bank support and allow selecting the bank - #3629

Open
alistair23 wants to merge 3 commits into
DMTF:mainfrom
alistair23:alistair/bank-support
Open

Add initial certificate bank support and allow selecting the bank#3629
alistair23 wants to merge 3 commits into
DMTF:mainfrom
alistair23:alistair/bank-support

Conversation

@alistair23

Copy link
Copy Markdown
Contributor

SPDM 1.4 add support for the banked architecture. Basically allowing a number of banks of certificate slots.

Currently bank support can be handled by the implementer using the CONNECTION_STATE_NEGOTIATED callback registered with libspdm_register_connection_state_callback_func().

The problem with this is it pushes a lot of complexity back to the implementer and it makes supporting the Slot Management commands tricky, as the implementer will need to handle the commands as well.

Instead let's move the bank support into libspdm. Currently we just allow at build time a 2-D array of certificates, for banks and slots. We then allow the implementation to select the bank to use, which defaults to zero.

Future patches will work on the Slot Management commands, which can then operate on specific banks.

At build time we allow users to specify their own bank count, allowing smaller bank counts on size reduced systems.

@alistair23
alistair23 force-pushed the alistair/bank-support branch 13 times, most recently from cfa50b4 to 0484933 Compare May 25, 2026 05:35
Comment thread script/format_nix.sh
@alistair23
alistair23 force-pushed the alistair/bank-support branch from 0484933 to 5a803e8 Compare May 27, 2026 02:39
Comment thread include/library/spdm_lib_config.h Outdated
Comment thread include/internal/libspdm_common_lib.h Outdated
@jyao1

jyao1 commented Jun 2, 2026

Copy link
Copy Markdown
Member

SPDM 1.4 add support for the banked architecture. Basically allowing a number of banks of certificate slots.

Currently bank support can be handled by the implementer using the CONNECTION_STATE_NEGOTIATED callback registered with libspdm_register_connection_state_callback_func().

The problem with this is it pushes a lot of complexity back to the implementer and it makes supporting the Slot Management commands tricky, as the implementer will need to handle the commands as well.

Instead let's move the bank support into libspdm. Currently we just allow at build time a 2-D array of certificates, for banks and slots. We then allow the implementation to select the bank to use, which defaults to zero.

Future patches will work on the Slot Management commands, which can then operate on specific banks.

At build time we allow users to specify their own bank count, allowing smaller bank counts on size reduced systems.

I think we might want a high level design for bank management.
To me, there are too different ways:

  1. Put bank management into libspdm. That means, the integrator needs to register everything to libspdm. As such, READ operation does not need to reach integrator, while WRITE operation is write-through from libspdm to the integrator's permanent configuration (e.g. NVRAM).
  2. Put bank management out of libspdm. All bank management command (READ and WRITE) needs to pass-thru to integrator.

Bank management is similar to key pair info feature. Today we are using option 2) - see https://github.com/DMTF/libspdm/blob/main/include/hal/library/responder/key_pair_info.h

I feel we can start from option 2.

@alistair23

Copy link
Copy Markdown
Contributor Author
  • Put bank management out of libspdm. All bank management command (READ and WRITE) needs to pass-thru to integrator.

The issue here though is it's very complex managing the libspdm_local_context_t state in libspdm. The implementor needs to regularly update the state inside libspdm as well as handling the READ and WRITE commands.

Bank management is similar to key pair info feature. Today we are using option 2) - see https://github.com/DMTF/libspdm/blob/main/include/hal/library/responder/key_pair_info.h

I think key pairs are simpler then banks. Note that the bank approach I'm thinking about here will be similar to https://github.com/DMTF/libspdm/blob/main/include/hal/library/responder/key_pair_info.h, in that we will have to call to the implementer for handling the commands. The main difference is that libspdm understands the banks as well, compared to currently where libspdm doesn't comprehend banks and the implementer has to fake it by manually swapping out the certs

@jyao1

jyao1 commented Jun 3, 2026

Copy link
Copy Markdown
Member
  • Put bank management out of libspdm. All bank management command (READ and WRITE) needs to pass-thru to integrator.

The issue here though is it's very complex managing the libspdm_local_context_t state in libspdm. The implementor needs to regularly update the state inside libspdm as well as handling the READ and WRITE commands.

Bank management is similar to key pair info feature. Today we are using option 2) - see https://github.com/DMTF/libspdm/blob/main/include/hal/library/responder/key_pair_info.h

I think key pairs are simpler then banks. Note that the bank approach I'm thinking about here will be similar to https://github.com/DMTF/libspdm/blob/main/include/hal/library/responder/key_pair_info.h, in that we will have to call to the implementer for handling the commands. The main difference is that libspdm understands the banks as well, compared to currently where libspdm doesn't comprehend banks and the implementer has to fake it by manually swapping out the certs

Ok. That is fine.
But so far, I do not have a full picture on your solution. As such, I do not feel comfortable to approve and merge this partial solution, because it might not be needed if we choose a different direction. For example, if we go option 2, then we do not need to cache all bank info in local content.

Can you do a design review to compare those 2 directions?

@jyao1

jyao1 commented Jun 3, 2026

Copy link
Copy Markdown
Member

BTW: I am trying to add SLOT_MANAGEMENT feature using option 2.
I will submit patch soon.

@alistair23
alistair23 force-pushed the alistair/bank-support branch from 5a803e8 to df458c2 Compare June 4, 2026 03:14
@alistair23

Copy link
Copy Markdown
Contributor Author

But so far, I do not have a full picture on your solution. As such, I do not feel comfortable to approve and merge this partial solution, because it might not be needed if we choose a different direction. For example, if we go option 2, then we do not need to cache all bank info in local content.

But option 2 requires a lot more duplicated work each implementer, with a greater chance of bugs in all of them.

My plan is to keep working on this, but I didn't want to spend too much time on the bank management if it wasn't going to be accepted.

Can you do a design review to compare those 2 directions?

I can, but I'm not sure what that is

@jyao1

jyao1 commented Jun 4, 2026

Copy link
Copy Markdown
Member

@alistair23
I have submitted draft #3637, which is idea for option 2.
Please take a look and feedback.

@alistair23
alistair23 force-pushed the alistair/bank-support branch 2 times, most recently from cd62347 to d0fa40d Compare June 29, 2026 02:57
@alistair23
alistair23 force-pushed the alistair/bank-support branch 2 times, most recently from 3e56b2e to bd43131 Compare June 30, 2026 23:14

@ayj ayj left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In libspdm_check_context, certificate provision sizes are checked against max_spdm_msg_size. When hybrid ECDSA (Bank 0, ~2KB) and PQC (Bank 1, e.g. ML-DSA/Dilithium, ~15KB) are both populated, max_spdm_msg_size must either be sized for the greater of the two banks or SPDM 1.2 Chunking (CHUNK_CAP) must be strictly enforced whenever Bank 1 or higher contains large PQC chains.

Should libspdm_check_context iterate across all configured banks during startup (rather than checking only current_bank) to catch PQC buffer overflow misconfigurations early?

@alistair23
alistair23 force-pushed the alistair/bank-support branch from bd43131 to 8ab0d80 Compare July 13, 2026 03:31
Currently bank support can be handled by the implementer using the
CONNECTION_STATE_NEGOTIATED callback registered with
libspdm_register_connection_state_callback_func().

The problem with this is it pushes a lot of complexity back to the
implementer and it makes supporting the Slot Management commands tricky,
as the implementer will need to handle the commands as well.

Instead let's move the bank support into libspdm. For step 1 we just
convert the array of certificate information for the slots into a 2-D
array of slots and banks. We hard code to use bank 0 at all times, so
this is no functional change.

At build time we allow users to specify their own bank count, allowing
smaller bank counts on size reduced systems.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Allow the implementation to select the bank to use by setting the
LIBSPDM_DATA_LOCAL_CURRENT_BANK property. This selects the current bank
for all existing operations.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
@alistair23
alistair23 force-pushed the alistair/bank-support branch from 8ab0d80 to 5835b3a Compare July 13, 2026 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants