Harden GitHub workflows

Signed-off-by: Pol Henarejos <pol.henarejos@cttc.es>

.github/workflows/codeql.yml

@@ -40,7 +40,9 @@ jobs:
 
     steps:
     - name: Checkout repository
-      uses: actions/checkout@v3
+      uses: actions/checkout@v4
+      with:
+        persist-credentials: false
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL

.github/workflows/nightly.yml

@@ -5,34 +5,49 @@ on:
     - cron: '0 2 * * *'
   workflow_dispatch:
 
+permissions:
+  contents: write
+
 jobs:
   nightly:
     name: Deploy nightly
+    permissions:
+      contents: write
     strategy:
       fail-fast: false
       matrix:
         refs: [main, development]
     runs-on: ubuntu-latest
+    env:
+      SIGNING_KEY_PATH: ${{ github.workspace }}/../private.pem
     steps:
+      - name: Validate signing secret
+        run: |
+          test -n "${{ secrets.PRIVATE_KEY_B64 }}" || {
+            echo "PRIVATE_KEY_B64 is required for nightly signed releases." >&2
+            exit 1
+          }
       - name: Checkout repository
         uses: actions/checkout@v4
         with:
           ref: ${{ matrix.refs }}
           submodules: 'recursive'
+          persist-credentials: false
       - name: Restore private key
         run: |
-          echo "${{ secrets.PRIVATE_KEY_B64 }}" | base64 -d > private.pem
+          echo "${{ secrets.PRIVATE_KEY_B64 }}" | base64 -d > "${SIGNING_KEY_PATH}"
-          chmod 600 private.pem
+          chmod 600 "${SIGNING_KEY_PATH}"
       - name : Build
         env:
           PICO_SDK_PATH: ../pico-sdk
-          SECURE_BOOT_PKEY: ../private.pem
+          SECURE_BOOT_PKEY: ${{ github.workspace }}/../private.pem
         run: |
            ./workflows/autobuild.sh pico
            ./build_pico_fido.sh --no-eddsa
            ./workflows/autobuild.sh esp32
       - name: Delete private key
-        run: rm private.pem
+        if: always()
+        run: rm -f "${SIGNING_KEY_PATH}"
       - name: Update nightly release
         uses: pyTooling/Actions/releaser@v6.7.0
         with:

.github/workflows/test.yml

@@ -21,16 +21,22 @@ on:
     - cron: '23 5 * * 4'
   workflow_dispatch:
 
+permissions:
+  contents: read
+
 jobs:
   build:
 
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
 
     steps:
     - name: Checkout repository and submodules
-      uses: actions/checkout@v3
+      uses: actions/checkout@v4
       with:
         submodules: recursive
+        persist-credentials: false
     - name: Build in container
       run: ./tests/build-in-docker.sh
     - name: Start emulation and test

src/fido/cbor.c

@@ -34,9 +34,9 @@ int cbor_get_assertion(const uint8_t *data, size_t len, bool next);
 
 const uint8_t aaguid[16] = { 0x89, 0xFB, 0x94, 0xB7, 0x06, 0xC9, 0x36, 0x73, 0x9B, 0x7E, 0x30, 0x52, 0x6D, 0x96, 0x81, 0x45 }; // First 16 bytes of SHA256("Pico FIDO2")
 
-const uint8_t *cbor_data = NULL;
+static const uint8_t *volatile cbor_data = NULL;
-size_t cbor_len = 0;
+static volatile size_t cbor_len = 0;
-uint8_t cbor_cmd = 0;
+static volatile uint8_t cbor_cmd = 0;
 
 int cbor_parse(uint8_t cmd, const uint8_t *data, size_t len) {
     if (len == 0 && cmd == CTAPHID_CBOR) {
@@ -108,7 +108,10 @@ void *cbor_thread(void *arg) {
         if (m == EV_EXIT) {
             break;
         }
-        apdu.sw = (uint16_t)cbor_parse(cbor_cmd, cbor_data, cbor_len);
+        const uint8_t *data = (const uint8_t *)cbor_data;
+        size_t len = cbor_len;
+        uint8_t cmd = cbor_cmd;
+        apdu.sw = (uint16_t)cbor_parse(cmd, data, len);
         if (apdu.sw == 0) {
             DEBUG_DATA(res_APDU, res_APDU_size);
         }

src/fido/cbor_get_assertion.c

@@ -32,6 +32,7 @@
 #include "random.h"
 
 int cbor_get_assertion(const uint8_t *data, size_t len, bool next);
+extern char *rp_id, *user_name, *display_name;
 
 bool residentx = false;
 Credential credsx[MAX_CREDENTIAL_COUNT_IN_LIST] = { 0 };
@@ -205,6 +206,9 @@ int cbor_get_assertion(const uint8_t *data, size_t len, bool next) {
     if (rpId.present == false || clientDataHash.present == false) {
         CBOR_ERROR(CTAP2_ERR_MISSING_PARAMETER);
     }
+    rp_id = rpId.data;
+    user_name = NULL;
+    display_name = NULL;
 
     uint8_t flags = 0;
     uint8_t rp_id_hash[32] = {0};

src/fido/cbor_make_credential.c

@@ -28,6 +28,8 @@
 #include "random.h"
 #include "crypto_utils.h"
 
+char *rp_id = NULL, *user_name = NULL, *display_name = NULL;
+
 int cbor_make_credential(const uint8_t *data, size_t len) {
     CborParser parser;
     CborValue map;
@@ -192,6 +194,9 @@ int cbor_make_credential(const uint8_t *data, size_t len) {
         }
     }
     CBOR_PARSE_MAP_END(map, 1);
+    rp_id = rp.id.data;
+    user_name = user.parent.name.data;
+    display_name = user.displayName.data;
 
     uint8_t flags = FIDO2_AUT_FLAG_AT;
     uint8_t rp_id_hash[32] = {0};

src/fido/cbor_reset.c

@@ -35,7 +35,7 @@ int cbor_reset(void) {
         return CTAP2_ERR_NOT_ALLOWED;
     }
 #endif
-    if (wait_button_pressed() == true) {
+    if (wait_button_pressed() > 0) {
         return CTAP2_ERR_USER_ACTION_TIMEOUT;
     }
 #endif

src/fido/cbor_selection.c

@@ -20,9 +20,16 @@
 #include "ctap2_cbor.h"
 #include "ctap.h"
 
+extern char *rp_id, *user_name, *display_name;
+
 int cbor_selection(void) {
-    if (wait_button_pressed() == true) {
+    rp_id = user_name = display_name = NULL;
+    int ret = wait_button_pressed() ;
+    if (ret == 1) {
         return CTAP2_ERR_USER_ACTION_TIMEOUT;
     }
+    else if (ret == 2) {
+        return CTAP2_ERR_OPERATION_DENIED;
+    }
     return CTAP2_OK;
 }

src/fido/cmd_authenticate.c

@@ -34,7 +34,7 @@ int cmd_authenticate(void) {
     if (req->keyHandleLen < KEY_HANDLE_LEN) {
         return SW_INCORRECT_PARAMS();
     }
-    if (P1(apdu) == CTAP_AUTH_ENFORCE && wait_button_pressed() == true) {
+    if (P1(apdu) == CTAP_AUTH_ENFORCE && wait_button_pressed() > 0) {
         return SW_CONDITIONS_NOT_SATISFIED();
     }
 

src/fido/cmd_register.c

@@ -63,7 +63,7 @@ int cmd_register(void) {
     if (apdu.nc != CTAP_APPID_SIZE + CTAP_CHAL_SIZE) {
         return SW_WRONG_LENGTH();
     }
-    if (wait_button_pressed() == true) {
+    if (wait_button_pressed() > 0) {
         return SW_CONDITIONS_NOT_SATISFIED();
     }
     if (memcmp(req->appId, bogus_firefox,

src/fido/fido.c

@@ -498,22 +498,28 @@ void init_fido(void) {
     needs_power_cycle = false;
 }
 
-bool wait_button_pressed(void) {
+int wait_button_pressed(void) {
     uint32_t val = EV_PRESS_BUTTON;
 #if defined(PICO_PLATFORM) || defined(ESP_PLATFORM)
     queue_try_add(&card_to_usb_q, &val);
     do {
         queue_remove_blocking(&usb_to_card_q, &val);
-    } while (val != EV_BUTTON_PRESSED && val != EV_BUTTON_TIMEOUT);
+    } while (val != EV_BUTTON_PRESSED && val != EV_BUTTON_TIMEOUT && val != EV_BUTTON_CANCELLED);
 #endif
-    return val == EV_BUTTON_TIMEOUT;
+    if (val == EV_BUTTON_TIMEOUT) {
+        return 1;
+    }
+    else if (val == EV_BUTTON_CANCELLED) {
+        return 2;
+    }
+    return 0;
 }
 
 uint32_t user_present_time_limit = 0;
 
 bool check_user_presence(void) {
     if (user_present_time_limit == 0 || user_present_time_limit + TRANSPORT_TIME_LIMIT < board_millis()) {
-        if (wait_button_pressed() == true) { //timeout
+        if (wait_button_pressed() > 0) { //timeout
             return false;
         }
         //user_present_time_limit = board_millis();

src/fido/fido.h

@@ -41,7 +41,7 @@ extern int derive_key(const uint8_t *app_id,
                       int,
                       mbedtls_ecp_keypair *key);
 extern int verify_key(const uint8_t *appId, const uint8_t *keyHandle, mbedtls_ecp_keypair *);
-extern bool wait_button_pressed(void);
+extern int wait_button_pressed(void);
 extern void init_fido(void);
 extern void init_otp(void);
 extern void scan_all(void);