From d01c7ffbc56c63e73a2130a77d23f80ac1925881 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Thu, 18 Aug 2022 20:12:10 +0200 Subject: [PATCH] fix: add support for darwin/arm64 (#7) This diff ensures we hardcode the capabilities of darwin/arm64. While there, strive to make the packages naming slightly less confusing. Reference issue: ooni/probe#2122. Surpersedes (and draws from) #5. Co-authored-by: ain ghazal ainghazal42@gmail.com Co-authored-by: ain ghazal --- aes/cipher_asm.go | 6 +-- internal/cpuarm64/cpuarm64_otherwise.go | 15 ------- internal/cpuarm64/doc.go | 22 ---------- internal/cpuoverlay/cpoverlay_otherwise.go | 22 ++++++++++ internal/cpuoverlay/cpuoverlay.go | 44 +++++++++++++++++++ .../cpuoverlay_android_arm64.go} | 18 +++++--- .../cpuoverlay/cpuoverlay_darwin_arm64.go | 24 ++++++++++ tls/cipher_suites.go | 4 +- 8 files changed, 107 insertions(+), 48 deletions(-) delete mode 100644 internal/cpuarm64/cpuarm64_otherwise.go delete mode 100644 internal/cpuarm64/doc.go create mode 100644 internal/cpuoverlay/cpoverlay_otherwise.go create mode 100644 internal/cpuoverlay/cpuoverlay.go rename internal/{cpuarm64/cpuarm64_android_arm64.go => cpuoverlay/cpuoverlay_android_arm64.go} (80%) create mode 100644 internal/cpuoverlay/cpuoverlay_darwin_arm64.go diff --git a/aes/cipher_asm.go b/aes/cipher_asm.go index 25dc6b92..2c4bb5fe 100644 --- a/aes/cipher_asm.go +++ b/aes/cipher_asm.go @@ -9,7 +9,7 @@ package aes import ( "crypto/cipher" - "github.com/ooni/oocrypto/internal/cpuarm64" + "github.com/ooni/oocrypto/internal/cpuoverlay" "github.com/ooni/oocrypto/internal/subtle" "golang.org/x/sys/cpu" ) @@ -29,8 +29,8 @@ type aesCipherAsm struct { aesCipher } -var supportsAES = cpu.X86.HasAES || cpuarm64.HasAES() -var supportsGFMUL = cpu.X86.HasPCLMULQDQ || cpuarm64.HasPMULL() +var supportsAES = cpu.X86.HasAES || cpuoverlay.Arm64HasAES() +var supportsGFMUL = cpu.X86.HasPCLMULQDQ || cpuoverlay.Arm64HasPMULL() // aesCipherGCM implements crypto/cipher.gcmAble so that crypto/cipher.NewGCM // will use the optimised implementation in aes_gcm.go when possible. diff --git a/internal/cpuarm64/cpuarm64_otherwise.go b/internal/cpuarm64/cpuarm64_otherwise.go deleted file mode 100644 index 69f57705..00000000 --- a/internal/cpuarm64/cpuarm64_otherwise.go +++ /dev/null @@ -1,15 +0,0 @@ -//go:build !android || !arm64 - -package cpuarm64 - -import "golang.org/x/sys/cpu" - -// HasAES returns whether the CPU supports AES. -func HasAES() bool { - return cpu.ARM64.HasAES -} - -// HasPMULL returns whether the CPU supports PMULL. -func HasPMULL() bool { - return cpu.ARM64.HasPMULL -} diff --git a/internal/cpuarm64/doc.go b/internal/cpuarm64/doc.go deleted file mode 100644 index f049f1b8..00000000 --- a/internal/cpuarm64/doc.go +++ /dev/null @@ -1,22 +0,0 @@ -// Package cpuarm64 is a getauxval aware proxy for arm64 CPUs. -// -// The problem that we want to solve is that on Android there are -// cases where reading /proc/self/auxv is not possible. -// -// This causes crypto/tls to not choose AES where it would otherwise -// be possible, in turn causing censorship. See also the -// https://github.com/ooni/probe/issues/1444 issue for more details. -// -// Ideally we would like to call getauxval(3) when initializing -// the runtime package. However, runtime cannot use CGO. Doing that -// leads to an import loop, so we cannot build. -// -// We could also try to parse /proc/cpuinfo (I didn't explore this route). -// -// The solution chosen here is to export predicates on the CPU -// functionality. We limit ourselves to what we need in order to -// choose AES in crypto/tls when the CPU supports it. -// -// This package is only a replacement for arm64. We use x/sys/cpu for -// all arm64 systems but Android where we call getauxval(3). -package cpuarm64 diff --git a/internal/cpuoverlay/cpoverlay_otherwise.go b/internal/cpuoverlay/cpoverlay_otherwise.go new file mode 100644 index 00000000..5e54e6c6 --- /dev/null +++ b/internal/cpuoverlay/cpoverlay_otherwise.go @@ -0,0 +1,22 @@ +//go:build !arm64 || (!darwin && !android) + +package cpuoverlay + +import "golang.org/x/sys/cpu" + +// +// This file is built when we're not on arm64 or we're on windows/arm64. In the +// former case, just returning false would do. In the latter case, the right thing +// to do is to return cpu.ARM64.HasXXX. Because cpu.ARM64.HasXXX are always false +// when not on arm64, we conflate these two cases in a single file. +// + +// arm64HasAES returns whether the CPU supports AES. +func arm64HasAES() bool { + return cpu.ARM64.HasAES +} + +// arm64HasPMULL returns whether the CPU supports PMULL. +func arm64HasPMULL() bool { + return cpu.ARM64.HasPMULL +} diff --git a/internal/cpuoverlay/cpuoverlay.go b/internal/cpuoverlay/cpuoverlay.go new file mode 100644 index 00000000..0cf546c3 --- /dev/null +++ b/internal/cpuoverlay/cpuoverlay.go @@ -0,0 +1,44 @@ +// Package cpuoverlay is a logic overlay on top of x/sys/cpu that +// attempts to avoid cases in which x/sys/cpu is wrong. +// +// android/arm64 +// +// The main problem that we want to solve is that on Android there are +// cases where reading /proc/self/auxv is not possible. +// +// This causes crypto/tls to not choose AES where it would otherwise +// be possible, in turn causing censorship. See also the +// https://github.com/ooni/probe/issues/1444 issue for more details. +// +// Ideally we would like to call getauxval(3) when initializing +// the runtime package. However, runtime cannot use CGO. Doing that +// leads to an import loop, so we cannot build. +// +// Until this is fixed in src/runtime, we call getauxval(3) here. +// +// darwin/arm64 +// +// Additionally, we may use this package to solve other CPU issues. For +// example, x/sys/cpu does not currently know about darwin/arm64 features. +// +// Design +// +// This package contains GOOS/GOARCH-specific files with predicate +// functions returning the correct value in cases in which x/sys/cpu +// is wrong. You are expected to replace the code that normally +// lives inside src/crypto/tls and src/cryto/aes and that we have +// forked in this repository such that you call the predicates +// of this package as opposed to using directly x/sys/cpu values. +package cpuoverlay + +// We dispatch to GOOS/GOARCH specific implementations + +// Arm64HasAES returns whether the CPU supports AES. +func Arm64HasAES() bool { + return arm64HasAES() +} + +// Arm64HasPMULL returns whether the CPU supports PMULL. +func Arm64HasPMULL() bool { + return arm64HasPMULL() +} diff --git a/internal/cpuarm64/cpuarm64_android_arm64.go b/internal/cpuoverlay/cpuoverlay_android_arm64.go similarity index 80% rename from internal/cpuarm64/cpuarm64_android_arm64.go rename to internal/cpuoverlay/cpuoverlay_android_arm64.go index 289c8c93..927eaa78 100644 --- a/internal/cpuarm64/cpuarm64_android_arm64.go +++ b/internal/cpuoverlay/cpuoverlay_android_arm64.go @@ -1,4 +1,4 @@ -//go:build android && arm64 +//go:build arm64 && android // Copyright 2019 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style @@ -7,7 +7,13 @@ // This file is based on the diff available at // https://go-review.googlesource.com/c/sys/+/197540/ -package cpuarm64 +package cpuoverlay + +// +// On android/arm64 /proc/sys/auxv is not readable on most +// systems, therefore we need to call getauxval to load the +// correct values, otherwise we think there's no HW AES. +// /* #include @@ -67,12 +73,12 @@ const ( hwcap_CPUID = 1 << 11 ) -// HasAES returns whether the CPU supports AES. -func HasAES() bool { +// arm64HasAES returns whether the CPU supports AES. +func arm64HasAES() bool { return (gethwcap() & hwcap_AES) != 0 } -// HasPMULL returns whether the CPU supports PMULL. -func HasPMULL() bool { +// arm64HasPMULL returns whether the CPU supports PMULL. +func arm64HasPMULL() bool { return (gethwcap() & hwcap_PMULL) != 0 } diff --git a/internal/cpuoverlay/cpuoverlay_darwin_arm64.go b/internal/cpuoverlay/cpuoverlay_darwin_arm64.go new file mode 100644 index 00000000..91b597de --- /dev/null +++ b/internal/cpuoverlay/cpuoverlay_darwin_arm64.go @@ -0,0 +1,24 @@ +//go:build arm64 && darwin + +package cpuoverlay + +// +// As documented below, there's no support for detecting the +// capabilities of darwin/arm64 in x/sys/cpu so we're going to +// do what internal/cpu currently is doing, i.e., hardcoding +// true because we know M1 supports these HW capabilities. +// +// See https://github.com/ooni/probe/issues/2122 +// +// See https://github.com/golang/go/issues/43046 +// + +// arm64HasAES returns whether the CPU supports AES. +func arm64HasAES() bool { + return true +} + +// arm64HasPMULL returns whether the CPU supports PMULL. +func arm64HasPMULL() bool { + return true +} diff --git a/tls/cipher_suites.go b/tls/cipher_suites.go index d5ca4022..a28f4c9b 100644 --- a/tls/cipher_suites.go +++ b/tls/cipher_suites.go @@ -18,7 +18,7 @@ import ( "github.com/ooni/oocrypto/aes" "github.com/ooni/oocrypto/internal/boring" - "github.com/ooni/oocrypto/internal/cpuarm64" + "github.com/ooni/oocrypto/internal/cpuoverlay" "golang.org/x/crypto/chacha20poly1305" "golang.org/x/sys/cpu" ) @@ -357,7 +357,7 @@ var defaultCipherSuitesTLS13NoAES = []uint16{ var ( hasGCMAsmAMD64 = cpu.X86.HasAES && cpu.X86.HasPCLMULQDQ - hasGCMAsmARM64 = cpuarm64.HasAES() && cpuarm64.HasPMULL() + hasGCMAsmARM64 = cpuoverlay.Arm64HasAES() && cpuoverlay.Arm64HasPMULL() // Keep in sync with crypto/aes/cipher_s390x.go. hasGCMAsmS390X = cpu.S390X.HasAES && cpu.S390X.HasAESCBC && cpu.S390X.HasAESCTR && (cpu.S390X.HasGHASH || cpu.S390X.HasAESGCM)