Feature: Security Domain and SCP11a/b/c features #164

DennisDyallo · 2024-11-27T18:06:40Z

Description

This PR implements comprehensive support for SCP11a/b/c secure channels and enhances Security Domain functionality. It also enables OATH, OTP, and YubiHsm applications to operate over SCP connections.

Key Changes

Introduced new ../Scp/ namespace for improved secure channel management
Added support for SCP11a/b/c secure channels
Enabled OATH, OTP, and YubiHsm applications over SCP connections
Implemented new Security Domain application through SecurityDomainSession

New Components

SecurityDomainSession: Application that manages the Security Domain
Scp03KeyParameters: SCP03 key configuration
Scp11KeyParameters: SCP11 key configuration
KeyReference: Refers to keys on the Yubikey Security Domain
Scp03State: SCP03 state management
Scp11State: SCP11 state management

Deprecation and misc changes

Deprecated all classes in ../Scp03/ namespace
Replaced with new implementations in ../Scp11/ namespace
Modified some class names and functionality to align with Android SDK (yubikit-android) and Python SDK (yubikey-manager)

Migration Notes

The new architecture favors using SecurityDomainSession for Security Domain operations rather than individual command classes. Update existing implementations accordingly.

Type of change

Refactor (non-breaking change which improves code quality or performance)
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How has this been tested?

Tested using a 5.7.2 key.
Needs to test more thoroughly on other keys.

Test configuration:

OS version: Win 11
Firmware version: 5.4.3, 5.7.2 Non FIPS
Yubikey model¹: 5 MPE

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have run dotnet format to format my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

See Yubikey models (Multi-protocol, Security Key, FIPS, Bio, YubiHSM, YubiHSM FIPS) ↩

feat(scp11): Generate EC Key feat(sd): Reset Security Domain misc: Changed static analysis mode misc: SampleCode projects won't build nuget packages

Store identifiers in dictionary Utilize current and next TLV values Improve method functionality Enhance code readability

update auth-decrypt.md update auth-sign.md updated RSA key sizes in user manual files updated RSA key info in API docs updated docs and minor adjustments and new tests

support OATH and SCP support OTP and SCP support YubiHSM and SCP

…fier

added YubiHsm SCP 11b tests

throws exception on invalid KeyParameters add docs

ChannelEncyrption: use Span<byte> instead of byte[] ChannelMac: use Span<byte> instead of byte[] StoreDataCommand: change order of fields

DennisDyallo

Will address these things

DennisDyallo · 2024-11-27T19:28:18Z

Yubico.YubiKey/src/Yubico/YubiKey/Scp/Commands/PutKeyCommand.cs

+
+namespace Yubico.YubiKey.Scp.Commands
+{
+    /// <summary>


This should probably be rewritten to account for SCP11

DennisDyallo · 2024-11-27T21:07:32Z

Yubico.YubiKey/src/Yubico/YubiKey/Scp/SecurityDomainSession.cs

+
+namespace Yubico.YubiKey.Scp
+{
+    /// <summary>


Needs a rewrite to account for SCP11

Yubico.YubiKey/src/Yubico/YubiKey/Scp/SecurityDomainSession.cs

Yubico.YubiKey/src/Yubico/YubiKey/SmartCardConnection.cs

DennisDyallo · 2024-11-28T09:42:11Z

Yubico.YubiKey/src/Yubico/YubiKey/Cryptography/AesUtilities.cs

 #pragma warning disable CA5358 // Allow the usage of cipher mode 'ECB'
-                aesObj.Mode = CipherMode.ECB;
+            aesObj.Mode = CipherMode.ECB;


To fix the problem, we should replace the use of ECB mode with a more secure mode of operation, such as CBC (Cipher Block Chaining) or GCM (Galois/Counter Mode). These modes provide better security by introducing randomness and ensuring that identical plaintext blocks produce different ciphertext blocks.

The best way to fix the problem without changing existing functionality is to use CBC mode with a randomly generated initialization vector (IV). This will require modifying the code to generate a random IV and use it for encryption. The IV should be stored or transmitted along with the ciphertext to allow for decryption.

Changes needed:

Replace CipherMode.ECB with CipherMode.CBC.

Generate a random IV and set it in the AES object.

Ensure the IV is stored or transmitted with the ciphertext.

CBC is required and allowed for our use case.

github-actions · 2024-11-28T00:51:20Z

Test Results: Windows

2 files 2 suites 5s ⏱️
3 690 tests 3 690 ✅ 0 💤 0 ❌
3 692 runs 3 692 ✅ 0 💤 0 ❌

Results for commit 15e7efa.

♻️ This comment has been updated with latest results.

github-actions · 2024-11-28T00:51:28Z

Test Results: Ubuntu

2 files 2 suites 9s ⏱️
3 682 tests 3 682 ✅ 0 💤 0 ❌
3 684 runs 3 684 ✅ 0 💤 0 ❌

Results for commit 15e7efa.

♻️ This comment has been updated with latest results.

github-actions · 2024-11-28T00:51:35Z

Test Results: MacOS

2 files 2 suites 4s ⏱️
3 682 tests 3 682 ✅ 0 💤 0 ❌
3 684 runs 3 684 ✅ 0 💤 0 ❌

Results for commit 15e7efa.

♻️ This comment has been updated with latest results.

Yubico.YubiKey/docs/users-manual/application-piv/apdu/auth-sign.md

added validation of hostchallenge length added documentation for the class refactor around external authenticate for clarity

in: ScpState and SessionKeys.cs

docs: revert auto format of .md files misc: fixed typo in SecurityDomainSession

YubiKeyDeviceExtensions.cs was not meant to be deleted yet

Scp03 enum is obsolete removed comments added SecurityDomain enum to ConnectionManager.cs

Scp03Tests: All tests select transport. Will fail test if Nfc and Fips are selected

github-actions · 2024-11-29T14:21:39Z

Package	Line Rate	Branch Rate	Complexity	Health
Yubico.Core	44%	32%	4311	➖
Yubico.YubiKey	47%	44%	19716	➖
Summary	46% (31697 / 68460)	42% (8050 / 19020)	24027	➖

Minimum allowed line rate is 40%

DennisDyallo added the enhancement New feature or request label Nov 27, 2024

DennisDyallo self-assigned this Nov 27, 2024

DennisDyallo force-pushed the feature/scp11 branch 2 times, most recently from cfa5db9 to 69d6525 Compare November 27, 2024 19:07

DennisDyallo added 20 commits November 27, 2024 23:44

initial Security Domain implementation

6e537c7

feat(scp11): Generate EC Key feat(sd): Reset Security Domain misc: Changed static analysis mode misc: SampleCode projects won't build nuget packages

feat(scp11b): Import Key

2bd752f

feat(scp): Import Static Keys

366aa8b

feat(scp11ab): Initialize SCP Connection

5e9ec88

feat(scp): remaining SCP Commands implemented

d2d1cf5

Store identifiers in dictionary Utilize current and next TLV values Improve method functionality Enhance code readability

docs: updated docstrings and manual

4017ae4

update auth-decrypt.md update auth-sign.md updated RSA key sizes in user manual files updated RSA key info in API docs updated docs and minor adjustments and new tests

tests, misc: added and fixed tests, misc updates

093f919

feat(scp): supports apps over SCP

81799ca

support OATH and SCP support OTP and SCP support YubiHSM and SCP

docs: add comments to TlvObject

01b6ab2

docs: fix incorrect values in docs for RSA key sizes and their identi…

c41fff1

…fier

docs: add docs to ApplicationSession.cs

00cdeae

logs: change log level in ConnectionFactory

f90fbee

misc: return ReadOnlyMemory<byte> in ECPublicKeyParameters.cs

f5fbbbf

misc: OtpSession cleanup

a786bef

misc: ScpApduTransform.cs edits

7bd0b36

misc: PivSession.cs edits

23af160

misc: ExternalAuthenticateResponse.cs edits

cd13afd

misc: InitializeUpdateCommand.cs edits

d9cc85f

docs: added docs to PutKeyCommand

660095d

docs: adding comments and docs

6f8b74e

DennisDyallo force-pushed the feature/scp11 branch from 69d6525 to 7a5053b Compare November 27, 2024 22:45

DennisDyallo added 2 commits November 27, 2024 23:49

misc: misc edits

de43147

tests: test edits

ff36d90

added YubiHsm SCP 11b tests

DennisDyallo force-pushed the feature/scp11 branch from 7a5053b to ff36d90 Compare November 27, 2024 22:49

docs, misc: edits in ScpApduTransform

d77bc6a

throws exception on invalid KeyParameters add docs

DennisDyallo force-pushed the feature/scp11 branch from f2d9555 to b6b97ad Compare November 27, 2024 23:38

DennisDyallo force-pushed the feature/scp11 branch 2 times, most recently from 75a40ae to e122693 Compare November 27, 2024 23:55

misc: edits to StoreDataCommand and ChannelEncryption

11b87d9

ChannelEncyrption: use Span<byte> instead of byte[] ChannelMac: use Span<byte> instead of byte[] StoreDataCommand: change order of fields

DennisDyallo force-pushed the feature/scp11 branch from e122693 to 11b87d9 Compare November 27, 2024 23:56

DennisDyallo commented Nov 28, 2024

View reviewed changes

DennisDyallo force-pushed the feature/scp11 branch from 224b1e9 to 38141bc Compare November 28, 2024 00:25

Merge remote-tracking branch 'origin' into feature/scp11

3b2753d

DennisDyallo force-pushed the feature/scp11 branch from 38141bc to 3b2753d Compare November 28, 2024 00:38

github-advanced-security bot found potential problems Nov 28, 2024

View reviewed changes

DennisDyallo added 2 commits November 28, 2024 01:48

misc: remove bad tests

7e125ca

misc: no need to check for null

1f13c2c

DennisDyallo marked this pull request as ready for review November 28, 2024 09:42

DennisDyallo commented Nov 28, 2024

View reviewed changes

Yubico.YubiKey/docs/users-manual/application-piv/apdu/auth-sign.md Outdated Show resolved Hide resolved

DennisDyallo added 5 commits November 28, 2024 11:13

docs: removed unintended comments in pivsession

e39cd5a

docs: added docstring for Scp11State

207a45d

misc: scp03state edits

263a232

added validation of hostchallenge length added documentation for the class refactor around external authenticate for clarity

tests: rename test methods for Scp03Tests.cs

35a3bba

misc: use ReadOnlyMemory

2b6cd76

in: ScpState and SessionKeys.cs

DennisDyallo requested review from AdamVe, dainnilsson and jensutbult November 28, 2024 11:14

DennisDyallo added 5 commits November 28, 2024 12:35

misc: typos and formatting

d643d33

docs: revert auto format of .md files misc: fixed typo in SecurityDomainSession

Merge branch 'develop' into feature/scp11

8360fb1

misc: re-add deprecated file

5be4499

YubiKeyDeviceExtensions.cs was not meant to be deleted yet

misc: minor edits

d491ea1

Scp03 enum is obsolete removed comments added SecurityDomain enum to ConnectionManager.cs

tests: possibility to select by transport

15e7efa

Scp03Tests: All tests select transport. Will fail test if Nfc and Fips are selected

@@ -54,3 +54,3 @@
             #pragma warning disable CA5358 // Allow the usage of cipher mode 'ECB'
-                        aesObj.Mode = CipherMode.ECB;
+                        aesObj.Mode = CipherMode.CBC;
             #pragma warning restore CA5358
@@ -59,4 +59,5 @@
                         aesObj.Key = aesObjKey;
-                        aesObj.IV = new byte[BlockSizeBytes];
                         aesObj.Padding = PaddingMode.None;
+                        aesObj.GenerateIV();
+                        byte[] iv = aesObj.IV;
             #pragma warning disable CA5401 // Justification: Allow the symmetric encryption to use

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Security Domain and SCP11a/b/c features #164

Feature: Security Domain and SCP11a/b/c features #164

DennisDyallo commented Nov 27, 2024 •

edited

Loading

DennisDyallo left a comment

DennisDyallo Nov 27, 2024

DennisDyallo Nov 27, 2024 •

edited

Loading

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

DennisDyallo Nov 28, 2024

github-actions bot commented Nov 28, 2024 •

edited

Loading

github-actions bot commented Nov 28, 2024 •

edited

Loading

github-actions bot commented Nov 28, 2024 •

edited

Loading

github-actions bot commented Nov 29, 2024

Feature: Security Domain and SCP11a/b/c features #164

Are you sure you want to change the base?

Feature: Security Domain and SCP11a/b/c features #164

Conversation

DennisDyallo commented Nov 27, 2024 • edited Loading

Description

Key Changes

New Components

Deprecation and misc changes

Migration Notes

Type of change

How has this been tested?

Checklist:

Footnotes

DennisDyallo left a comment

Choose a reason for hiding this comment

DennisDyallo Nov 27, 2024

Choose a reason for hiding this comment

DennisDyallo Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

DennisDyallo Nov 28, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 28, 2024 • edited Loading

Test Results: Windows

github-actions bot commented Nov 28, 2024 • edited Loading

Test Results: Ubuntu

github-actions bot commented Nov 28, 2024 • edited Loading

Test Results: MacOS

github-actions bot commented Nov 29, 2024

DennisDyallo commented Nov 27, 2024 •

edited

Loading

DennisDyallo Nov 27, 2024 •

edited

Loading

github-actions bot commented Nov 28, 2024 •

edited

Loading

github-actions bot commented Nov 28, 2024 •

edited

Loading

github-actions bot commented Nov 28, 2024 •

edited

Loading