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

After last update IPFS-Desktop fails duet macOS signature virification #1474

Closed
Gozala opened this issue Apr 30, 2020 · 10 comments · Fixed by Kirobi92/ipfs-desktop#1 · 4 remaining pull requests
Closed

After last update IPFS-Desktop fails duet macOS signature virification #1474

Gozala opened this issue Apr 30, 2020 · 10 comments · Fixed by Kirobi92/ipfs-desktop#1 · 4 remaining pull requests
Labels
need/analysis Needs further analysis before proceeding need/author-input Needs input from the original author need/triage Needs initial labeling and prioritization

Comments

@Gozala
Copy link

Gozala commented Apr 30, 2020

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Steps to reproduce the behavior:

  1. Restart IPFS Desktop after seeing an upgrade message.

Expected behavior
IPFS Desktop should not fail after it updated itself, or for the very least it should provide some actionable instruction to the user.

Screenshots
If applicable, add screenshots to help explain your problem.

image

Desktop (please complete the following information):

  • OS: macOS 10.15.3
  • Version: Presumably whatever the last update was

Additional context
Add any other context about the problem here.

@Gozala Gozala added the need/triage Needs initial labeling and prioritization label Apr 30, 2020
@Gozala
Copy link
Author

Gozala commented Apr 30, 2020

image

Version it updated to

@Gozala
Copy link
Author

Gozala commented Apr 30, 2020

Still seem to work, I assume fs-repo-migrations was supposed to migrate the repo and failed due to apple's about com.apple.quarantine xattr. Not sure where that binary is nor I see the same error on restarting the IPFS Desktop.

@hacdias
Copy link
Member

hacdias commented Apr 30, 2020

"FirefoxNighty.app downloaded this file on June 12, 2017" are you sure this was caused by IPFS Desktop? Multiple people on macOS Catalina tried it out and no one had an error.

It seems to be trying to execute something downloaded in 2017, which sounds fishy. Did you haver install that binary on your system? Can you try which fs-repo-migrations?

I'm not sure about the behaviour of go-ipfs in relation of that binary.

/cc @lidel

@lidel
Copy link
Member

lidel commented Apr 30, 2020

I don't believe fs-repo-migrations is shipped with Desktop, but I think I know why that old binary was called when you restarted the app.

go-ipfs shipped with Desktop app is run with --migrate for seamless repo migrations. With that parameter, go-ipfs checks if fs-repo-migrations is on PATH, and if not, downloads it:

https://github.com/ipfs/go-ipfs/blob/v0.5.0/repo/fsrepo/migrations/migrations.go#L40

Sounds like you had an older version on your path, which you manually downloaded in 2017, and that metadata caused macOS Catalina to show this error. (macOS started requiring notarization recently for things downloaded with a web browser, but using wget or curl should be fine)

@Gozala I believe the fix here is to remove old fs-repo-migrations – can you confirm if it removed the error?

cc @Stebalien to be aware lack of notarization of fs-repo-migrations may be a problem if people downloaded them manually in past.

@lidel lidel added need/analysis Needs further analysis before proceeding need/author-input Needs input from the original author labels Apr 30, 2020
@Stebalien
Copy link
Member

@lidel can we notarize those old fs-repo-migrations binaries?

@Stebalien
Copy link
Member

That is, write a script to notarize everything in dist.ipfs.io?

@Stebalien
Copy link
Member

Actually, can we do this in CI somewhere?

@lidel
Copy link
Member

lidel commented May 4, 2020

@Stebalien non-Electron projects should be able to use https://github.com/mitchellh/gon
Someone familiar with macOS can run it locally (gon must be run on a macOS machine with XCode 11.0 or later) to manually notarize old binaries or set up it in CI to run on mac worker.

Caveats:

  • Notarization stappling changes the .dmg – not sure how it works when standalone binary is notarized
    • for old stuff, to keep binaries and hashes intact, we may ignore stappled bianry and only rely on online notarization (requires pinging apple servers, but better than nothing/error)
    • for new stuff notarization with stappling needs to happen before checksums/cids are calculated for dist.ipfs.io manifests
  • When set up at CI, ensure secrets are safe (eg. exposing them only on release branch).
  • I have no mac 🙃

@Gozala
Copy link
Author

Gozala commented May 4, 2020

go-ipfs shipped with Desktop app is run with --migrate for seamless repo migrations. With that parameter, go-ipfs checks if fs-repo-migrations is on PATH, and if not, downloads it:

Sounds like you had an older version on your path, which you manually downloaded in 2017, and that metadata caused macOS Catalina to show this error. (macOS started requiring notarization recently for things downloaded with a web browser, but using wget or curl should be fine)

Indeed

xattr -v $(which fs-repo-migrations)                                                                                                         
/Users/gozala/.local/bin/fs-repo-migrations: com.apple.quarantine

@Gozala I believe the fix here is to remove old fs-repo-migrations – can you confirm if it removed the error?

I can't confirm it fixes that as that error has not occurred ever since that last time. I assume macOS prompts user with that message only once & fails silently on subsequent runs.

@lidel
Copy link
Member

lidel commented May 5, 2020

Ok, I was not able to reproduce with "implicit" repo migration. I believe it is safe to say this does not impact regular users of ipfs-desktop, nor is caused by the app itself – closing.

(macOS notarization of command line tools at dist.ipfs.io needs to be solved upstream)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment