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

[FEATURE] Fingerprint migration #49

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

felixranesberger
Copy link
Contributor

No description provided.

@felixranesberger felixranesberger added the enhancement New feature or request label Apr 29, 2024
@einpraegsam
Copy link
Contributor

Thx Felix, I'm going to test this issue as soon as possible :)

@einpraegsam
Copy link
Contributor

einpraegsam commented May 1, 2024

Thx Felix, I'm just playing a bit with the new library. In my case fingerprint values are calculated with this properties:

{
  "audio": {
    "sampleHash": 1019.0094796679914,
    "oscillator": "sine",
    "maxChannels": 1,
    "channelCountMode": "max"
  },
  "hardware": {
    "videocard": {
      "vendor": "Mozilla",
      "vendorUnmasked": "Intel",
      "renderer": "Intel(R) HD Graphics, or similar",
      "rendererUnmasked": "Intel(R) HD Graphics, or similar",
      "version": "WebGL 1.0",
      "shadingLanguageVersion": "WebGL GLSL ES 1.0"
    },
    "architecture": 255,
    "deviceMemory": "0",
    "jsHeapSizeLimit": 0
  },
  "locales": {
    "languages": "de-DE",
    "timezone": "Europe/Berlin"
  },
  "permissions": {
    "geolocation": "prompt",
    "midi": "prompt",
    "notifications": "prompt",
    "persistent-storage": "prompt",
    "push": "prompt",
    "storage-access": "prompt"
  },
  "plugins": {
    "plugins": [
      "PDF Viewer|internal-pdf-viewer|Portable Document Format",
      "Chrome PDF Viewer|internal-pdf-viewer|Portable Document Format",
      "Chromium PDF Viewer|internal-pdf-viewer|Portable Document Format",
      "Microsoft Edge PDF Viewer|internal-pdf-viewer|Portable Document Format",
      "WebKit built-in PDF|internal-pdf-viewer|Portable Document Format"
    ]
  },
  "screen": {
    "is_touchscreen": false,
    "maxTouchPoints": 0,
    "colorDepth": 24,
    "mediaMatches": [
      "prefers-contrast: no-preference",
      "any-hover: hover",
      "any-pointer: fine",
      "pointer: fine",
      "hover: hover",
      "update: fast",
      "prefers-reduced-motion: no-preference",
      "scripting: enabled",
      "forced-colors: none"
    ]
  },
  "system": {
    "platform": "Linux x86_64",
    "cookieEnabled": true,
    "productSub": "20100101",
    "product": "Gecko",
    "useragent": "Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:123.0) Gecko/20100101 Firefox/123.0",
    "browser": {
      "name": "Firefox",
      "version": "123.0"
    },
    "applePayVersion": 0
  },
  "webgl": {
    "commonImageHash": "lyda82"
  },
  "math": {
    "acos": 1.0471975511965979,
    "asin": -9.614302481290016e-17,
    "atan": 4.578239276804769e-17,
    "cos": -4.854249971455313e-16,
    "cosh": 1.9468519159297506,
    "e": 2.718281828459045,
    "largeCos": 0.7639704044417283,
    "largeSin": -0.6452512852657808,
    "largeTan": -0.8446024630198843,
    "log": 6.907755278982137,
    "pi": 3.141592653589793,
    "sin": -1.9461946644816207e-16,
    "sinh": -0.6288121810679035,
    "sqrt": 1.4142135623730951,
    "tan": 6.980860926542689e-14,
    "tanh": -0.39008295789884684
  }
}

In FireFox one single warning is still listed:
WEBGL_debug_renderer_info is deprecated in Firefox and will be removed. Please use RENDERER.

to get a string with 32 characters as before
@einpraegsam
Copy link
Contributor

einpraegsam commented May 1, 2024

@felixranesberger I just pushed a small update with a small fix. In addition a md5 function is used for hashing the short string (e.g. "17gwjmp") to 32 character lenghts again (because this is an important part in backend functionality).
Internal note: https://www.thumbmarkjs.com/ is the testing page

@felixranesberger
Copy link
Contributor Author

I thought using setOption to exclude WEBGL would be work :(

setOption('exclude', ['webgl', 'system.browser.version]'])

Maybe I'm using the function incorrectly. I'll have a look tomorrow.

@felixranesberger
Copy link
Contributor Author

In FireFox one single warning is still listed:
WEBGL_debug_renderer_info is deprecated in Firefox and will be removed. Please use RENDERER.

I found this issue where they discuss this warning.
After reading the answer, I think we can actually ignore the error:
thumbmarkjs/thumbmarkjs#47

@felixranesberger
Copy link
Contributor Author

Regarding the md5 hashing: can we hash the value on the serverside?
This would probably improve the client side performance and Google Pagespeed, since we don't hash directly on the client.
Lux currently has some issues with Pagespeed warnings of long main threads, this could be (one) of the underlying issues behind the warning.

@einpraegsam
Copy link
Contributor

einpraegsam commented May 2, 2024

Regarding the md5 hashing: can we hash the value on the serverside? This would probably improve the client side performance and Google Pagespeed, since we don't hash directly on the client. Lux currently has some issues with Pagespeed warnings of long main threads, this could be (one) of the underlying issues behind the warning.

Of course we could but this is a bigger change that you might guess. At the moment it's kind of hardcoded that 32 character strings are fingerprints while 33 character strings are local storage values.
What about forget the md5 library and simply add zeros at the beginning via JS? "17gwjmp" could be changed in JS to "000000000000000000000000017gwjmp"? That would also do the trick (we don't need a second hashing).

In general: I think the thumbmarkjs library is a good solution and we should go for it. I will discuss this with @lefloe in the next LUX meeting

Thank you very much so far

@felixranesberger
Copy link
Contributor Author

What about forget the md5 library and simply add zeros at the beginning via JS?

I think this is a pretty good solution.
My main concern is just client performance and this would be easily fixed by your suggestion without breaking current functionality. I will implement the changes.

@felixranesberger
Copy link
Contributor Author

With the new commit I added this line:

const computedFingerprint = `${'0'.repeat(32 - fingerprint.length)}${fingerprint}`;

It makes sure to fill the remaining space to 32 character length with zeros.
This is important when the fingerprint hash length would change in the future without us knowing it.

@felixranesberger felixranesberger changed the title Draft: [FEATURE] Fingerprint migration [FEATURE] Fingerprint migration May 2, 2024
@felixranesberger felixranesberger self-assigned this May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants