refactor: PivSession

- added field DefaultManagementKey
- early exit when fail TryVerifyPin
- use range operator

Author: DennisDyallo

Yubico.YubiKey/src/Yubico/YubiKey/Piv/Objects/PinProtectedData.cs

@@ -142,7 +142,7 @@ public void SetManagementKey(ReadOnlyMemory<byte> managementKey)
             if (IsValidKeyLength(managementKey.Length))
             {
                 managementKey.CopyTo(_mgmtKey);
-                ManagementKey = _mgmtKey.Slice(0, managementKey.Length);
+                ManagementKey = _mgmtKey[..managementKey.Length];
                 return;
             }
 

Yubico.YubiKey/src/Yubico/YubiKey/Piv/PivSession.ManagementKey.cs

@@ -24,6 +24,13 @@ namespace Yubico.YubiKey.Piv
     // operations.
     public sealed partial class PivSession : IDisposable
     {
+        private static readonly Memory<byte> DefaultManagementKey = new byte[]
+        {
+            0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
+            0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
+            0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08
+        };
+
         /// <summary>
         /// This specifies the algorithm of the management key.
         /// </summary>
@@ -71,12 +78,6 @@ public sealed partial class PivSession : IDisposable
         /// </remarks>
         public AuthenticateManagementKeyResult ManagementKeyAuthenticationResult { get; private set; }
 
-        private PivAlgorithm DefaultManagementKeyAlgorithm =>
-            YubiKey.HasFeature(YubiKeyFeature.PivAesManagementKey) &&
-            YubiKey.FirmwareVersion >= FirmwareVersion.V5_7_0
-                ? PivAlgorithm.Aes192
-                : PivAlgorithm.TripleDes;
-
         /// <summary>
         /// Try to authenticate the management key.
         /// </summary>
@@ -278,7 +279,7 @@ private bool TryAuthenticateWithKeyCollector(bool mutualAuthentication)
             {
                 keyEntryData.Clear();
 
-                if (!(KeyCollector is null))
+                if (KeyCollector is not null)
                 {
                     keyEntryData.Request = KeyEntryRequest.Release;
                     _ = KeyCollector(keyEntryData);
@@ -656,7 +657,8 @@ public bool TryChangeManagementKey(PivTouchPolicy touchPolicy = PivTouchPolicy.D
         /// </exception>
         public bool TryChangeManagementKey(PivTouchPolicy touchPolicy, PivAlgorithm newKeyAlgorithm)
         {
-            Logger.LogInformation("Try to change the management key, touch policy = {TouchPolicy}, algorithm = {PivALgorithm}.",
+            Logger.LogInformation(
+                "Try to change the management key, touch policy = {TouchPolicy}, algorithm = {PivALgorithm}.",
                 touchPolicy.ToString(), newKeyAlgorithm.ToString());
 
             CheckManagementKeyAlgorithm(newKeyAlgorithm, true);
@@ -668,7 +670,7 @@ public bool TryChangeManagementKey(PivTouchPolicy touchPolicy, PivAlgorithm newK
 
             try
             {
-                if (TryAuthenticateWithKeyCollector(true, keyEntryData) == false)
+                if (!TryAuthenticateWithKeyCollector(true, keyEntryData))
                 {
                     return false;
                 }
@@ -691,10 +693,9 @@ public bool TryChangeManagementKey(PivTouchPolicy touchPolicy, PivAlgorithm newK
             finally
             {
                 keyEntryData.Clear();
-
                 keyEntryData.Request = KeyEntryRequest.Release;
 
-                if (!(KeyCollector is null))
+                if (KeyCollector is not null)
                 {
                     _ = KeyCollector(keyEntryData);
                 }
@@ -766,10 +767,11 @@ public void ChangeManagementKey(PivTouchPolicy touchPolicy = PivTouchPolicy.Defa
         /// </exception>
         public void ChangeManagementKey(PivTouchPolicy touchPolicy, PivAlgorithm newKeyAlgorithm)
         {
-            Logger.LogInformation("Change the management key, touch policy = {TouchPolicy}, algorithm = {PivAlgorithm}.",
+            Logger.LogInformation(
+                "Change the management key, touch policy = {TouchPolicy}, algorithm = {PivAlgorithm}.",
                 touchPolicy.ToString(), newKeyAlgorithm.ToString());
 
-            if (TryChangeManagementKey(touchPolicy, newKeyAlgorithm) == false)
+            if (!TryChangeManagementKey(touchPolicy, newKeyAlgorithm))
             {
                 throw new OperationCanceledException(
                     string.Format(
@@ -828,9 +830,10 @@ public void ChangeManagementKey(PivTouchPolicy touchPolicy, PivAlgorithm newKeyA
         /// Mutual authentication was performed and the YubiKey was not
         /// authenticated.
         /// </exception>
-        public bool TryChangeManagementKey(ReadOnlyMemory<byte> currentKey,
-                                           ReadOnlyMemory<byte> newKey,
-                                           PivTouchPolicy touchPolicy = PivTouchPolicy.Default) =>
+        public bool TryChangeManagementKey(
+            ReadOnlyMemory<byte> currentKey,
+            ReadOnlyMemory<byte> newKey,
+            PivTouchPolicy touchPolicy = PivTouchPolicy.Default) =>
             TryChangeManagementKey(currentKey, newKey, touchPolicy, DefaultManagementKeyAlgorithm);
 
         /// <summary>
@@ -890,10 +893,11 @@ public bool TryChangeManagementKey(ReadOnlyMemory<byte> currentKey,
         /// Mutual authentication was performed and the YubiKey was not
         /// authenticated.
         /// </exception>
-        public bool TryChangeManagementKey(ReadOnlyMemory<byte> currentKey,
-                                           ReadOnlyMemory<byte> newKey,
-                                           PivTouchPolicy touchPolicy,
-                                           PivAlgorithm newKeyAlgorithm)
+        public bool TryChangeManagementKey(
+            ReadOnlyMemory<byte> currentKey,
+            ReadOnlyMemory<byte> newKey,
+            PivTouchPolicy touchPolicy,
+            PivAlgorithm newKeyAlgorithm)
         {
             CheckManagementKeyAlgorithm(newKeyAlgorithm, true);
 
@@ -902,10 +906,11 @@ public bool TryChangeManagementKey(ReadOnlyMemory<byte> currentKey,
 
         // Try to change the management key, even if the YubiKey is set to
         // PIN-derived.
-        private bool TryForcedChangeManagementKey(ReadOnlyMemory<byte> currentKey,
-                                                  ReadOnlyMemory<byte> newKey,
-                                                  PivTouchPolicy touchPolicy,
-                                                  PivAlgorithm newKeyAlgorithm)
+        private bool TryForcedChangeManagementKey(
+            ReadOnlyMemory<byte> currentKey,
+            ReadOnlyMemory<byte> newKey,
+            PivTouchPolicy touchPolicy,
+            PivAlgorithm newKeyAlgorithm)
         {
             if (TryAuthenticateManagementKey(currentKey, true))
             {
@@ -974,9 +979,10 @@ private bool TryAuthenticateWithKeyCollector(bool mutualAuthentication, KeyEntry
         // if the auth succeeds.
         // If auth works, return true, otherwise, return false.
         // Throw an exception if the YubiKey fails to auth.
-        private bool TryAuthenticateManagementKey(bool mutualAuthentication,
-                                                  ReadOnlySpan<byte> mgmtKey,
-                                                  PivAlgorithm algorithm)
+        private bool TryAuthenticateManagementKey(
+            bool mutualAuthentication,
+            ReadOnlySpan<byte> mgmtKey,
+            PivAlgorithm algorithm)
         {
             var initCommand = new InitializeAuthenticateManagementKeyCommand(mutualAuthentication, algorithm);
             var initResponse = Connection.SendCommand(initCommand);
@@ -1058,26 +1064,27 @@ private void CheckManagementKeyAlgorithm(PivAlgorithm algorithm, bool checkMode)
                 }
             }
 
-            bool isValid = IsValid(algorithm);
-            if (!isValid)
+            if (!IsValidManagementKeyAlgorithm(algorithm))
             {
                 throw new ArgumentException(
                     string.Format(
                         CultureInfo.CurrentCulture,
                         ExceptionMessages.UnsupportedAlgorithm));
             }
-
-            return;
-
-            bool IsValid(PivAlgorithm pa) =>
-                pa switch
-                {
-                    PivAlgorithm.TripleDes => true, // Default for keys below fw version 5.7
-                    PivAlgorithm.Aes128 => YubiKey.HasFeature(YubiKeyFeature.PivAesManagementKey),
-                    PivAlgorithm.Aes192 => YubiKey.HasFeature(YubiKeyFeature.PivAesManagementKey),
-                    PivAlgorithm.Aes256 => YubiKey.HasFeature(YubiKeyFeature.PivAesManagementKey),
-                    _ => false
-                };
         }
+        
+        private bool IsValidManagementKeyAlgorithm(PivAlgorithm pivAlgorithm) =>
+            pivAlgorithm switch
+            {
+                PivAlgorithm.TripleDes => true, // Default for keys below fw version 5.7
+                PivAlgorithm.Aes128 or PivAlgorithm.Aes192 or PivAlgorithm.Aes256 => YubiKey.HasFeature(YubiKeyFeature.PivAesManagementKey),
+                _ => false
+            };
+        
+        private PivAlgorithm DefaultManagementKeyAlgorithm =>
+            YubiKey.HasFeature(YubiKeyFeature.PivAesManagementKey) &&
+            YubiKey.FirmwareVersion >= FirmwareVersion.V5_7_0
+                ? PivAlgorithm.Aes192
+                : PivAlgorithm.TripleDes;
     }
 }

Yubico.YubiKey/src/Yubico/YubiKey/Piv/PivSession.Pin.cs

@@ -288,20 +288,21 @@ public bool TryVerifyPin(ReadOnlyMemory<byte> pin, out int? retriesRemaining)
             var response = Connection.SendCommand(command);
 
             PinVerified = response.Status == ResponseStatus.Success;
-            if (!PinVerified)
+            if (PinVerified)
             {
-                retriesRemaining = response.GetData();
+                return true;
+            }
 
-                if ((retriesRemaining ?? 1) == 0)
-                {
-                    throw new SecurityException(
-                        string.Format(
-                            CultureInfo.CurrentCulture,
-                            ExceptionMessages.NoMoreRetriesRemaining));
-                }
+            retriesRemaining = response.GetData();
+            if ((retriesRemaining ?? 1) == 0)
+            {
+                throw new SecurityException(
+                    string.Format(
+                        CultureInfo.CurrentCulture,
+                        ExceptionMessages.NoMoreRetriesRemaining));
             }
 
-            return PinVerified;
+            return false;
         }
 
         /// <summary>