View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0006042 | Taler | wallet (WebExtension) | public | 2020-01-12 22:28 | 2021-08-24 16:23 |
Reporter | Christian Grothoff | Assigned To | sebasjm | ||
Priority | high | Severity | tweak | Reproducibility | N/A |
Status | closed | Resolution | fixed | ||
Platform | i7 | OS | Debian GNU/Linux | OS Version | squeeze |
Product Version | git (master) | ||||
Target Version | 0.8 | Fixed in Version | 0.8 | ||
Summary | 0006042: test webextensions wallet on GNU IceCat | ||||
Description | We should test the Webex wallet on IceCat, fix it if it is broken, and if it works list GNU IceCat on the wallet Website. Requested by RMS. Marcos should do this. | ||||
Tags | No tags attached. | ||||
|
After debugging the serviceworker I found that the problem with the Taler web extension is that it uses BigInt, which is not available in the current version of IceCat. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt#Browser_compatibility There may be other probelamas, but this is the first blocker I found. |
|
The wallet uses the big-integer package (https://www.npmjs.com/package/big-integer), which should only use native BigInt in the environments that support it, and fall back to a custom implementation of big integers on older browsers. Could you please check where exactly the native BigInt is used and why the big-integer library doesn't fall back to its custom implementation? |
|
BigInt is used in wallet-core/src/crypto/workers/cryptoImplementation.ts (amountToBuffer and timestampToBuffer). The funny thing is that the lines used seem to affect what the functions return, but it does! If I remove these lines the signature validation process fails. These two functions also uses DataView.prototype.setBigUint64() which has existed since Firefox version 68 and has no external library to replace it. |
|
I don't quite understand what you wrote. Of course these functions all affect signature generation and verification! What is surprising (and what I would like you to debug) is why the big-integer library (used in the functions you were referring to above) tries to use the native JavaScript big integer, even though the browser doesn't support it. The big-integer *should* have a fallback implementation for older browsers, and we need to find out why it is not used! |
|
The library is the one that is not being used in that file. It's called the native method, the big-integer package is not even imported at https://git.taler.net/wallet-core.git/tree/src/crypto/workers/cryptoImplementation.ts I can do some debugging but solving this part of the implementation is beyond my current understanding of the code. |
|
Note to self: in an e-mail Christian says: Ok, maybe you can take care of 0006043/0006042 and _really_ test the various features of the demo with Brave and IceCat and _also_ look at the console (F12) to see if any issues/failures are reported there while you do it? |
|
Firefox Extension downloads but failed on initial install with error: GNU Taler Wallet could not be installed because it is not compatible with IceCat 60.7.0. F12 (Console and Debugger) show blank screens. No further testing done at this time. My first plan will be to look at the code to see if it is expecting a specific user agent string with the word "Firefox." |
|
Maybe? src/webex/compat.ts Line 22: export function isFirefox(): boolean { const rt = chrome.runtime as any; if (typeof rt.getBrowserInfo === "function") { return true; } return false; } I tried editing this and rebuilding the add-on but the INSTALL file in wallet-core reads "Run `./configure && make' to create an archive containing the extension in ./build/taler-wallet-$VERSION.zip" However `./configure` does not exist and this was not my highest priority at this time. |
|
I'm pretty sure we're not checking for "Firefox", but we do have a version requirement (but it says 57+ on the Web site). Not sure 57+ is still accurate though. |
|
Anyway, with a sufficiently recent IceCat, the issue _should_ be the BigInt stuff. It could be that the BigInt issue is explicitly detected now, or that IceCat 60 is 'too low' because of BitInt, not sure. The real question is why the BigInt library doesn't fall back to a software implementation as it should when the Browser doesn't support it natively. So debugging that _should_ be the hard part here. |
|
Florian: I don't know much about javascript but why are you surprised that "the big-integer library (used in the functions you were referring to above) tries to use the native JavaScript big integer, even though the browser doesn't support it?" It seems to me (knowing nothing) that one of these must be true: 1 - BigInt is a generic term, so a browser will treat it however its libraries tell it to. 2 - BigInt is a specific term, so any browser that does not support it will break unless you have some sort of `case` statement that includes specific alternatives. But even with a case statement, this might something that is really either in or out. Because the workaround to generate a number larger than Number supports depends on the specific use case (do you use a String, or Number.MAX_SAFE_INTEGER, etc.?) I think the situation is closer to (2) so my question now is, maybe the choice is between (a) using numbers supported by BigInt, or (b) supporting IceCat, or (c) a substantial rewrite that uses some workaround. I do not see any reason to expect graceful fallback? IF this analysis is correct (and it may not be) then the next question is: does IceCat have a plan to support the BigInt function? If it is as useful for strong crypto as I think then it may be on the release map. |
|
The big-integer library is supposed to detect whether the browser natively supports big integers. Big integers were introduced into browser only relatively recently. If the browser *doesn't* support big integers natively, the library has a fallback implementation. See this table (that unfortunately doesn't include icecat directly): https://caniuse.com/#feat=bigint Maybe the code in big-integer that detects native support doesn't work correctly. Or it specifically doesn't detect it correctly on icecat. I do think that icecat will eventually support is, once it merges in the latest gecko / other components from Firefox. |
|
Currency-symbol for kudos: ク (ku in Katakana). |
|
> The big-integer library is supposed to detect whether the browser natively supports big integers. Big integers were introduced into browser only relatively recently. If the browser *doesn't* support big integers natively, the library has a fallback implementation. So a call to BigInt will read a library that has another set of instructions to run instead, after sensing if the browser has this support. That seems weird to me, because it is very complicated to manually write an alternative to a BigInt function(I think?), so it seems very difficult to automate a fallback. (It is not a set procedure; BigInt solves a number of different approaches, not simply replaces one other method.) > I do think that icecat will eventually support is, once it merges in the latest gecko / other components from Firefox. In that case, my little recommendation is I think we should put this issue on hold until BigInt support is merged. We have a reason to use BigInt, and (I assume?) we have other priorities than learning why BigInt does not do what you expect. If RMS requests this maybe we can ask him what the timetable is for this merging? (And if it is a long time, then maybe this becomes higher priority.) But if you want me to investigate, and you have no more experienced js devs to do it, I will. |
|
The big-integer library provides a superset of the functionality of the browser-native BigInt. Stuff like computing the GCD or modpow of big integers isn't available in the browser. Thus can't just get rid of the library, even if all browsers we use support native BigInt ;-) I think we should first find out *why* this library doesn't properly detect it's running in a browser without native BigInt support. Once some more high-priority issues are out of the way, I can look at the detection code myself. |
|
> The big-integer library provides a superset of the functionality of the browser-native BigInt Maybe I am really confused but the extension specifically uses "BigInt," not some generic big integer call. So it's not using a generic term and the browser can use the best native library available; the browser would have to know that BigInt is a modern subset of big-integer. And how would it know that if it was built before BigInt? |
|
Assigning to Florian re: we are waiting for his feedback. My synopsis: Error is due to explicit use of "BigInt," instead of a generic call that could be interpreted differently by different browsers. I know nothing, but the docs I've read do not support the statement that "The big-integer library is supposed to detect whether the browser natively supports big integers." Also I am not a javascript programmer, I am just reading the docs I find. Suggested solutions: - may be moot point if GNU Icecat and all browsers we intend to support will move to BigInt ( From https://www.npmjs.com/package/big-integer: "Update (December 2, 2018): BigInt is being added as a native feature of JavaScript.") - Or maybe we can include the library in all pages required by the wallet with <script src="https://peterolson.github.io/BigInteger.js/BigInteger.min.js"></script> I remain available to test/verify/etc. |
|
fixed in d42a7456 changed strict_min_version to 57.0 based on https://bugs.gnunet.org/view.php?id=6042#c15524 replaced native bigint usage when there is not Dataview.setBigUint64 tested on $ icecat --version GNU IceCat 60.7.0 |
|
I have been testing this for a while and seems to work as expected. I'm considering this browser ready and if any bugs appears we can work it in other ticket. |
Date Modified | Username | Field | Change |
---|---|---|---|
2020-01-12 22:28 | Christian Grothoff | New Issue | |
2020-01-12 22:28 | Christian Grothoff | Status | new => assigned |
2020-01-12 22:28 | Christian Grothoff | Assigned To | => Florian Dold |
2020-01-12 22:28 | Christian Grothoff | Assigned To | Florian Dold => |
2020-01-12 22:30 | Christian Grothoff | Relationship added | related to 0006043 |
2020-01-14 10:23 | Christian Grothoff | Assigned To | => gmarcos87 |
2020-01-14 17:20 | gmarcos87 | Note Added: 0015268 | |
2020-01-14 23:26 | Florian Dold | Note Added: 0015270 | |
2020-01-15 12:36 | gmarcos87 | Note Added: 0015271 | |
2020-01-15 12:49 | gmarcos87 | Note Edited: 0015271 | |
2020-01-15 12:52 | Florian Dold | Note Added: 0015272 | |
2020-01-15 16:20 | gmarcos87 | Note Added: 0015273 | |
2020-01-17 00:35 | Christian Grothoff | Target Version | 0.6 => 0.7.1 |
2020-03-17 02:08 | Christian Grothoff | Assigned To | gmarcos87 => |
2020-03-17 02:09 | Christian Grothoff | Status | assigned => confirmed |
2020-03-29 21:59 | Christian Grothoff | Target Version | 0.7.1 => 0.8.1 |
2020-04-02 10:00 | Christian Grothoff | Assigned To | => buckE |
2020-04-02 10:00 | Christian Grothoff | Status | confirmed => assigned |
2020-04-04 03:23 | buckE | Note Added: 0015518 | |
2020-04-04 03:34 | buckE | Note Added: 0015519 | |
2020-04-04 04:29 | buckE | Note Added: 0015521 | |
2020-04-04 10:21 | Christian Grothoff | Note Added: 0015524 | |
2020-04-04 10:26 | Christian Grothoff | Note Added: 0015525 | |
2020-04-07 06:24 | buckE | Note Added: 0015541 | |
2020-04-07 08:30 | Florian Dold | Note Added: 0015544 | |
2020-04-07 10:13 | Christian Grothoff | Note Added: 0015546 | |
2020-04-08 10:01 | buckE | Note Added: 0015570 | |
2020-04-08 11:04 | Florian Dold | Note Added: 0015573 | |
2020-04-20 11:12 | buckE | Note Added: 0015704 | |
2020-04-30 10:01 | buckE | Note Added: 0015814 | |
2020-04-30 10:01 | buckE | Assigned To | buckE => Florian Dold |
2020-07-10 23:29 | Christian Grothoff | Target Version | 0.8.1 => 0.9 |
2021-06-14 11:53 | Florian Dold | Assigned To | Florian Dold => sebasjm |
2021-06-21 16:10 | sebasjm | Note Added: 0017974 | |
2021-07-28 21:09 | sebasjm | Status | assigned => resolved |
2021-07-28 21:09 | sebasjm | Resolution | open => fixed |
2021-07-28 21:09 | sebasjm | Fixed in Version | => git (master) |
2021-07-28 21:09 | sebasjm | Note Added: 0018020 | |
2021-07-30 13:58 | Christian Grothoff | Fixed in Version | git (master) => 0.8.1 |
2021-07-30 13:59 | Christian Grothoff | Target Version | 0.9 => 0.8.1 |
2021-07-30 14:02 | Christian Grothoff | Fixed in Version | 0.8.1 => 0.8 |
2021-07-30 14:02 | Christian Grothoff | Target Version | 0.8.1 => 0.8 |
2021-08-24 16:23 | Christian Grothoff | Status | resolved => closed |
2023-04-13 20:37 | Florian Dold | Category | wallet (WebExtensions) => wallet (WebExtension) |