RISC-V: GCM: Implement .ghash()

[cmuellner]

The current RISC-V GCM implementation does not include a .ghash() callback. This is an issue for future improvements (i.e. adding new GCM implementations for the upcoming vector crypto ISA extensions). Therefore, this PR adds this missing piece.

Unfortunately, this was a bit more work than expected because the current implementation needed to be replaced by an improved .gmult() code sequence to free up registers so the loop in .ghash() could be realized without stack utilization.

During testing, it was observed that no test calls .ghash() with more than 1 block (i.e. 16 bytes) of data. Therefore, I wrote a new one. Plus, yet another one which also triggers another code path in gcm128.c (running into the GHASH_CHUNK limitation).

During the work it was observed that the RISC-V extension test macros deserve some clean-ups to make them easier to read (e.g. extension tests are only relevant for run-time decisions) and extend. The related changes are included in this series.

The .ghash() implementation itself is straightforward. It just includes the XOR-loop into the gmult() sequence.

All code has been tested using upstream QEMU.

Test instructions:

OPENSSL_riscvcap_base="rv64gc"
OPENSSL_riscvcap_zbc="rv64gc_zbc"
OPENSSL_riscvcap_zbc_zbb="rv64gc_zbc_zbb"
OPENSSL_riscvcap_zbc_zbkb="rv64gc_zbc_zbkb"

QEMU_CPU_BASE="rv64"
QEMU_CPU_ZBC="rv64,zbc=true"
QEMU_CPU_ZBC_ZBB="rv64,zbc=true,zbb=true"
QEMU_CPU_ZBC_ZBKB="rv64,zbc=true,zbkb=true"

# Must succeed
OPENSSL_riscvcap=$OPENSSL_riscvcap_base QEMU_CPU=$QEMU_CPU_BASE ../build.sh test build noclean
OPENSSL_riscvcap=$OPENSSL_riscvcap_zbc QEMU_CPU=$QEMU_CPU_ZBC ../build.sh test build noclean
OPENSSL_riscvcap=$OPENSSL_riscvcap_zbc_zbb QEMU_CPU=$QEMU_CPU_ZBC_ZBB ../build.sh test build noclean
OPENSSL_riscvcap=$OPENSSL_riscvcap_zbc_zbkb QEMU_CPU=$QEMU_CPU_ZBC_ZBKB ../build.sh test build noclean

# Must fail (illegal instr)
OPENSSL_riscvcap=$OPENSSL_riscvcap_zbc QEMU_CPU=$QEMU_CPU_BASE ../build.sh test build noclean
OPENSSL_riscvcap=$OPENSSL_riscvcap_zbc_zbb QEMU_CPU=$QEMU_CPU_ZBC ../build.sh test build noclean
OPENSSL_riscvcap=$OPENSSL_riscvcap_zbc_zbkb QEMU_CPU=$QEMU_CPU_ZBC ../build.sh test build noclean

[cmuellner]

Updated PR to address CI errors.

[cmuellner]

Updated PR for yet another CI error (before we had warnings because of dynamic stack arrays, now zero-initializers were not allowed).

I've already sent the ICLA.

[cmuellner]

Updated the PR:

• removed the copyright date change
• reverted the feature test macro renaming (back to RISCV_HAS_*)
• removed the new test

[cmuellner]

Rebased this PR on top of https://github.com/openssl/openssl/pull/20107/commits/7b49d6aeff1e6d744d4355a832bff59ed812c4f5, which introduces cross-compile tests that trigger this PR. See https://github.com/openssl/openssl/pull/20107 for more details.

[cmuellner]

The build was failing because of the use Devel::StackTrace; in the Perl code: https://github.com/openssl/openssl/pull/20078/files#diff-e66a71edbf0eff8e8b1c5afbceac9ddf1e4b41990a8780befd6e47c91770e087R9

The error message was:

Can't locate Devel/StackTrace.pm in @INC (you may need to install the Devel::StackTrace module) (@INC contains: /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.34.0 /usr/local/share/perl/5.34.0 /usr/lib/x86_64-linux-gnu/perl5/5.34 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl-base /usr/lib/x86_64-linux-gnu/perl/5.34 /usr/share/perl/5.34 /usr/local/lib/site_perl) at crypto/modes/asm/ghash-riscv64.pl line 9.

I've updated the PR to fail gracefully if the package perl-stacktrace is not present (there will be no helping stacktrace printed in this case).

[t8m]

It would be nice to get some independent review from someone who knows RISC-V assembly.

[cmuellner]

Ok, understood. I will try to find someone in the RISC-V community to do this review.

[cmuellner]

Updated the PR:

• moved common Perl-Asm code into a Perl module (like other architectures have one as well)
• make Perl-Asm running with use strict
• rebased on upstream/master

[ptomsich]

Note that a specification change for GCM is in progress: https://github.com/riscv/riscv-crypto/commit/19d044f978c1aa71aa189fae7a3442852feff555

We will update this PR (over the course of the next days) as soon as the specification has settled.

[paulidale]

At what point should we expect the standardisation to be completed?

OpenSSL generally doesn't accept algorithms before they are published as a national or international standard. I don't know if this would apply to instruction sets or not but will ask the question of the management committee.

[ptomsich]

Once the standard goes to the 45-day public review (called the "FREEZE"-milestone in RISC-V parlance), no semantic changes are expected. This is the point when the Linux kernel, GCC, etc. accept changes as well. We expect FREEZE/45-day public review to happen by end of this quarter and we will then update the PRs appropriately with a link to the published specification.

However, to make it to FREEZE, prototype patches against upstream projects need to exist (not merged, but publicly available) and have been reviewed to ensure that the proposed extension can be supported in software (i.e., as an end-to-end proof-of-concept requirement).

Just a side-note: the current changes to the ghash instruction (creating a second instruction that is more gmult-like) have been triggered by the experience with exactly this pull-request.

[paulidale]

Thanks for the information.

My question to the OMC is about accepting such patches into the code before they are standardised. I've no problem seeing pull requests and getting them reviewed before such.

[cmuellner]

I think there is a slight misunderstanding, as this PR does not need to be reworked for the spec update. This PR optimizes and improves the current code in OpenSSL, which uses instructions of the final and ratified scalar crypto specification (ratified December 2021).

The RISC-V PR (#20149), which introduces support for vector crypto instructions, is affected by the mentioned change in the upcoming vector crypto specification. The vector crypto PR also clearly states that the instructions use a specification that is not frozen yet and is marked as RFC.

[ptomsich]

Indeed, these comments went onto the wrong one of our PRs — the RFC ones are clearly marked. My comments apply verbatim there. Sorry for any confusion I may have caused.

Thanks for the clarification (and spotting that I had the wrong PR open...), Christoph!

[cmuellner]

Updated the PR:

• rebased on upstream/master and retested
• worked in comments from @nibrunieAtSi5 (Thanks!)

[cmuellner]

There were some minor comments (regarding code comments) in the review of #20149 which affected this PR.

Updated PR:

• worked in comments
• rebased on master
• retested

[openssl-machine]

This pull request is ready to merge

[paulidale]

Merged, thanks for the contribution.