fix(listApps): fallback if listAppsApdu not supported in old firmware

LedgerHQ · Aug 28, 2024 · 1d1bfd1 · 1d1bfd1
1 parent 238efb7
commit 1d1bfd1
Show file tree

Hide file tree

Showing 4 changed files with 131 additions and 18 deletions.
diff --git a/.changeset/thick-fans-give.md b/.changeset/thick-fans-give.md
@@ -0,0 +1,5 @@
+---
+"@ledgerhq/live-common": patch
+---
+
+Fix listApps: fallback to HSM script runner if listApps APDU is not available
diff --git a/libs/ledger-live-common/src/apps/listApps.test.ts b/libs/ledger-live-common/src/apps/listApps.test.ts
@@ -1,8 +1,8 @@
 import { from } from "rxjs";
-import { UnexpectedBootloader } from "@ledgerhq/errors";
+import { StatusCodes, TransportStatusError, UnexpectedBootloader } from "@ledgerhq/errors";
 import { aTransportBuilder } from "@ledgerhq/hw-transport-mocker";
 import { listApps } from "./listApps";
-import ManagerAPI from "../manager/api";
+import ManagerAPI, { ListInstalledAppsEvent } from "../manager/api";
 import { aDeviceInfoBuilder } from "../mock/fixtures/aDeviceInfo";
 import {
   ManagerApiRepository,
@@ -25,10 +25,15 @@ const mockedGetDeviceName = jest.mocked(getDeviceName);
 const mockedListCryptoCurrencies = jest.mocked(listCryptoCurrencies);
 const mockedCurrenciesByMarketCap = jest.mocked(currenciesByMarketcap);
 
+const mockedListInstalledAppEvent: ListInstalledAppsEvent = {
+  type: "result",
+  payload: [],
+};
+
 describe("listApps", () => {
   let mockedManagerApiRepository: ManagerApiRepository;
   let listAppsCommandSpy: jest.SpyInstance;
-  let listInstalledAppsSpy: jest.SpyInstance;
+  let listAppsWithManagerApiSpy: jest.SpyInstance;
 
   beforeEach(() => {
     jest
@@ -46,7 +51,9 @@ describe("listApps", () => {
       .spyOn(jest.requireActual("../hw/listApps"), "default")
       .mockReturnValue(Promise.resolve([]));
 
-    listInstalledAppsSpy = jest.spyOn(ManagerAPI, "listInstalledApps").mockReturnValue(from([]));
+    listAppsWithManagerApiSpy = jest
+      .spyOn(ManagerAPI, "listInstalledApps")
+      .mockReturnValue(from([mockedListInstalledAppEvent]));
   });
 
   afterEach(() => {
@@ -150,7 +157,83 @@ describe("listApps", () => {
     jest.advanceTimersByTime(1);
 
     expect(listAppsCommandSpy).toHaveBeenCalled();
-    expect(listInstalledAppsSpy).not.toHaveBeenCalled();
+    expect(listAppsWithManagerApiSpy).not.toHaveBeenCalled();
+  });
+
+  [
+    StatusCodes.CLA_NOT_SUPPORTED,
+    StatusCodes.INS_NOT_SUPPORTED,
+    StatusCodes.UNKNOWN_APDU,
+    0x6e01,
+    0x6d01,
+  ].forEach(statusCode => {
+    it(`should call ManagerAPI.listInstalledApps() if deviceInfo.managerAllowed is true but list apps APDU returns 0x${statusCode.toString(16)}`, done => {
+      const transport = aTransportBuilder();
+      const deviceInfo = aDeviceInfoBuilder({
+        isOSU: false,
+        isBootloader: false,
+        managerAllowed: true,
+        targetId: 0x33200000,
+      });
+
+      listAppsCommandSpy.mockRejectedValue(new TransportStatusError(statusCode));
+
+      listApps({
+        managerDevModeEnabled: false,
+        transport,
+        deviceInfo,
+        managerApiRepository: mockedManagerApiRepository,
+        forceProvider: 1,
+      }).subscribe({
+        complete: () => {
+          try {
+            expect(listAppsCommandSpy).toHaveBeenCalled();
+            expect(listAppsWithManagerApiSpy).toHaveBeenCalled();
+            done();
+          } catch (e) {
+            done(e);
+          }
+        },
+        error: e => {
+          done(e);
+        },
+      });
+      jest.advanceTimersByTime(1);
+    });
+  });
+
+  it("should return an observable that errors if listApps() throws an error that is not a TransportStatusError", done => {
+    const transport = aTransportBuilder();
+    const deviceInfo = aDeviceInfoBuilder({
+      isOSU: false,
+      isBootloader: false,
+      managerAllowed: true,
+      targetId: 0x33200000,
+    });
+
+    listAppsCommandSpy.mockRejectedValue(new Error("listApps failed"));
+
+    listApps({
+      managerDevModeEnabled: false,
+      transport,
+      deviceInfo,
+      managerApiRepository: mockedManagerApiRepository,
+      forceProvider: 1,
+    }).subscribe({
+      error: err => {
+        try {
+          expect(err).toEqual(new Error("listApps failed"));
+          done();
+        } catch (e) {
+          done(e);
+        }
+      },
+      complete: () => {
+        done("this observable should not complete");
+      },
+    });
+
+    jest.advanceTimersByTime(1);
   });
 
   it("should call ManagerAPI.listInstalledApps() if deviceInfo.managerAllowed is false", () => {
@@ -172,7 +255,7 @@ describe("listApps", () => {
     jest.advanceTimersByTime(1);
 
     expect(listAppsCommandSpy).not.toHaveBeenCalled();
-    expect(listInstalledAppsSpy).toHaveBeenCalled();
+    expect(listAppsWithManagerApiSpy).toHaveBeenCalled();
   });
 
   it("should return an observable that errors if getDeviceVersion() throws", done => {

diff --git a/libs/ledger-live-common/src/apps/listApps.ts b/libs/ledger-live-common/src/apps/listApps.ts
@@ -1,6 +1,6 @@
 import Transport from "@ledgerhq/hw-transport";
 import { DeviceModelId, getDeviceModel, identifyTargetId } from "@ledgerhq/devices";
-import { UnexpectedBootloader } from "@ledgerhq/errors";
+import { StatusCodes, TransportStatusError, UnexpectedBootloader } from "@ledgerhq/errors";
 import { Observable, throwError, Subscription } from "rxjs";
 import { App, DeviceInfo, idsToLanguage, languageIds } from "@ledgerhq/types-live";
 import { LocalTracer } from "@ledgerhq/logs";
@@ -72,17 +72,13 @@ export const listApps = ({
        */
 
       let listAppsResponsePromise: Promise<ListAppResponse>;
-      if (deviceInfo.managerAllowed) {
-        // If the user has already allowed a secure channel during this session we can directly
-        // ask the device for the installed applications instead of going through a scriptrunner,
-        // this is a performance optimization, part of a larger rework with Manager API v2.
-        tracer.trace("Using direct apdu listapps");
-        listAppsResponsePromise = hwListApps(transport);
-      } else {
-        // Fallback to original web-socket list apps
-        tracer.trace("Using scriptrunner listapps");
 
-        listAppsResponsePromise = new Promise<ListAppResponse>((resolve, reject) => {
+      function listAppsWithSingleCommand(): Promise<ListAppResponse> {
+        return hwListApps(transport);
+      }
+
+      function listAppsWithManagerApi(): Promise<ListAppResponse> {
+        return new Promise<ListAppResponse>((resolve, reject) => {
           // TODO: migrate this ManagerAPI call to ManagerApiRepository
           sub = ManagerAPI.listInstalledApps(transport, {
             targetId: deviceInfo.targetId,
@@ -104,6 +100,35 @@ export const listApps = ({
         });
       }
 
+      if (deviceInfo.managerAllowed) {
+        // If the user has already allowed a secure channel during this session we can directly
+        // ask the device for the installed applications instead of going through a scriptrunner,
+        // this is a performance optimization, part of a larger rework with Manager API v2.
+        tracer.trace("Using direct apdu listapps");
+        listAppsResponsePromise = listAppsWithSingleCommand().catch(e => {
+          // For some old versions of the firmware, the listapps command is not supported.
+          // In this case, we fallback to the scriptrunner listapps.
+          if (
+            e instanceof TransportStatusError &&
+            [
+              StatusCodes.CLA_NOT_SUPPORTED,
+              StatusCodes.INS_NOT_SUPPORTED,
+              StatusCodes.UNKNOWN_APDU,
+              0x6e01, // No StatusCodes definition
+              0x6d01, // No StatusCodes definition
+            ].includes(e.statusCode)
+          ) {
+            tracer.trace("Fallback to scriptrunner listapps");
+            return listAppsWithManagerApi();
+          }
+          throw e;
+        });
+      } else {
+        // Fallback to original web-socket list apps
+        tracer.trace("Using scriptrunner listapps");
+        listAppsResponsePromise = listAppsWithManagerApi();
+      }
+
       const filteredListAppsPromise = listAppsResponsePromise.then(result => {
         // Empty HashData can come from apps that are not real apps (such as language packs)
         // or custom applications that have been sideloaded.

diff --git a/libs/ledger-live-common/src/hw/connectManager.ts b/libs/ledger-live-common/src/hw/connectManager.ts
@@ -93,9 +93,9 @@ const cmd = ({ deviceId, request }: Input): Observable<ConnectManagerEvent> =>
                   [
                     StatusCodes.CLA_NOT_SUPPORTED,
                     StatusCodes.INS_NOT_SUPPORTED,
+                    StatusCodes.UNKNOWN_APDU,
                     0x6e01, // No StatusCodes definition
                     0x6d01, // No StatusCodes definition
-                    0x6d02, // No StatusCodes definition
                   ].includes(e.statusCode))
               ) {
                 return from(getAppAndVersion(transport)).pipe(