4 ms·
I suspect the encryption here is broken due to CTR mode AES with IV reuse. Try printing out the IV in the segment function. It's the same value across sessions.
by throwaway81733 3y ago
I suspect the encryption here is broken due to CTR mode AES with IV reuse. Try printing out the IV in the segment function. It's the same value across sessions.
Another thing I see is that AES-CTR is being used alone, so there is no integrity guarantee for the messages.
I didn't have time to look into it any more deeply. The user interface and concept look really nice, but I would strongly recommend a cryptographic audit. In general, you shouldn't have to reach for the subtle constructs in a cryptographic library to build product features.
- ekzhang 3y agoHey, this is a serious accusation. I would be very careful about raising false suspicions like this publicly. The application does not reuse IVs. For your second point, I am aware of authenticated encryption and the possibility of bit-level tampering with commands. The point of the encryption is to make sure to preserve confidentiality here. Tampering with commands is not really part of the threat model. The server needs to be trusted in some regard. If the server were compromised and actually modified commands, and it could send arbitrary network messages, then it could just change the HTML payload to the website and show the end user something different. The point of the encryption is only to preserve confidentiality with a properly running server, so CTR mode was chosen. I appreciate your dedication to security, but I would strongly advise you do more research before making comments like this — I take security very seriously as well, and making vague, unfounded accusations without actually reading the code only serves to confuse people.
- jchook 3y agoMaybe you can provide intuition about how the IVs are not predictable? They seem to be formulaic. https://github.com/ekzhang/sshx/blob/91c82d46cde4d1ffa0ae34e2a9a49911e2e53baa/src/lib/encrypt.ts#L55 https://github.com/ekzhang/sshx/blob/91c82d46cde4d1ffa0ae34e...
- ekzhang 3y agoYou're exactly right. The IVs are completely predictable, and crucially, that allows us to guarantee that they are not reused. The security of CTR mode of encryption depends on avoiding IV reuse. Once again, I'm very happy to have this discussion at length! I just would prefer a context where people are not confused by misleading information from the start, and the readers are able to spend time thinking about it.
- pvg 3y agoHey, this is a serious accusation. I would be very careful about raising false suspicions like this publicly. It's not an 'accusation', it's feedback on your thing. It could be mistaken but it's a Show HN, the purpose is public feedback. If that sort of feedback is not what you're looking for, don't do a Show HN.
- ekzhang 3y agoOnce again, security is very tricky to get right, and I'm very transparent about the security model and encryption of the application. I've discussed this at length and have thoroughly documented the code. The commenter makes factual errors, and stating something is insecure, even on the level of suspicion, is a claim that as software developers, we take very seriously.
- pvg 3y agoIt doesn't matter if the commenter is wrong or right, you can't start writing grumpcomments with threatening auras at them. Particularly as an author in a Show HN but really anywhere on HN in general. If you think the critique is wrong, you can address that without hyperbole and without suggesting they're making accusations or need to 'be careful'. People are allowed to be simply wrong and if you think they are, you can explain that to them.
- sneak 3y agoAccusations are against people. GP criticized the code.
- biorach 3y ago> making vague, unfounded accusations without actually reading the code only serves to confuse people. It wasnt accusations, it was feedback. And it wasn't that vague. And it seems to be based on reading the code, quickly perhaps, but still. So I think it's on you to respond to the specific points in the feedback and either explain why they don't apply or accept and act on them.
- ekzhang 3y agoThanks, yes as you suggested, I already responded to their specific points and explained my rationale for the security model in detail.
- simonebrunozzi 3y agoYou are way off here. It's not a false suspicion, it's a suspicion. And it was worded very gently. Don't do a show HN if you don't want feedback like this.
- ekzhang 3y agoThe comment says that the IV is reused. The IV is not reused! I am very open to feedback, but false statements about the security of an application need to be taken very seriously, especially when given from a commenter who speaks with a tone of authority on the subject. If I am truly unwelcome to do a Show HN, please let me know — but I think I have comported myself reasonably. I'm just trying to share something of interest to people!
- simonebrunozzi 3y agoYou are certainly not unwelcome. If a comment is wrong, just say so. It's a discussion.
- egberts1 3y agoA suspicion is a non-factual feeling based on seemingly incomplete fact. To say false suspicion, is to run counter to a person's feeling. To mitigate a feeling of this (suspicion), you will need to demonstrate some (not-so-counter-but-initial-)factual presentation. Asserting a false suspicion is a blatant insult to the Aristotle logic discourse and two-debate debate exchange. You say False suspicion? We say: you prove it, don't evade it.
- ekzhang 3y agoMy concern with the comment is that it speaks from a position of authority and raises a lot of false alarms about the security of the application. The anonymous commenter publicly shared a suspicion of something that is 1) very serious and 2) false, and it essentially forces me to respond immediately. That is really stressful, especially since it's a tricky technical subject that a general audience of software developers is not expected to be familiar with. It's hard to explain why the comment is vague without giving more cryptographic background. I cannot disprove IV reuse in a short comment of this regard. It's an imperfect analogy, but imagine writing a piece of systems software and someone commenting "this leaks memory, just add a print statement to line N. I don't have time to look more." I could show them in a couple minutes why they were incorrect about line N, but "leaks memory" in general is a very vague accusation that could refer to any part of the program, as are the comments about integrity and AE and so on.
- pvg 3y agoThe anonymous commenter To most casual readers, you're a pseudonymous and unknown poster too (and that goes for nearly everybody, really). it essentially forces me to respond immediately. That is really stressful There are a lot of processes and rules to make Show HN welcoming and unstressful - it has extra guidelines, accepts a very wide variety of work in terms of quality, size, polish, etc. But it can't remove the stress of public feedback, including potentially inaccurate public feedback and you can't start going after your critics like that because of it.
- throwaway81733 3y agoI intended this to be constructive feedback and I'm sorry if it came off as otherwise. Even if the IVs themselves aren't actually problematic, I think the main point is that users of this would be interested in integrity being part of the threat model. To me, there is a tangible difference between trusting the server with complete shell access and only trusting it to deliver frontend assets that I can inspect. Additionally, using misuse-resistant cryptography constructs and libraries is a good practice to avoid some of these problems entirely. I look forward to seeing iterations, since otherwise this is very nice work.
- ekzhang 3y agoThe other comments make it fairly clear that this suspicion was unfounded but convinced people otherwise, and I have given a fair response to your comments. I of course appreciate you scrutinizing the application, and while the feedback was constructive, comments about the brokenness of a security model need to be taken very seriously. --- From the application perspective, the root of our disagreement seems to be that you would prefer using higher-level managed cryptographic primitives. I understand this criticism, but there were systems reasons why these were not appropriate for sshx. I agree with you that for most standard applications, you would avoid having to use stream ciphers directly as much as possible. sshx kind of straddles the boundary of what's technically feasible with real-time stream multiplexing and communication though. I'm glad you find the work interesting, it's something I've spent a lot of time on!
- d-z-m 3y ago> I suspect the encryption here is broken due to CTR mode AES with IV reuse. Try printing out the IV in the segment function. It's the same value across sessions. I think it's a little more subtle. IVs are being generated deterministically, but keys are generated randomly per session. The nonce uniqueness requirement in CTR is per key. That said, there doesn't seem like there's any guard against replays of messages, or dropping of messages, or altering of messages. > Another thing I see is that AES-CTR is being used alone, so there is no integrity guarantee for the messages. Indeed. They seem to try and do some integrity checking at the start of a session by sending an encrypted all-zero block, as a way of verifying that the client has the correct key (stored in the url hash). From what I can tell, there's no integrity checking on any subsequent messages. Also the KDF parameters for Argon2id seem way underpowered (19MB RAM, 1 lane, should probably be using at least 2GB RAM in a KDF setting)
- ekzhang 3y agoThanks for the comment, happy to elaborate. You're correct that the IVs are not reused. Replays and dropping messages are not possible due to the indexing of the stream. This is a necessary precaution from a distributed systems sense because networks can fail or deliver messages twice. Integrity / detecting server tampering of data is not really part of the security model here — I've discussed this in my comment below, but the aim of the encryption is to make sure to preserve confidentiality. --- Argon2id parameters are set as strong as possible while allowing for execution in a browser context.
- d-z-m 3y ago> Thanks for the comment, happy to elaborate. You're correct that the IVs are not reused. Perhaps you can elaborate a little more on this? For subsequent calls to the segment() function, holding the stream ID constant, how are IVs generated? > Replays and dropping messages are not possible due to the indexing of the stream. This is a necessary precaution from a distributed systems sense because networks can fail or deliver messages twice. I see. I was mainly looking at the encryption side of things, not the application layer. > Integrity / detecting server tampering of data is not really part of the security model here This seems like an own-goal, especially since you could've gotten it pretty much for free using an AEAD construction like GCM(I understand you're using the seeking capability of CTR, but it isn't clear to me why). > Argon2id parameters are set as strong as possible while allowing for execution in a browser context. This is incorrect. They're actually set to as weak as possible while still being safe for use in a password hashing context. I hunted around a while for the parameter set you've opted for here, and finally found it[0]. Is this where you got it from? Importantly, that parameter set is optimized for a minimum security margin in the context of hashing passwords. None of the KDF parameter sets in the Argon2 RFC[1] approach that level of RAM usage. I just ran Argon2id with 5 passes 500 MB RAM in my browser and it executed in a couple seconds. It's also unclear why you need a KDF at all. From what I can tell, users can't specify the password that gets KDF'd into your AES key. Why are you trying to key stretch a 14 character alphanumeric password? Why aren't you just reading 16 random bytes and storing that in the URL hash? [0]: https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html https://cheatsheetseries.owasp.org/cheatsheets/Password_Stor... [1]: https://datatracker.ietf.org/doc/rfc9106/ https://datatracker.ietf.org/doc/rfc9106/