Giter Club home page Giter Club logo

Comments (9)

vhscom avatar vhscom commented on May 20, 2024 2

I adapted this work to run offline: https://www.npmjs.com/package/oathqr

from 2fa-qr.

stefansundin avatar stefansundin commented on May 20, 2024 1

Note I built my work using yours as a source of design inspiration to create a transformative work that runs offline. If you look closely at the source you will see there are no substantial parts of your code in my work.

While I get this sentiment, it is not entirely true. The user interface is exactly the same, most of the strings are copied word by word. The presentation and the way it functions is a substantial part of the work. But fair enough.

if you wish to challenge my statements

I just wanted to help correct some things that I thought were incorrect. There is no need to blow this out of proportions. Please take a deep breath.

take it up with GH legal team

GitHub has nothing to do with this. They would just tell you to go to the courts. I don't want to do that and have no intention of doing that. There is no need to do that. We're adults and we can have a discussion about this without the need to escalate things.

I do not make claims about security on the Internet.

By writing that there was a vulnerability in the original work, you kinda did.


You may have read my post in a more negative tone than I wrote it in. I am very happy that you decided to share your work, and I think everyone who reads this would agree with me that it makes the world a better place to have more implementations of this idea. All I wanted to do was to improve your work by pointing out some things. You should be happy about that. There is no need to get upset.

Anyway, please have a good rest of your day.

from 2fa-qr.

stefansundin avatar stefansundin commented on May 20, 2024

Hey.

Yes, it is true that everything is done in the browser and nothing is sent over the network. One reason why I haven't tried to make a statement about it on the website is that a statement might make have the opposite effect if you are not technical enough to verify the claim yourself.

I have tried to make it easy to verify yourself with the following design choices:

  1. Use as few dependencies as possible. The website uses jquery and jquery-qrcode. I will soon replace jquery-qrcode with kjua (both made by the same author), and remove jquery altogether.
  2. Load the dependencies from CDNs including subresource integrity (using the integrity attribute), which ensures that I haven't tampered with them. This would be the best place to hide some obfuscated javascript to send things to a third party server.

    2fa-qr/index.html

    Lines 17 to 18 in 4156199

    <script src="https://code.jquery.com/jquery-3.6.0.slim.min.js" integrity="sha256-u7e5khyithlIdTpu22PHhENmPcRdFiHRjhAuHcs05RI=" crossorigin="anonymous"></script>
    <script src="https://cdn.jsdelivr.net/gh/lrsjng/[email protected]/dist/jquery-qrcode.min.js" integrity="sha384-W+CodFEp2EiGOM49HsUJwCpf58Vkgsx5CtgCl6bQJiAfGb+ndvp/fE8dZtI9fnUD" crossorigin="anonymous"></script>
  3. Put everything in a single file to make it easy to inspect. It is ~150 lines of javascript inline in the file.

So I would encourage you to inspect the source code and make a determination yourself. Perhaps we can use this issue as a thread where anyone can report their own conclusion on the safety of 2fa-qr?

Anyone should feel free to do this themselves and post a comment here that says something like the following:

I have inspected the source code of this project (commit 4156199) and in my opinion it is safe.

Of course if you don't think it is safe or want to add other information, please do so. It is just an idea. But be sure to include the commit hash since I will be pushing more changes to the repo.

Finally, even if this project was sending data to another server, it would only have your second factor and not your primary credentials (username and password). The project also aims to be a bit educational by demystifying the QR code data. So for those that want to be extra safe, you can generate your QR codes without a web browser, e.g. using the qrencode cli program.

# macos example
brew install qrencode
qrencode -o qr.png otpauth://totp/me%40example.com?secret=MYSECRET

And then you open qr.png in another program and scan the code.

Another idea is to load the page and then go offline and generate your QR codes. But this is not guaranteed to work since the script could be storing data in localStorage and then send it later when you reconnect to the internet.

I would recommend that people start using a password manager that has TOTP support. My personal recommendation is KeePassXC. Because of this, I haven't actually used 2fa-qr in years, but I think it is a very neat little helper still so I have kept maintaining it and recently I even cleaned up the code a lot. I am planning on adding one more feature to it soon too.

Anyway.. best thing is to try to verify the safety yourself. And if you do then feel free to sign off on it like I suggested above. Maybe get some other people to also check it out and have them sign off too. Thanks for kicking that off, I guess. :)

from 2fa-qr.

stefansundin avatar stefansundin commented on May 20, 2024

Another point that I forgot to mention is that the page has a Content-Security-Policy meta tag that lists all of the remote servers that the page is allowed to talk to. Since this project is hosted on GitHub Pages, it is not possible to use a CSP header, but the meta tag is better than nothing. More info: https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP

<meta http-equiv="Content-Security-Policy" content="default-src 'none'; script-src 'unsafe-inline' https://code.jquery.com https://cdn.jsdelivr.net; style-src 'unsafe-inline' https://cdn.jsdelivr.net https://fonts.googleapis.com; font-src https://fonts.gstatic.com; img-src data:">

So if this page was sending your credentials somewhere, it would have to be to one of the domains listed in this tag. I will soon remove jquery so that will reduce the list a little bit. I think the only other way to reduce the list would to remove the Roboto font which is only used for the "In the Google Authenticator app, it will look something like this:" preview.

You can try this by downloading the file yourself and then trying to add an <img> tag that loads an image from another domain. Your web browser should refuse to load it (and print an error in the developer console).

from 2fa-qr.

pgorod avatar pgorod commented on May 20, 2024

Thanks for the write-up, this is all quite interesting.

I think you should put this (or with a slight rewrite) on the project's README, so it appears on the front page.

I like the way you approach the discussion... make no claim, let people check things and decide.

So I could see a README like this:

Things you need to check to ensure this script doesn't save any data, doesn't store anything, and isn't being intercepted anywhere:

1. check the 150 lines of the single file js

2. check the 2 dependencies

3. Check the CDNs serving those dependencies


You can use your text from above to provide those 3 explanations.

Then on the site, add the notice in the same tone: things you need to check to ensure etc., linking to the README.

This way, without making any claim, you're telling people what they need to know, and how to check if they're paranoid. What do you think?

from 2fa-qr.

stefansundin avatar stefansundin commented on May 20, 2024

Hi @vhscom. Thanks for sharing your fork. I feel like I must point out a few things though.

You state:

  1. Fixes XSS vulnerability due to unsafe CSP in original.
  2. Makes no external requests for dependencies at runtime.
  3. QR code containing secret cannot be saved to device.
  4. Uses a native font stack and doesn't rely on Google.
  5. Missing ability to paste custom otpauth URIs directly.
  1. There is no known XSS vulnerability in 2fa-qr, so that bullet point is definitely misleading. You can perhaps say that you "Hardened" the CSP instead?
  2. There are no external requests during runtime, only when the page loads. Once the page has loaded you can disconnect from the internet and everything works just fine.
  3. What does this mean?
  4. The page does not rely on Google. If the font cannot be loaded then the app still works.
  5. You can edit the input field. Not sure what you mean here.

You have added a copyright statement in your README. GPLv3 derivatives require attribution so you are required to have a line there with my name. And in the page you replaced "Made by Stefan Sundin" with "Made by VHS", and I would again insist that you keep my name there.

It would have been nice if you had continued your git commits on top of mine. When I push future updates to this repository then people are not able to see where you forked off from.

Finally, you claim that the fork is somehow more secure? No one can really audit the minified JavaScript that your version compiles everything to. And if someone decides to run the development version, it is very difficult to audit all of the dependencies (the lock file is over 2000 lines long). Pulling in hundreds of dependencies introduces a huge amount of possible supply chain attacks. I would argue that this is an impossible environment to guarantee anything in. Even you would have a hard time accounting for every line of code that runs in the users web browser in the end. At the very least I would recommend that you disable all minification and obfuscation in the build.

from 2fa-qr.

vhscom avatar vhscom commented on May 20, 2024

Hi Stefan. Thanks for reviewing the README. I've updated it based on your feedback. I believe you will find the adjustments to your liking. I don't want to nitpick too much over specific verbiage so I just did a slash and burn on the stuff you may have found offensive.

Note I built my work using yours as a source of design inspiration to create a transformative work that runs offline. If you look closely at the source you will see there are no substantial parts of your code in my work. In other words, it's not a fork. So suggesting I "replaced" your name is a misleading statement at its best. That would be like suggesting a complete rewrite of TodoMVC written in Vue was somehow the copyright of an earlier version written in React.

However, because I want you to continue writing GPL work have a nice day here's my offer... Because I may have incorrectly described my app as my derivative (or "modified" work using GPL vernacular) work, if you wish to challenge my statements above I can make a mirror of my work on GitHub and you can take it up with GH legal team. I'm actually genuinely curious where they might draw the line. If they decide after I rebuttle my work infringes on yours, I will add your name a second time, this time with with a copyright line, to my README.

Finally, you claim that the fork is somehow more secure?

I do not make claims about security on the Internet.

from 2fa-qr.

vhscom avatar vhscom commented on May 20, 2024

You brought up some good points and I value that. I made you an offer to put you in control as I disagree with your statement regarding your copyright over a layout and a couple snippets of text. What you choose to do with that is up to you. Cheers and thanks again for the inspiration.

from 2fa-qr.

stefansundin avatar stefansundin commented on May 20, 2024

Ok, you need to stop deleting and resubmitting your post. It's making my email notifications sad. Use the edit feature.

I don't really care about your project any more. Let's just stop this discussion here. I'm more concerned now that this discussion issue has been completely derailed and I better just close it.

If anyone wants to continue the discussion on the original topic, please open a new issue instead so we can start from a blank slate.

And if someone wants to advertise their transformative work here, then please don't hijack existing topics.

from 2fa-qr.

Related Issues (2)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.