From a329fa2f03ce48c9317ed5f5e057d96df8fdb819 Mon Sep 17 00:00:00 2001
From: "nicolas.dorier" <nicolas.dorier@gmail.com>
Date: Fri, 8 Dec 2023 16:09:54 +0900
Subject: [PATCH] Fix potential NRE in the PCSC transport, retry spurious
 failure of authentication in AuthenticateEV2First

---
 src/BTCPayServer.NTag424.PCSC/Extensions.cs   | 25 +++++++++++++++++++
 .../PCSCAPDUTransport.cs                      |  2 +-
 src/BTCPayServer.NTag424/Ntag424.cs           | 25 +++++++++++++------
 src/BTCPayServer.NTag424/Properties.cs        |  1 +
 tests/BTCPayServer.NTag424.Tests.csproj       |  1 -
 5 files changed, 45 insertions(+), 9 deletions(-)
 create mode 100644 src/BTCPayServer.NTag424.PCSC/Extensions.cs

diff --git a/src/BTCPayServer.NTag424.PCSC/Extensions.cs b/src/BTCPayServer.NTag424.PCSC/Extensions.cs
new file mode 100644
index 0000000..4122b8a
--- /dev/null
+++ b/src/BTCPayServer.NTag424.PCSC/Extensions.cs
@@ -0,0 +1,25 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using PCSC;
+using PCSC.Exceptions;
+using PCSC.Extensions;
+
+namespace BTCPayServer.NTag424.PCSC;
+internal static class Extensions
+{
+    public static void ThrowEx(this SCardError sc)
+    {
+        try
+        {
+            sc.Throw();
+        }
+        catch (NullReferenceException)
+        {
+            throw new PCSCException(sc, $"Unknown PCSC error: {(int)sc}");
+        }
+        throw new PCSCException(sc);
+    }
+}
diff --git a/src/BTCPayServer.NTag424.PCSC/PCSCAPDUTransport.cs b/src/BTCPayServer.NTag424.PCSC/PCSCAPDUTransport.cs
index 70321f6..31e81f8 100644
--- a/src/BTCPayServer.NTag424.PCSC/PCSCAPDUTransport.cs
+++ b/src/BTCPayServer.NTag424.PCSC/PCSCAPDUTransport.cs
@@ -24,7 +24,7 @@ public class PCSCAPDUTransport : IAPDUTransport
                 int received = resp.Length;
                 var sc = CardReader.Transmit(apdu, resp, ref received);
                 if (sc != SCardError.Success)
-                    sc.Throw();
+                    sc.ThrowEx();
                 var sw1sw2 = (ushort)(resp[received - 2] << 8 | resp[received - 1]);
                 var data = resp[..(received - 2)];
                 return new NtagResponse(data, sw1sw2);
diff --git a/src/BTCPayServer.NTag424/Ntag424.cs b/src/BTCPayServer.NTag424/Ntag424.cs
index 481bedf..65d1055 100644
--- a/src/BTCPayServer.NTag424/Ntag424.cs
+++ b/src/BTCPayServer.NTag424/Ntag424.cs
@@ -125,13 +125,14 @@ public class Ntag424
     {
         return AuthenticateEV2(keyNo, key, false, cancellationToken);
     }
-    public Task<Session> AuthenticateEV2First(int keyNo, AESKey key, CancellationToken cancellationToken = default)
+    public Task<Session> AuthenticateEV2First(int keyNo, AESKey? key, CancellationToken cancellationToken = default)
     {
         return AuthenticateEV2(keyNo, key, true, cancellationToken);
     }
-    async Task<Session> AuthenticateEV2(int keyNo, AESKey key, bool first, CancellationToken cancellationToken = default)
+    async Task<Session> AuthenticateEV2(int keyNo, AESKey? key, bool first, CancellationToken cancellationToken = default)
     {
-        int sessionCounter = CurrentSession?.Counter ?? 0;
+        key ??= AESKey.Default;
+        int sessionCounter;
         if (first)
         {
             await IsoSelectFile(ISOLevel.Application);
@@ -143,7 +144,8 @@ public class Ntag424
                 throw new InvalidOperationException("Authentication required for AuthenticateEV2NonFirst");
             sessionCounter = CurrentSession.Counter;
         }
-
+        bool illegalCommandThrown = false;
+retry:
         NtagResponse resp;
         if (first)
         {
@@ -164,10 +166,19 @@ public class Ntag424
         var rndA = RandomNumberGenerator.GetBytes(16);
         var encRnd = key.Encrypt(Concat(rndA, rndBp));
         var secondPart = first ? NtagCommands.AuthenticateEV2FirstPart2 : NtagCommands.AuthenticateEV2NonFirstPart2;
-        resp = await SendAPDU(secondPart with
+        try
         {
-            Data = encRnd
-        }, cancellationToken);
+            resp = await SendAPDU(secondPart with
+            {
+                Data = encRnd
+            }, cancellationToken);
+        }
+        // Sometimes, the card returns this error, unsure why, but retrying the auth seems to work
+        catch (UnexpectedStatusException ex) when (first && !illegalCommandThrown && ex.Details?.Code.Equals("ILLEGAL_COMMAND_CODE", StringComparison.Ordinal) is true)
+        {
+            illegalCommandThrown = true;
+            goto retry;
+        }
 
         var data = key.Decrypt(resp.Data);
         var rndAp = RotateLeft(rndA);
diff --git a/src/BTCPayServer.NTag424/Properties.cs b/src/BTCPayServer.NTag424/Properties.cs
index d8dc545..14af44f 100644
--- a/src/BTCPayServer.NTag424/Properties.cs
+++ b/src/BTCPayServer.NTag424/Properties.cs
@@ -1,3 +1,4 @@
 using System.Runtime.CompilerServices;
 
 [assembly: InternalsVisibleTo("BTCPayServer.NTag424.Tests")]
+[assembly: InternalsVisibleTo("BTCPayServer.NTag424.PCSC")]
diff --git a/tests/BTCPayServer.NTag424.Tests.csproj b/tests/BTCPayServer.NTag424.Tests.csproj
index 1100a07..e8ebb4d 100644
--- a/tests/BTCPayServer.NTag424.Tests.csproj
+++ b/tests/BTCPayServer.NTag424.Tests.csproj
@@ -4,7 +4,6 @@
     <TargetFramework>net6.0</TargetFramework>
     <ImplicitUsings>enable</ImplicitUsings>
     <Nullable>enable</Nullable>
-
     <IsPackable>false</IsPackable>
     <IsTestProject>true</IsTestProject>
   </PropertyGroup>