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

crypto/internal/boring/syso: boringssl builds on musl broken due to fopen64 calls #64698

Closed
ashishb-solo opened this issue Dec 13, 2023 · 7 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@ashishb-solo
Copy link

ashishb-solo commented Dec 13, 2023

Go version

1.21.5

What operating system and processor architecture are you using (go env)?

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/root/.cache/go-build'
GOENV='/root/.config/go/env'
GOEXE=''
GOEXPERIMENT='boringcrypto'
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/root/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/root/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/usr/lib/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.5'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/go/src/example/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3533798560=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I attempted to build a go program using cgo and boringssl in a Alpine 3.19 dockerfile:

FROM alpine:3.19

RUN mkdir -p /go/src/example
WORKDIR /go/src/example
RUN apk add go gcc musl-dev libc6-compat
ENV GOEXPERIMENT=boringcrypto
ENV CGO_ENABLED=1
RUN echo $'package main \n\
 \n\
import ( \n\
	"crypto/boring" \n\
	"fmt" \n\
) \n\
 \n\
func main() { \n\
	fmt.Println(boring.Enabled()) \n\
} \n\
' > main.go
RUN go mod init example && go mod tidy
RUN go env
RUN go build main.go

Changing the base image from Alpine 3.19 to 3.18 causes compilation to succeed, so this is due to a recent change in musl where symbols like fopen64 were removed (see here). However, the following libraries still make calls to the removed fopen64 symbol:

ashish@localhost /dev/shm/go $ git grep -w fopen64
Binary file src/crypto/internal/boring/syso/goboringcrypto_linux_amd64.syso matches
Binary file src/crypto/internal/boring/syso/goboringcrypto_linux_arm64.syso matches

What did you expect to see?

Compilation should succeed

What did you see instead?

The above quoted dockerfile failed with the following error message:

# command-line-arguments
/usr/lib/go/pkg/tool/linux_amd64/link: running gcc failed: exit status 1
/usr/lib/gcc/x86_64-alpine-linux-musl/13.2.1/../../../../x86_64-alpine-linux-musl/bin/ld: /tmp/go-link-795436458/000023.o: in function `BIO_new_file':
(.text+0xcb1dc): undefined reference to `fopen64'
/usr/lib/gcc/x86_64-alpine-linux-musl/13.2.1/../../../../x86_64-alpine-linux-musl/bin/ld: /tmp/go-link-795436458/000023.o: in function `BIO_rw_filename':
(.text+0xcb60d): undefined reference to `fopen64'
collect2: error: ld returned 1 exit status
@gopherbot gopherbot added this to the Unreleased milestone Dec 13, 2023
@seankhliao seankhliao changed the title x/crypto: boringssl builds on musl broken due to fopen64 calls crypto/internal/boring/syso: boringssl builds on musl broken due to fopen64 calls Dec 13, 2023
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 13, 2023
@seankhliao
Copy link
Member

cc @golang/security

@mauri870
Copy link
Member

That appears to be a problem with boringssl upstream https://github.com/google/boringssl/blob/master/crypto/bio/file.c#L57-L72.

#if defined(__linux) || defined(__sun) || defined(__hpux)
// Following definition aliases fopen to fopen64 on above mentioned
// platforms. This makes it possible to open and sequentially access
// files larger than 2GB from 32-bit application. It does not allow to
// traverse them beyond 2GB with fseek/ftell, but on the other hand *no*
// 32-bit platform permits that, not with fseek/ftell. Not to mention
// that breaking 2GB limit for seeking would require surgery to *our*
// API. But sequential access suffices for practical cases when you
// can run into large files, such as fingerprinting, so we can let API
// alone. For reference, the list of 32-bit platforms which allow for
// sequential access of large files without extra "magic" comprise *BSD,
// Darwin, IRIX...
#ifndef _FILE_OFFSET_BITS
#define _FILE_OFFSET_BITS 64
#endif
#endif

From what I understood, all system calls in musl are 64bit, so there is no need to use the 64 variants of the system calls. I'm not sure how to detect this tho. Maybe always compile the syso with the 32 bit versions? I'm not sure what are the downsides of this for glibc. Boring being a GOEXPERIMENT I don't know if it is even supposed to work on musl.

@davidben
Copy link
Contributor

That #define only does whatever the libc headers decides to do with it. If musl does not provide suffixed symbols, one can assume its headers will not swap over to them.

It sounds like you may have built BoringSSL against one libc's headers and then linked it against a different libc. That cannot work. You need to build against the headers for the library you're linking against.

@mauri870
Copy link
Member

mauri870 commented Dec 14, 2023

So the problem, in summary, is:

  • musl dropped support for the 64 bit names of libc functions, its functions are 64bit anyway.
  • the boring syso file we provide has 64bit variants in it, this causes compilation errors with latest musl

It is not clear to me a path to resolution that does not involve either us recompiling the syso files using only the normal variants or musl providing some kind of solution to this, but since they already removed the 64bit shims I think they expect us to fix it somehow.

@FiloSottile
Copy link
Contributor

It sounds like you may have built BoringSSL against one libc's headers and then linked it against a different libc.

We build the FIPS module against glibc and then check in the object file, so that's effectively what's happening, although it's not clear to me why the BoringCrypto module would call fopen64.

@davidben
Copy link
Contributor

Ah. I don't think anything in BCM itself calls it, but since BCM is an internal chunk of libcrypto, I assume you're building all of libcrypto and picking it up by way of the file BIO.

I assume musl does not promise to have the same ABI as glibc (given they removed a symbol glibc provides...), even if it probably accidentally matches for simple stuff. So I think that means a musl and glibc build would want to be different, or this mode should just only support glibc.

@rsc
Copy link
Contributor

rsc commented Dec 14, 2023

I'm sorry but BoringCrypto is not supported for use outside Google. We share the code with the world in case anyone wants it, but when it breaks in different environments, we don't support it. Musl is a source of many problems generally so it's unsurprising it would be a problem here too, but we're not going to take on the complexity of making musl work and keeping it working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

7 participants