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

Safari Support #995

Closed
lightninglu10 opened this Issue Sep 5, 2017 · 9 comments

Comments

Projects
None yet
5 participants
@lightninglu10

lightninglu10 commented Sep 5, 2017

  • Version: 0.25.0
  • Platform: JS
  • Subsystem:

Type: Bug

Severity: High

Description: IPFS does not work in Safari. It looks like something dealing with the webcrypto package here: libp2p/js-libp2p-crypto#18

Getting an error in Safari that

alg — ipfs.min.js:42864SyntaxError: Bad algorithm name

Is there anyway to remedy this?

@lightninglu10

This comment has been minimized.

Show comment
Hide comment
@lightninglu10

lightninglu10 Sep 6, 2017

Tracked it down to an algorithm named ECDH in this part of the code:

function alg(a) {
            var r = {
                name: (a.name || a || "").toUpperCase().replace("V", "v")
            };
            switch (r.name) {
            case "SHA-1":
            case "SHA-256":
            case "SHA-384":
            case "SHA-512":
                break;
            case "AES-CBC":
            case "AES-GCM":
            case "AES-KW":
                a.length && (r.length = a.length);
                break;
            case "HMAC":
                a.hash && (r.hash = alg(a.hash)), a.length && (r.length = a.length);
                break;
            case "RSAES-PKCS1-v1_5":
                a.publicExponent && (r.publicExponent = new Uint8Array(a.publicExponent)), a.modulusLength && (r.modulusLength = a.modulusLength);
                break;
            case "RSASSA-PKCS1-v1_5":
            case "RSA-OAEP":
                a.hash && (r.hash = alg(a.hash)), a.publicExponent && (r.publicExponent = new Uint8Array(a.publicExponent)), a.modulusLength && (r.modulusLength = a.modulusLength);
                break;
            default:
                throw new SyntaxError("Bad algorithm name")
            }
            return r
        }

lightninglu10 commented Sep 6, 2017

Tracked it down to an algorithm named ECDH in this part of the code:

function alg(a) {
            var r = {
                name: (a.name || a || "").toUpperCase().replace("V", "v")
            };
            switch (r.name) {
            case "SHA-1":
            case "SHA-256":
            case "SHA-384":
            case "SHA-512":
                break;
            case "AES-CBC":
            case "AES-GCM":
            case "AES-KW":
                a.length && (r.length = a.length);
                break;
            case "HMAC":
                a.hash && (r.hash = alg(a.hash)), a.length && (r.length = a.length);
                break;
            case "RSAES-PKCS1-v1_5":
                a.publicExponent && (r.publicExponent = new Uint8Array(a.publicExponent)), a.modulusLength && (r.modulusLength = a.modulusLength);
                break;
            case "RSASSA-PKCS1-v1_5":
            case "RSA-OAEP":
                a.hash && (r.hash = alg(a.hash)), a.publicExponent && (r.publicExponent = new Uint8Array(a.publicExponent)), a.modulusLength && (r.modulusLength = a.modulusLength);
                break;
            default:
                throw new SyntaxError("Bad algorithm name")
            }
            return r
        }
@diasdavid

This comment has been minimized.

Show comment
Hide comment
@diasdavid

diasdavid Sep 6, 2017

Member

Thanks for reporting this @lightninglu10. It is a known issue for a while as we focused on developing for Chrome and Firefox (due to their WebRTC support). See compat table here: libp2p/js-libp2p-crypto#18

This means that missing alg need to be shimmed. Is this something you would like to help with?

Member

diasdavid commented Sep 6, 2017

Thanks for reporting this @lightninglu10. It is a known issue for a while as we focused on developing for Chrome and Firefox (due to their WebRTC support). See compat table here: libp2p/js-libp2p-crypto#18

This means that missing alg need to be shimmed. Is this something you would like to help with?

@diasdavid diasdavid changed the title from SyntaxError: Bad algorithm name -- IPFS not working in Safari to Safari Support Sep 8, 2017

@diasdavid diasdavid added ready and removed bug labels Sep 8, 2017

@bertrandrustle

This comment has been minimized.

Show comment
Hide comment
@bertrandrustle

bertrandrustle Oct 6, 2017

A major version of Safari (11.0) was released a couple of weeks ago. The WebCrypto API section of the release notes is a little vague and the JS API docs are even less informative, but the release appears to include recent commits to WebKit adding support for the remaining WebCrypto spec ciphers mentioned in libp2p/js-libp2p-crypto#18. See:

https://bugs.webkit.org/show_bug.cgi?id=166746
https://bugs.webkit.org/show_bug.cgi?id=160880
https://trac.webkit.org/browser/webkit/releases/Apple/Safari%2011.0/WebCore/crypto/algorithms

bertrandrustle commented Oct 6, 2017

A major version of Safari (11.0) was released a couple of weeks ago. The WebCrypto API section of the release notes is a little vague and the JS API docs are even less informative, but the release appears to include recent commits to WebKit adding support for the remaining WebCrypto spec ciphers mentioned in libp2p/js-libp2p-crypto#18. See:

https://bugs.webkit.org/show_bug.cgi?id=166746
https://bugs.webkit.org/show_bug.cgi?id=160880
https://trac.webkit.org/browser/webkit/releases/Apple/Safari%2011.0/WebCore/crypto/algorithms

@diasdavid

This comment has been minimized.

Show comment
Hide comment
@diasdavid

diasdavid Oct 9, 2017

Member

Do we have a volunteer to go and test these new grounds? Thank you for the update, @bertrandrustle

Member

diasdavid commented Oct 9, 2017

Do we have a volunteer to go and test these new grounds? Thank you for the update, @bertrandrustle

@bertrandrustle

This comment has been minimized.

Show comment
Hide comment
@bertrandrustle

bertrandrustle Oct 9, 2017

Here's an exhaustive browser test of the WebCrypto API:
https://diafygi.github.io/webcrypto-examples/

Source code:
https://github.com/diafygi/webcrypto-examples/

When Safari is detected the test shims in Safari's soon-to-be-removed crypto.webKitSubtle JS interface. These are the results using Safari 11:

webcryptoapi live table

So that interface already seems to support everything IPFS needs. After modifying the test to make Safari use the standard crypto.subtle interface instead, the results with Safari 11 are identical (except now there's no warning in the console about the deprecated crypto.webKitSubtle):

webcryptoapi live table - crypto subtle

bertrandrustle commented Oct 9, 2017

Here's an exhaustive browser test of the WebCrypto API:
https://diafygi.github.io/webcrypto-examples/

Source code:
https://github.com/diafygi/webcrypto-examples/

When Safari is detected the test shims in Safari's soon-to-be-removed crypto.webKitSubtle JS interface. These are the results using Safari 11:

webcryptoapi live table

So that interface already seems to support everything IPFS needs. After modifying the test to make Safari use the standard crypto.subtle interface instead, the results with Safari 11 are identical (except now there's no warning in the console about the deprecated crypto.webKitSubtle):

webcryptoapi live table - crypto subtle

@VictorBjelkholm

This comment has been minimized.

Show comment
Hide comment
@VictorBjelkholm

VictorBjelkholm Oct 9, 2017

Member

Got × Unhandled Rejection (TypeError): Member JsonWebKey.kty is required and must be an instance of DOMString from webcrypto-shim which there is a open issue for (vibornoff/webcrypto-shim#22).

However, there is a merged PR with a working commit though, vibornoff/webcrypto-shim@ae3e9fa made a PR to our fork that fixes it and now it works 🎉

dignifiedquire/webcrypto-shim#2

screen shot 2017-10-09 at 18 46 18

Member

VictorBjelkholm commented Oct 9, 2017

Got × Unhandled Rejection (TypeError): Member JsonWebKey.kty is required and must be an instance of DOMString from webcrypto-shim which there is a open issue for (vibornoff/webcrypto-shim#22).

However, there is a merged PR with a working commit though, vibornoff/webcrypto-shim@ae3e9fa made a PR to our fork that fixes it and now it works 🎉

dignifiedquire/webcrypto-shim#2

screen shot 2017-10-09 at 18 46 18

@VictorBjelkholm

This comment has been minimized.

Show comment
Hide comment
@VictorBjelkholm

VictorBjelkholm Oct 10, 2017

Member

PR linked in previous commit has been merged and libp2p-crypto module points to github:dignifiedquire/webcrypto-shim#master so no more changes should be required (just another npm install), js-ipfs should work on Safari 11 now!

Member

VictorBjelkholm commented Oct 10, 2017

PR linked in previous commit has been merged and libp2p-crypto module points to github:dignifiedquire/webcrypto-shim#master so no more changes should be required (just another npm install), js-ipfs should work on Safari 11 now!

@diasdavid

This comment has been minimized.

Show comment
Hide comment
@diasdavid

diasdavid Nov 3, 2017

Member

Safari support is here! 🚀

image

image

Member

diasdavid commented Nov 3, 2017

Safari support is here! 🚀

image

image

@alexsicart

This comment has been minimized.

Show comment
Hide comment
@alexsicart

alexsicart Nov 3, 2017

Ohh coool thanks @diasdavid !!

alexsicart commented Nov 3, 2017

Ohh coool thanks @diasdavid !!

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