From 4158ca8d7c521aee5cc48f285f559e74845e973c Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Thu, 8 May 2025 16:27:36 -0700 Subject: [PATCH] crypto: add a test for disallowed instructions WORD and BYTE usage in crypto assembly cores is an anti-pattern which makes extremely sensitive code significantly harder to understand, and can result in unexpected behavior. Because of this, we've decided to ban their usage in the crypto/ tree (as part of the cryptography assembly policy). This test walks the crypto/ tree looking for assembly files (those with the filetype .s) and look for lines that match the regular rexpression "(^|;)\s(BYTE|WORD)\s". Change-Id: I60b5283e05e8588fa53273904a9611a411741f72 Reviewed-on: https://go-review.googlesource.com/c/go/+/671099 Reviewed-by: David Chase LUCI-TryBot-Result: Go LUCI Reviewed-by: Daniel McCarney Auto-Submit: Roland Shoemaker --- src/crypto/crypto_test.go | 53 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/crypto/crypto_test.go b/src/crypto/crypto_test.go index b80fb49c13..66babcc2fb 100644 --- a/src/crypto/crypto_test.go +++ b/src/crypto/crypto_test.go @@ -5,13 +5,19 @@ package crypto_test import ( + "bytes" "crypto" "crypto/rand" "crypto/rsa" "crypto/x509" "encoding/pem" "errors" + "internal/testenv" "io" + "io/fs" + "os" + "path/filepath" + "regexp" "strings" "testing" ) @@ -88,3 +94,50 @@ UjmopwKBgAqB2KYYMUqAOvYcBnEfLDmyZv9BTVNHbR2lKkMYqv5LlvDaBxVfilE0 t.Errorf("VerifyPSS failed for MessageSigner signature: %s", err) } } + +func TestDisallowedAssemblyInstructions(t *testing.T) { + // This test enforces the cryptography assembly policy rule that we do not + // use BYTE or WORD instructions, since these instructions can obscure what + // the assembly is actually doing. If we do not support specific + // instructions in the assembler, we should not be using them until we do. + // + // Instead of using the output of the 'go tool asm' tool, we take the simple + // approach and just search the text of .s files for usage of BYTE and WORD. + // We do this because the assembler itself will sometimes insert WORD + // instructions for things like function preambles etc. + + boringSigPath := filepath.Join("internal", "boring", "sig") + + matcher, err := regexp.Compile(`(^|;)\s(BYTE|WORD)\s`) + if err != nil { + t.Fatal(err) + } + if err := filepath.WalkDir(filepath.Join(testenv.GOROOT(t), "src/crypto"), func(path string, d fs.DirEntry, err error) error { + if err != nil { + t.Fatal(err) + } + if d.IsDir() || !strings.HasSuffix(path, ".s") { + return nil + } + if strings.Contains(path, boringSigPath) { + return nil + } + + f, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + + i := 1 + for line := range bytes.Lines(f) { + if matcher.Match(line) { + t.Errorf("%s:%d assembly contains BYTE or WORD instruction (%q)", path, i, string(line)) + } + i++ + } + + return nil + }); err != nil { + t.Fatal(err) + } +} -- 2.50.0