From 2ab56c8a92f07b8c883602801ebc21137de78120 Mon Sep 17 00:00:00 2001 From: InterLinked1 <24227567+InterLinked1@users.noreply.github.com> Date: Mon, 23 Sep 2024 18:54:29 -0400 Subject: [PATCH] crypt.c: Don't attempt to seed crypt_data.current_salt. (#43) current_salt is a deprecated field that is only set when NEED_CRYPTO_IMPL is defined, such as on Debian 10 and other older distros that don't provide a sufficiently capable crypt_r. However, some distros, such as Rocky Linux 8, don't even have these fields in their crypt_data struct. Since it's not clear what this accomplished and appears to be unnecessary, just disable this for now. To ensure this doesn't cause a regression, all the Linux CI's now also run at least the menu tests, to ensure that authentication succeeds (since that can rely on the affected code). --- .github/workflows/codeql.yml | 6 ++--- .github/workflows/main.yml | 51 +++++++++++++++++++++++++++--------- bbs/crypt.c | 7 ++++- 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 17ba4974..1cd31601 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -46,11 +46,11 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL - uses: github/codeql-action/init@v2 + uses: github/codeql-action/init@v3 with: languages: ${{ matrix.language }} # If you wish to specify custom queries, you can do so here or in a config file. @@ -71,6 +71,6 @@ jobs: make - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v2 + uses: github/codeql-action/analyze@v3 with: category: "/language:${{matrix.language}}" diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 5afc6881..792eb0aa 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -14,10 +14,10 @@ jobs: runs-on: ubuntu-24.04 name: Ubuntu 24.04 steps: - - name: Checkout - uses: actions/checkout@v4 - - name: Build LBBS - run: | + - name: Checkout + uses: actions/checkout@v4 + - name: Build LBBS + run: | sudo sed -i 's/azure\.//' /etc/apt/sources.list sudo ./scripts/install_prereq.sh sudo make modcheck @@ -25,8 +25,8 @@ jobs: sudo make install sudo make samples sudo make tests - - name: Run tests - run: | + - name: Run tests + run: | sudo tests/test -ddddddddd -DDDDDDDDDD -x sudo apt-get install -y valgrind sudo tests/test -ddddddddd -DDDDDDDDDD -ex @@ -34,10 +34,10 @@ jobs: runs-on: ubuntu-22.04 name: Ubuntu 22.04 steps: - - name: Checkout - uses: actions/checkout@v4 - - name: Build LBBS - run: | + - name: Checkout + uses: actions/checkout@v4 + - name: Build LBBS + run: | sudo sed -i 's/azure\.//' /etc/apt/sources.list sudo ./scripts/install_prereq.sh sudo make modcheck @@ -45,8 +45,8 @@ jobs: sudo make install sudo make samples sudo make tests - - name: Run tests - run: | + - name: Run tests + run: | sudo tests/test -ddddddddd -DDDDDDDDDD -x sudo apt-get install -y valgrind sudo tests/test -ddddddddd -DDDDDDDDDD -ex @@ -64,6 +64,9 @@ jobs: make install make samples make tests + - name: Run basic tests + run: | + tests/test -ttest_menus -ddddddddd -DDDDDDDDDD -x debian-11: runs-on: ubuntu-24.04 name: Debian 11 @@ -78,6 +81,9 @@ jobs: make install make samples make tests + - name: Run basic tests + run: | + tests/test -ttest_menus -ddddddddd -DDDDDDDDDD -x debian-10: runs-on: ubuntu-24.04 name: Debian 10 @@ -92,6 +98,9 @@ jobs: make install make samples make tests + - name: Run basic tests + run: | + tests/test -ttest_menus -ddddddddd -DDDDDDDDDD -x fedora-42: runs-on: ubuntu-24.04 name: Fedora 42 @@ -107,6 +116,9 @@ jobs: make install make samples make tests + - name: Run basic tests + run: | + tests/test -ttest_menus -ddddddddd -DDDDDDDDDD -x fedora-40: runs-on: ubuntu-24.04 name: Fedora 40 @@ -122,6 +134,9 @@ jobs: make install make samples make tests + - name: Run basic tests + run: | + tests/test -ttest_menus -ddddddddd -DDDDDDDDDD -x rocky-9: runs-on: ubuntu-24.04 name: Rocky Linux 9.3 @@ -137,6 +152,9 @@ jobs: make install make samples make tests + - name: Run basic tests + run: | + tests/test -ttest_menus -ddddddddd -DDDDDDDDDD -x rocky-8: runs-on: ubuntu-24.04 name: Rocky Linux 8.9 @@ -152,6 +170,9 @@ jobs: make install make samples make tests + - name: Run basic tests + run: | + tests/test -ttest_menus -ddddddddd -DDDDDDDDDD -x opensuse: runs-on: ubuntu-24.04 name: openSUSE Tumbleweed @@ -167,6 +188,9 @@ jobs: make install make samples make tests + - name: Run basic tests + run: | + tests/test -ttest_menus -ddddddddd -DDDDDDDDDD -x archlinux: runs-on: ubuntu-24.04 name: Arch Linux @@ -182,6 +206,9 @@ jobs: make install make samples make tests + - name: Run basic tests + run: | + tests/test -ttest_menus -ddddddddd -DDDDDDDDDD -x freebsd-14: runs-on: ubuntu-24.04 name: FreeBSD diff --git a/bbs/crypt.c b/bbs/crypt.c index 8811713a..5965e08d 100644 --- a/bbs/crypt.c +++ b/bbs/crypt.c @@ -198,8 +198,13 @@ char *bbs_password_hash(const char *password, const char *salt) #ifdef CRYPT_DEBUG bbs_debug(9, "Using alternate crypt_r\n"); #endif /* CRYPT_DEBUG */ - data.current_salt[0] = '$'; +#if 0 + /* XXX Modern documentation doesn't really discuss the purpose of this field or when you would want + * to seed the first two bytes like this, and I don't really remember why this was done now. + * Also, on some platforms these fields aren't available, so just disable this for now. */ + data.current_salt[0] = '$'; /* See encrypt(3) */ data.current_salt[1] = '2'; +#endif hash = __crypt_r(password, salt, &data); /* Use our custom implementation of crypt_r */ #endif /* NEED_CRYPTO_IMPL */