diff --git a/.github/codeql/codeql-config.yml b/.github/codeql/codeql-config.yml new file mode 100644 index 0000000000..fb4cafd616 --- /dev/null +++ b/.github/codeql/codeql-config.yml @@ -0,0 +1,6 @@ +# Configuration for .github/workflows/codeql.yml: run only liboqs's own +# queries. GitHub's stock suite already runs through CodeQL default setup. +name: liboqs custom queries +disable-default-queries: true +queries: + - uses: ./.github/codeql/queries diff --git a/.github/codeql/codeql-workspace.yml b/.github/codeql/codeql-workspace.yml new file mode 100644 index 0000000000..32544f86ac --- /dev/null +++ b/.github/codeql/codeql-workspace.yml @@ -0,0 +1,3 @@ +# Lets the test pack resolve the query pack locally. +provide: + - "*/qlpack.yml" diff --git a/.github/codeql/queries/openssl-return-check.ql b/.github/codeql/queries/openssl-return-check.ql new file mode 100644 index 0000000000..ec3b29ad56 --- /dev/null +++ b/.github/codeql/queries/openssl-return-check.ql @@ -0,0 +1,25 @@ +/** + * @name Unchecked OpenSSL EVP return value + * @description Calls to OpenSSL EVP functions whose status code is discarded + * silently ignore OpenSSL errors. Check the result, normally + * with the OQS_OPENSSL_GUARD macro. + * @kind problem + * @problem.severity warning + * @precision high + * @id liboqs/openssl-unchecked-return + * @tags security + * correctness + */ + +import cpp + +from FunctionCall call, Function f +where + f = call.getTarget() and + f.getName().matches("EVP%") and + not f.getUnspecifiedType() instanceof PointerType and + not f.getUnspecifiedType() instanceof VoidType and + // The status code is thrown away. Tested (e.g. by OQS_OPENSSL_GUARD), stored, + // or explicitly discarded with a (void) cast are all accepted. + call instanceof ExprInVoidContext +select call, "Return value of " + f.getName() + "() is discarded; check it with OQS_OPENSSL_GUARD." diff --git a/.github/codeql/queries/qlpack.yml b/.github/codeql/queries/qlpack.yml new file mode 100644 index 0000000000..31e2c96c9a --- /dev/null +++ b/.github/codeql/queries/qlpack.yml @@ -0,0 +1,4 @@ +name: liboqs/ci-queries +version: 0.0.0 +dependencies: + codeql/cpp-all: "*" diff --git a/.github/codeql/report_results.py b/.github/codeql/report_results.py new file mode 100755 index 0000000000..005bf847dd --- /dev/null +++ b/.github/codeql/report_results.py @@ -0,0 +1,35 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: MIT + +import json +import sys + +# Report the results of liboqs's custom CodeQL queries, failing if there are any. + +def escape(text): + # Characters with a special meaning in workflow command messages + return text.replace('%', '%25').replace('\r', '%0D').replace('\n', '%0A') + + +def main(sarif_files): + count = 0 + for sarif_file in sarif_files: + with open(sarif_file) as f: + sarif = json.load(f) + for run in sarif['runs']: + for result in run.get('results', []): + location = result['locations'][0]['physicalLocation'] + path = location['artifactLocation']['uri'] + line = location['region']['startLine'] + message = f"[{result['ruleId']}] {result['message']['text']}" + print(f"::error file={path},line={line}::{escape(message)}") + count += 1 + + print(f"{count} result(s) from liboqs CodeQL queries") + return 1 if count else 0 + + +if __name__ == '__main__': + if len(sys.argv) < 2: + sys.exit(__doc__) + sys.exit(main(sys.argv[1:])) diff --git a/.github/codeql/test/openssl-return-check/openssl-return-check.expected b/.github/codeql/test/openssl-return-check/openssl-return-check.expected new file mode 100644 index 0000000000..2e1a0b8af9 --- /dev/null +++ b/.github/codeql/test/openssl-return-check/openssl-return-check.expected @@ -0,0 +1,2 @@ +| test.c:22:2:22:18 | call to EVP_DigestInit_ex | Return value of EVP_DigestInit_ex() is discarded; check it with OQS_OPENSSL_GUARD. | +| test.c:23:2:23:17 | call to EVP_DigestUpdate | Return value of EVP_DigestUpdate() is discarded; check it with OQS_OPENSSL_GUARD. | diff --git a/.github/codeql/test/openssl-return-check/openssl-return-check.qlref b/.github/codeql/test/openssl-return-check/openssl-return-check.qlref new file mode 100644 index 0000000000..196399338d --- /dev/null +++ b/.github/codeql/test/openssl-return-check/openssl-return-check.qlref @@ -0,0 +1 @@ +openssl-return-check.ql diff --git a/.github/codeql/test/openssl-return-check/test.c b/.github/codeql/test/openssl-return-check/test.c new file mode 100644 index 0000000000..29de78b45d --- /dev/null +++ b/.github/codeql/test/openssl-return-check/test.c @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: MIT +// Test cases for queries/openssl-return-check.ql. +// Self-contained: OpenSSL is stubbed so no headers are needed. + +typedef struct evp_md_ctx_st EVP_MD_CTX; +typedef struct evp_md_st EVP_MD; + +int EVP_DigestInit_ex(EVP_MD_CTX *ctx, const EVP_MD *type, void *impl); +int EVP_DigestUpdate(EVP_MD_CTX *ctx, const void *d, unsigned long cnt); +EVP_MD_CTX *EVP_MD_CTX_new(void); +void EVP_MD_CTX_free(EVP_MD_CTX *ctx); +void exit(int status); + +#define OQS_OPENSSL_GUARD(x) \ + do { \ + if (1 != (x)) { \ + exit(1); \ + } \ + } while (0) + +void bad_discarded(EVP_MD_CTX *ctx, const EVP_MD *md) { + EVP_DigestInit_ex(ctx, md, 0); // BAD + EVP_DigestUpdate(ctx, "x", 1); // BAD +} + +void good_guarded(EVP_MD_CTX *ctx, const EVP_MD *md) { + OQS_OPENSSL_GUARD(EVP_DigestInit_ex(ctx, md, 0)); +} + +int good_tested(EVP_MD_CTX *ctx, const EVP_MD *md) { + if (EVP_DigestInit_ex(ctx, md, 0) != 1) { + return -1; + } + int ok = EVP_DigestUpdate(ctx, "x", 1); + return ok ? 0 : -1; +} + +void good_explicitly_discarded(EVP_MD_CTX *ctx) { + (void)EVP_DigestUpdate(ctx, "x", 1); +} + +void good_pointer_and_void_returns(void) { + EVP_MD_CTX *ctx = EVP_MD_CTX_new(); + EVP_MD_CTX_free(ctx); +} diff --git a/.github/codeql/test/qlpack.yml b/.github/codeql/test/qlpack.yml new file mode 100644 index 0000000000..86cb8d6c91 --- /dev/null +++ b/.github/codeql/test/qlpack.yml @@ -0,0 +1,5 @@ +name: liboqs/ci-queries-tests +version: 0.0.0 +extractor: cpp +dependencies: + liboqs/ci-queries: "*" diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 0000000000..12d355676c --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,60 @@ +name: CodeQL custom queries + +permissions: + contents: read + +# Runs liboqs's own CodeQL queries (.github/codeql/queries) and fails if any of +# them reports a result. Results are not uploaded to code scanning: this +# repository uses CodeQL default setup, which rejects CodeQL SARIF uploads from +# other workflows. +on: + workflow_call: + workflow_dispatch: + +jobs: + + query-tests: + name: CodeQL query unit tests + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # pin@v4 + - name: Set up CodeQL CLI + id: init + uses: github/codeql-action/init@2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2 # v4.38.2 + with: + languages: cpp + build-mode: none + - name: Run query unit tests + env: + CODEQL: ${{ steps.init.outputs.codeql-path }} + run: '"$CODEQL" test run .github/codeql/test' + + analyze: + name: CodeQL custom queries + runs-on: ubuntu-latest + env: + # Works around a crash while finalizing the database (see #2415). + CODEQL_EXTRACTOR_CPP_TRAP_CACHING: false + steps: + - name: Checkout code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # pin@v4 + - name: Initialize CodeQL + uses: github/codeql-action/init@2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2 # v4.38.2 + with: + languages: cpp + build-mode: manual + config-file: ./.github/codeql/codeql-config.yml + - name: Build liboqs + # CodeQL only sees compiled code: build the default algorithms, with + # OpenSSL backing every primitive that supports it. + run: | + cmake -S . -B build -DOQS_USE_AES_OPENSSL=ON -DOQS_USE_SHA2_OPENSSL=ON -DOQS_USE_SHA3_OPENSSL=ON + cmake --build build --parallel "$(nproc)" + - name: Run CodeQL queries + uses: github/codeql-action/analyze@2892aa5e19bbd11bc0cff5427e3b750a04d9e3c2 # v4.38.2 + with: + upload: false + output: sarif-results + - name: Report results + run: python3 .github/codeql/report_results.py sarif-results/*.sarif diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 9765399fec..04249f11d3 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -9,81 +9,8 @@ concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true +# TEST ONLY: trimmed to the CodeQL job for the fork test run. Drop this commit. jobs: - check-push-result: - name: Check if push CI passed for this commit - runs-on: ubuntu-latest - outputs: - skip_basic: ${{ steps.set_output.outputs.skip_basic }} - steps: - - name: Determine skip status - id: set_output - run: | - if [ "${{ github.event.pull_request.head.repo.fork }}" = "true" ]; then - echo "Fork PR - basic-checks cannot be skipped" - echo "skip_basic=false" >> $GITHUB_OUTPUT - else - SHA="${{ github.event.pull_request.head.sha }}" - RESPONSE=$(curl -s -H "Authorization: Bearer $GH_TOKEN" \ - "https://api.github.com/repos/${{ github.repository }}/actions/runs?head_sha=$SHA&per_page=5") - - # Pruefe ob "Push tests" Workflow mit conclusion=success existiert - if echo "$RESPONSE" | grep -q '"name": "Push tests"' && \ - echo "$RESPONSE" | grep -q '"conclusion": "success"'; then - echo "Found successful Push tests for SHA $SHA - skipping basic-checks" - echo "skip_basic=true" >> $GITHUB_OUTPUT - else - echo "No successful Push tests found for SHA $SHA" - echo "skip_basic=false" >> $GITHUB_OUTPUT - fi - fi - env: - GH_TOKEN: ${{ github.token }} - - basic-checks: - needs: check-push-result - if: needs.check-push-result.outputs.skip_basic == 'false' - concurrency: - group: basic-checks-${{ github.sha }} - cancel-in-progress: true - uses: ./.github/workflows/basic.yml - permissions: - contents: read - actions: write # required to cancel the run on fail-fast (see basic.yml) - - platform-tests: - needs: check-push-result - uses: ./.github/workflows/platforms.yml - - code-coverage: - needs: check-push-result - if: needs.check-push-result.outputs.skip_basic == 'false' - uses: ./.github/workflows/code-coverage.yml - secrets: inherit - - nix-full-build: - needs: check-push-result - if: needs.check-push-result.outputs.skip_basic == 'false' && (contains( github.event.pull_request.title, '[full tests]' ) || contains( github.event.pull_request.body, '[full tests]' )) - runs-on: ubuntu-latest - permissions: - contents: read - steps: - - uses: DeterminateSystems/nix-installer-action@3138316df39ed29be04236d7ffc686fa525866aa # v23 - - name: Checkout code - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # pin@v4 - - name: Check devShell - run: nix develop --command echo - - name: Check flake syntax - run: nix flake check --no-build - - name: Full flake build - run: nix build - - scorecard: - needs: check-push-result - uses: ./.github/workflows/supplychain.yml - secrets: inherit - permissions: - contents: read - id-token: write - security-events: write + codeql: + uses: ./.github/workflows/codeql.yml diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 2eb2db5bea..eb285fdcdd 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -5,7 +5,7 @@ permissions: on: push: - branches-ignore: 'main' + branches-ignore: ['main', 'feat/revived-codeql-openssl-return-check'] # TEST ONLY concurrency: group: ${{ github.workflow }}-${{ github.ref }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f6a09cf283..eac650617b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -119,3 +119,39 @@ If you feel like contributing but don't know what specific topic to work on, please check the [open issues tagged "good first issue" or "help wanted"](https://github.com/open-quantum-safe/liboqs/issues). You can also take a look at the [contribution wishlist](https://github.com/open-quantum-safe/liboqs/wiki/Contribution-wishlist) for more substantial contributions we are interested in. + +## Custom CodeQL queries + +There are two ways in which CodeQL runs in this repo: via GitHub's standard +code scanning settings, and via `.github/workflows/codeql.yml`: +- GitHub's standard scans are run via liboqs' repository code scanning settings. + Results appear as code scanning alerts in the repository's Security tab. +- `.github/workflows/codeql.yml` runs liboqs's own CodeQL queries on every pull + request and fails if any of them reports a result. Each result is shown as an + error annotation on the pull request and in the job log. Its results are not uploaded + to code scanning: while default setup is enabled, GitHub rejects CodeQL results uploaded + by other workflows ([details](https://docs.github.com/en/code-security/reference/code-scanning/sarif-files/troubleshoot-sarif-uploads/default-setup-enabled)). + The workflow can also be run manually from the repository's Actions tab. + +The custom queries and their tests live in `.github/codeql/`: +- `.github/codeql/queries/` holds the queries (e.g. `openssl-return-check.ql`, + which flags `EVP_*` calls whose status code is discarded instead of checked + with `OQS_OPENSSL_GUARD`) +- `.github/codeql/test//` holds a unit test for each query: a small + C file with good and bad examples, a `.qlref` naming the query, and a + `.expected` file listing the results the query must report. + +The commands below need the CodeQL CLI: either install the GitHub CLI extension +(`gh extension install github/gh-codeql`, then use `gh codeql` in place of +`codeql`), or download a CodeQL bundle from the +[github/codeql-action releases](https://github.com/github/codeql-action/releases). + +To add a query, add the `.ql` file and its test directory, then generate the +`.expected` file and check that it lists exactly the bad examples: + +```bash +codeql test run --learn .github/codeql/test +``` + +Run `codeql test run .github/codeql/test` (without `--learn`) to check that +existing queries still behave as expected.