lnbits / withdraw

LNbits Withdraw Extension
MIT License
6 stars 9 forks source link

Update crud.py #9

Closed Liongrass closed 7 months ago

Liongrass commented 1 year ago

In LNbits, each lnurlw is retrievable through a url as identified by its short hash (e.g. https://legend.lnbits.com/withdraw/X5iYZN).

With only six digits this url is probably far from "safe" and only allows about 38 billion possible combinations. I propose to revert to the previous 22 digits to make it more difficult to brute-force all possible combinations and retrieve all lnurlws from the server.

dni commented 1 year ago

hm, it still seems pretty hard, especially with the addition of the built-in rate limiter. the reason why we shortened it, was to safe some bytes for the encoded LNURL to be able to fit better on nfc tags.

what kind of length would you think is reasonable?

@callebtc

prusnak commented 1 year ago

the reason why we shortened it, was to safe some bytes for the encoded LNURL to be able to fit better on nfc tags.

What's the capacity of the lamest NFC tag? Isn't it 128 bytes?

dni commented 1 year ago

i scanned my china ring and it has 180 bytes

prusnak commented 1 year ago

According to https://www.shopnfc.com/en/content/6-nfc-tags-specs the smallest capacities are:

So I think the question boils down to: do we want to reasonably support the really tiny NFC tags (MIFARE Ultralight and NTAG210) with only 46/48 bytes of storage?

If yes, than staying with 6 uuid chars (~35 bits of entropy) makes sense, although uuid 8 or 10 chars (~46 or ~58 bits of entropy) would provide better security at the expense of supporting domains names that are 2 or 4 characters shorter.

If we don't care about these, we might jump to 22 chars of uuid which translates to ~128 bits of entropy.

arbadacarbaYK commented 1 year ago

Swapping security over a cheap china ring actually doesnt sound reasonable to me tbh, NTAG215 is a long communicated minimum for lnurlw.. Apart from that LNURLw is just for vouchers and one-time usage, smth like a ring should be done with the boltcard-service so NTAG424

Liongrass commented 1 year ago

As I understand this line, it does not change the length of the actual LNURLw. It only changes the length of the sharelink.

Here is an example of a sharelink: https://legend.lnbits.com/withdraw/X5iYZN This is the corresponding LNURLw: LNURL1DP68GURN8GHJ7MR9VAJKUEPWD3HXY6T5WVHXXMMD9AMKJARGV3EXZAE0V9CXJTMKXYHKCMN4WFKZ7C2EV9556A23VAMKKWT0DEHRS6NYDAHX74ZY9UMHQ462VAFYSC3KTGUHXNZ5FDZXWUMGXEMH2X7HHP6 Which decodes to https://legend.lnbits.com/withdraw/api/v1/lnurl/aYaiMuQgwk9onn8jdonoTD/7pWJgRHb6Z9sLTKDgsh6wu

I don't see any benefit of having a six-character sharelink, and you're hopefully not loading the sharelink onto your NFC tags, but the LNURLw instead. This PR shouldn't change the length/weight of a LNURLw, which is about 160B, with or without that PR.

Maybe from a security aspect a six-character sharelink is somewhat acceptable with rate limiting, but there's also the birthday paradox. With only 38 billion possible permutations you run into a 50% of having two LNURLw with the same ID/sharelink after creating only 229,727 LNURLw! And then what?

dni commented 1 year ago

aah. you are absolutly right :). its only the shareable link. hm. we should shortened the lnurl links aswell :).

i agree with pavol i would go for 8 or 10. i see no need for 22.

i think the goal is to get that as low as reasonably possible without compromising security. there is no reason to waste bits. :)

Liongrass commented 1 year ago

Maybe for clarification, could you elaborate what this sharelink is meant to be used for? Why is the length of this sharelink important? Why not shorten the word withdraw to w instead to save 8 bytes? Or why not use a URL shortener instead of messing with the LNURL extension?

Liongrass commented 11 months ago

Any updates on this? I don't see the reason or justification for keeping the withdrawal link so dangerously short.

prusnak commented 11 months ago

My take is let's go for 22, so ACK for the proposed change

Liongrass commented 9 months ago

Any updates on this?