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

cmd/go: MacOS binaries invalid for eventual Apple Notary #30488

Closed
macetw opened this issue Feb 28, 2019 · 49 comments

Comments

@macetw
Copy link

commented Feb 28, 2019

What version of Go are you using (go version)?

1.12

Does this issue reproduce with the latest release?

Yes. (1.12)

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

go env Output
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/macet/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/macet/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/9v/6zqn9ncn39x63s0j25sqh7z00000gn/T/go-build033883546=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I applied my executable for an Apple Notary, required for release with the App Store on MacOS 10.14. Apple Notary gives users the assurance that their application is safe, but binaries must have the notary approval "stapled" to their .app. Apple Notary requirements include "runtime" option with a code signature and the MacOS SDK be "10.9" or newer. This is seen with "otool -l." With go, binaries are "10.7" based.

Here is the result of submission to Apple, presented in JSON format:
{"severity": "error", "code": null, "path": "mygobasedapplication.dmg/my/go/based/application", "message": "The binary uses an SDK older than the 10.9 SDK.", "docUrl": null}

What did you expect to see?

  version 10.9
      sdk 10.9

What did you see instead?

% otool -l main | tail -n4
      cmd LC_VERSION_MIN_MACOSX
  cmdsize 16
  version 10.7
      sdk 10.7
@macetw

This comment has been minimized.

Copy link
Author

commented Mar 1, 2019

This is an update on top of issue #12941

For Apple documentation of this, see:
Url: https://developer.apple.com/documentation/security/notarizing_your_app_before_distribution/resolving_common_notarization_issues
Section: "Use the macOS 10.9 SDK or Later"

@randall77

This comment has been minimized.

Copy link
Contributor

commented Mar 1, 2019

Is there anything we need to do other than change the constant?

@bradfitz bradfitz added this to the Go1.13 milestone Mar 1, 2019

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Mar 1, 2019

This is set in the linker
https://go.googlesource.com/go/+/refs/heads/master/src/cmd/link/internal/ld/macho.go#665

It seems this is set to the minimum version Go required at the time this code was written.

@macetw

This comment has been minimized.

Copy link
Author

commented Mar 1, 2019

At this time, I don't have full proof that this is the only change, but based on other evidence we have from a suite of binaries submitted, this is the only change: to change that constant in macho.go.

There are other requirements for Apple Notary, but those requirements fall outside of the scope of the compiler. They are more around the codesign step. Go binaries can be signed like any other binary.

@networkimprov

This comment has been minimized.

Copy link

commented Mar 1, 2019

Seems like a back-port candidate?

@macetw

This comment has been minimized.

Copy link
Author

commented Mar 1, 2019

@networkimprov yes, this is viable for back-port also. I'm so far unfamiliar with how I would approach that.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Mar 1, 2019

@gopherbot please open backport issues

@gopherbot

This comment has been minimized.

Copy link

commented Mar 1, 2019

Backport issue(s) opened: #30525 (for 1.11), #30526 (for 1.12).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@macetw

This comment has been minimized.

Copy link
Author

commented Mar 12, 2019

I plan to submit code changes for this, including the backports.

@macetw

This comment has been minimized.

Copy link
Author

commented Mar 12, 2019

How can I be recognized as the "assignee?"

@macetw

This comment has been minimized.

Copy link
Author

commented Mar 12, 2019

Also, how can we recognize this no longer as "Needs Investigation?"

@macetw

This comment has been minimized.

Copy link
Author

commented Mar 12, 2019

Haha. I just get @randall77 to do all my work. Thank you, Keith!

@networkimprov

This comment has been minimized.

Copy link

commented Mar 12, 2019

Gopherbot listens for commands re label manipulation, e.g.

@gopherbot add NeedsFix

Someone pls refresh my memory re the delete/drop command...

@networkimprov

This comment has been minimized.

Copy link

commented Mar 12, 2019

Can this patch provide a script to pull the version string from a canonical source, if networking is available?

@macetw

This comment has been minimized.

Copy link
Author

commented Mar 12, 2019

@networkimprov help me understand your question.
That "10.7" or "10.9" (or whatever) version numbers (not strings) are in linked binaries. They do not appear in source.

@macetw

This comment has been minimized.

Copy link
Author

commented Mar 12, 2019

At this time, I have a "No Contributor Agreement on file for user Tyler Mace" error, but I just got one completed this morning (via Autodesk). How long is the lag time for this to take effect? Is there a better forum for this question, other than my issue ticket?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2019

@macetw If you configure GitHub such that your e-mail address is @autodesk.com then I think the CLA robot will recognize that you are approved. If you don't want to do that then, if you are willing, I recommend that you sign the personal CLA as well, using the same e-mail address as your GitHub account.

@macetw

This comment has been minimized.

Copy link
Author

commented Mar 12, 2019

@ianlancetaylor the google-group had both my private email and my corporate email, but in the contrib guide, it was said to use consistent email addresses. (I had already done work as macetw@gmail.com). You're suggesting that I not be consistent, but if that's okay, my employer would actually prefer it I use my corporate email.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2019

@macetw Are you using Gerrit or GitHub (go-review.googlesource.com or github.com)? I believe that Gerrit lets you have multiple e-mail addresses.

If you've already contributed under the e-mail address macetw@gmail.com then I don't know why that would not still work today.

@macetw

This comment has been minimized.

Copy link
Author

commented Mar 12, 2019

@ianlancetaylor I haven't contributed lines of code yet. I've merely contributed via github issue conversations, like here. I'm new today on the CLA.

@networkimprov

This comment has been minimized.

Copy link

commented Mar 12, 2019

@macetw I should have written "hex constant".

Seems like its value should be pulled from an online source or config file via //go: generate

@networkimprov

This comment has been minimized.

Copy link

commented Mar 12, 2019

@gopherbot add release-blocker

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2019

From the document @macetw linked, macOS 10.9 or later is required "because of significant differences in the way code signing works prior to macOS 10.9". So it seems this doesn't need to change very frequently, therefore not necessarily release-blocker on every release.

@networkimprov

This comment has been minimized.

Copy link

commented Mar 13, 2019

The point is that it should be checked for every release, not that it will change.

@tmm1

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

Note that if https://golang.org/cl/168321 is merged, one way to override the minimum version set by golang would be by using CGO_ENABLED=1 CGO_CFLAGS=-mmacosx-version-min=10.9 go build

@bcmills

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

@macetw, are you still having CLA trouble?

@pieterclaerhout

This comment has been minimized.

Copy link

commented May 7, 2019

Any idea when this will be released?

We're actually waiting for this fix to be able to notarize our binaries for macOS. Since Apple will be pushing this after WWDC beginning of june, it would be awesome to be ready…

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented May 7, 2019

As far as I know this isn't fixed yet. I'm not sure what is happening here. We've currently marked this as a release blocker for Go 1.13, which will be released around August 1.

@pieterclaerhout

This comment has been minimized.

Copy link

commented May 7, 2019

:-(

Is there any way to get this fixed earlier? I presume we're not the only ones running into this, right?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented May 7, 2019

The first step is to get it fixed at all. Once we have a fix we can discuss backporting that fix to a release branch. Right now, as far as I know, it is not fixed.

@networkimprov

This comment has been minimized.

Copy link

commented May 7, 2019

It's a minor change to a pair of constant expressions, and already marked for backport to 1.11 & 1.12.

https://github.com/golang/go/tree/master/src/cmd/link/internal/ld/macho.go#L418

This issue should perhaps be reassigned?

EDIT: mistakenly added "help" and "wanted" whilst trying to add "help wanted"

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented May 7, 2019

If anyone has a patch to send that fixes this problem, please go ahead.

@tmm1

This comment has been minimized.

Copy link
Contributor

commented May 7, 2019

FYI as of 277609f the version number is copied from external objects when available, so compiling with CGO_ENABLED=1 MACOSX_DEPLOYMENT_TARGET=10.14 will produce a notary compatible binary.

@pieterclaerhout

This comment has been minimized.

Copy link

commented May 7, 2019

FYI as of 277609f the version number is copied from external objects when available, so compiling with CGO_ENABLED=1 MACOSX_DEPLOYMENT_TARGET=10.14 will produce a notary compatible binary.

In which version of Go will that be available?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented May 7, 2019

That CL was part of the work for #22395 and it will be available in the 1.13 release.

@eliasnaur @cherrymui Would it be appropriate to backport https://golang.org/cl/168321 to a Go 1.12 release?

@eliasnaur

This comment has been minimized.

Copy link
Contributor

commented May 7, 2019

CL 168321 is not entirely trivial and I believe it requires CL 168459 to apply. It doesn't even fix the root cause: the 10.7 version is still hardcoded in tip. That the CL enables a workaround as described by @tmm1 is accidental.

I propose someone mails the trivial change discussed earlier in this issue that bumps the version to 10.9 in tip, fixing this issue. The scope of the change is much narrower and can be manually backported. Would that be acceptable?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented May 7, 2019

I propose someone mails the trivial change discussed earlier in this issue that bumps the version to 10.9 in tip, fixing this issue. The scope of the change is much narrower and can be manually backported. Would that be acceptable?

Yes, I think so. Thanks.

@gopherbot

This comment has been minimized.

Copy link

commented May 8, 2019

Change https://golang.org/cl/175918 mentions this issue: cmd/link/internal/ld: bump macOS and macOS SDK version to 10.9

@gopherbot gopherbot closed this in 4cd6c3b May 8, 2019

@pieterclaerhout

This comment has been minimized.

Copy link

commented May 8, 2019

Can you make sure this ends up in 1.12 as well?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented May 8, 2019

@pieterclaerhout The 1.12 backport is #30526.

@networkimprov

This comment has been minimized.

Copy link

commented May 8, 2019

Ian, could you reopen this? It needs to be reviewed before each release, as the value will change again.

Also its milestone should be set appropriately.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented May 8, 2019

@networkimprov Would you mind opening a new issue for that? This issue is pretty cluttered, and I wouldn't want people to have to read through all of it for each release. Thanks.

@networkimprov

This comment has been minimized.

Copy link

commented May 8, 2019

Filed #31918, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.