• TheTaytay 2 days ago

    Thank you for writing/publishing this. I especially appreciate the prominent warning at the top not to mistake it for a production library and to suggest an alternative. (It’s surprising to me how often people forget to add disclaimers like that to their code.)

    • vhsdev 2 days ago

      Appreciate it, TheTaytay!

    • usefulposter 2 days ago

      Oy.

      Who specifically is this intended for? It's a wonder that the model didn't spice things up with some tangential compliance catnip like FIPS or PCI DSS.

      I would be curious to see the prompts used to create this.

      Recently, I don't think there could be a better example of applicability of Brandolini's law.

      • amichal 2 days ago

        I would love to see alternatives of educational code that implements these things in a "compliant" way.

        Security does not come from Compliance (sometimes they are at odds) but as someone who is not an academically trained security professional but who has read NIST* in detail, implements such code and has passed a number of code reviews from security professionals. And who has been asked to do things like STRIDE risk assessment on products I write code for I do appreciate the references and links along side actual code of any kind.

        Now to be fair, I have not yet looked at any of the code here, it's commit history or its level of AI-induced fantasy confidence in the validity of the specific solutions. That could be good or bad but the intent of this is really on point for me.

        Edit: I looked at some code:

        This is missing a lot from NIST SP 800-63B

        Looking at https://github.com/vhscom/private-landing/blob/main/packages...

            - the db select runs before the password has so you can detect if the account exists with timing attacks
            - there is no enforced minimum nor maximum length on the stored secret (e..g para 5.1.1.1 and 5.1.1.2 recommend length range of 8 to 64 unicode printable chars normalized to some form i forget)
        
            - there is no enforced min max length on the account identifier (in this case email) and no normalization
        
        At least not in the code i saw. so there is still a lot of basics/low hanging fruit from NIST recommendations at least you would find in any production grade auth framework missing
        • vhsdev 2 days ago

          Hi, amichal. Nice finds. I will dig into more of the particulars where sensible. Please feel free to send up a pull request! Thanks for taking a peek.

          • tracker1 2 days ago

            On the login... when failing either via user lookup, or password mismatch, I'll usually put a random 500-2500ms (or more) delay before logging and sending the response to handle timing attacks.

            You can try a db transaction against a lock table for IP and Username as part of multi-request mitigation during any given request. CF offers Durable objects that can be used for this purpose. Return "too many requests" error if a request is sent before another is finished... this will slow things down.

            On the minimum passphrase, there are some libraries you can use to get the printable character length... note: you should always normalize (NFC or NFKC) before doing any hashing or validation.

              function getPrintableLength(str) {
                // Use Intl.Segmenter for accurate, user-perceived character count
                const segmenter = new Intl.Segmenter("en-US", { granularity: "grapheme" });
                return [...segmenter.segment(str)].length;
              }
            
            Personally, I usually just transparently set a max of 1024 bytes, I don't display a hint for it at runtime, only an error on submit though... if someone exceeds that, they deserve the generic error I return.

            Email validation can be a bit rough, depending on how permissive or restricting you want to be. If you're willing to wait for a DNS/MX check on the domain, that's a good place to start. You most likely don't want less than 5 characters or more than 100.

            • vhsdev 2 days ago

              Pretty sure all those are covered, upon more careful review. PRs open!

              Edit: The create account I hadn't thought of for the email enum. Thanks!

              Edit 2: Fixed up two schema issues identified and the last mitigated already via call: await passwords.rejectPasswordWithConstantTime(validatedData.password)

          • vhsdev 2 days ago

            Everything you or your agent need to see is in the commit history.

            • chrisweekly 2 days ago

              Brandolini's law, aka the bullshit assymetry principle: it takes way more effort to refute bs than to create it.

              FTR I'm not commenting on whether the posted project is bs, just clarifying the meaning of your last sentence.

            • Terretta 2 days ago

              > I wrote it

              The commits feel more human than usual these days, as does the timeline.

              • undefined 2 days ago
                [deleted]