Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: amd64 builders don't support SHA extensions #69592

Open
FiloSottile opened this issue Sep 23, 2024 · 4 comments
Open

build: amd64 builders don't support SHA extensions #69592

FiloSottile opened this issue Sep 23, 2024 · 4 comments
Assignees
Labels
NeedsFix The path to resolution is known, but the work has not been done.

Comments

@FiloSottile
Copy link
Contributor

The amd64 builders don't support the SHA x86-64 extensions, so our crypto/sha256 assembly is untested, in violation of the Assembly Policy.

/cc @golang/security @golang/release

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/614656 mentions this issue: crypto/sha256,crypto/sha512: test fallback implementations

@gabyhelp
Copy link

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@dr2chase dr2chase added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 24, 2024
cpu pushed a commit to cpu/go that referenced this issue Oct 1, 2024
This will be required for golang#69536 but is also good hygiene and required
by go.dev/wiki/AssemblyPolicy.

> The code must be tested in our CI. This means there need to be
> builders that support the instructions, and if there are multiple (or
> fallback) paths they must be tested separately.

The new crypto/internal/impl registry lets us select alternative
implementations from both the same package and importers (such as
crypto/sha256 tests once we have crypto/internal/fips/sha256, or
crypto/hmac).

Updates golang#69592
Updates golang#69593

Change-Id: Ifea22a9fc9ccffcaf4924ff6bd08da7c9bd39e99
Cq-Include-Trybots: luci.golang.try:gotip-linux-arm64-longtest,gotip-linux-amd64-longtest,gotip-linux-ppc64le_power8,gotip-linux-ppc64_power8
@FiloSottile
Copy link
Contributor Author

@dr2chase not sure why this and #69593 are assigned to me? I think the next step is for @golang/release to figure out if we can select builders with more recent CPUs so we can test this code.

cpu pushed a commit to cpu/go that referenced this issue Oct 16, 2024
This will be required for golang#69536 but is also good hygiene and required
by go.dev/wiki/AssemblyPolicy.

> The code must be tested in our CI. This means there need to be
> builders that support the instructions, and if there are multiple (or
> fallback) paths they must be tested separately.

The new crypto/internal/impl registry lets us select alternative
implementations from both the same package and importers (such as
crypto/sha256 tests once we have crypto/internal/fips/sha256, or
crypto/hmac).

Updates golang#69592
Updates golang#69593

Change-Id: Ifea22a9fc9ccffcaf4924ff6bd08da7c9bd39e99
Cq-Include-Trybots: luci.golang.try:gotip-linux-arm64-longtest,gotip-linux-amd64-longtest,gotip-linux-ppc64le_power8,gotip-linux-ppc64_power8
gopherbot pushed a commit that referenced this issue Oct 23, 2024
This will be required for #69536 but is also good hygiene and required
by go.dev/wiki/AssemblyPolicy.

> The code must be tested in our CI. This means there need to be
> builders that support the instructions, and if there are multiple (or
> fallback) paths they must be tested separately.

The new crypto/internal/impl registry lets us select alternative
implementations from both the same package and importers (such as
crypto/sha256 tests once we have crypto/internal/fips/sha256, or
crypto/hmac).

Updates #69592
Updates #69593

Change-Id: Ifea22a9fc9ccffcaf4924ff6bd08da7c9bd39e99
Cq-Include-Trybots: luci.golang.try:gotip-linux-arm64-longtest,gotip-linux-amd64-longtest,gotip-linux-ppc64le_power8,gotip-linux-ppc64_power8
Reviewed-on: https://go-review.googlesource.com/c/go/+/614656
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
@rolandshoemaker
Copy link
Member

The C3D, N2D instances, based on AMD EYPC, should have these instructions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
Status: No status
Development

No branches or pull requests

5 participants