View Issue Details

IDProjectCategoryView StatusLast Update
0006042Talerwallet (WebExtension)public2021-08-24 16:23
ReporterChristian Grothoff Assigned Tosebasjm  
PriorityhighSeveritytweakReproducibilityN/A
Status closedResolutionfixed 
Platformi7OSDebian GNU/LinuxOS Versionsqueeze
Product Versiongit (master) 
Target Version0.8Fixed in Version0.8 
Summary0006042: test webextensions wallet on GNU IceCat
DescriptionWe 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.
TagsNo tags attached.

Relationships

related to 0006043 closedbuckE test webextensions wallet on Brave 

Activities

gmarcos87

2020-01-14 17:20

reporter   ~0015268

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.

Florian Dold

2020-01-14 23:26

manager   ~0015270

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?

gmarcos87

2020-01-15 12:36

reporter   ~0015271

Last edited: 2020-01-15 12:49

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.

Florian Dold

2020-01-15 12:52

manager   ~0015272

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!

gmarcos87

2020-01-15 16:20

reporter   ~0015273

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.

buckE

2020-04-04 03:23

reporter   ~0015518

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?

buckE

2020-04-04 03:34

reporter   ~0015519

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."

buckE

2020-04-04 04:29

reporter   ~0015521

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.

Christian Grothoff

2020-04-04 10:21

manager   ~0015524

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.

Christian Grothoff

2020-04-04 10:26

manager   ~0015525

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.

buckE

2020-04-07 06:24

reporter   ~0015541

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.

Florian Dold

2020-04-07 08:30

manager   ~0015544

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.

Christian Grothoff

2020-04-07 10:13

manager   ~0015546

Currency-symbol for kudos: ク (ku in Katakana).

buckE

2020-04-08 10:01

reporter   ~0015570

> 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.

Florian Dold

2020-04-08 11:04

manager   ~0015573

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.

buckE

2020-04-20 11:12

reporter   ~0015704

> 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?

buckE

2020-04-30 10:01

reporter   ~0015814

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.

sebasjm

2021-06-21 16:10

developer   ~0017974

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

sebasjm

2021-07-28 21:09

developer   ~0018020

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.

Issue History

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)