Show HN: Dat-keyserver: a distributed PGP keyserver based on the Dat protocol
github.comI like the ability to remove keys. I understand the theoretical reason for append-only keyservers, but in practice it just turns people off from using them. No one wants to look at their entries from 2005 from when they screwed up while learning about subkeys, or their defunct 2008 entry which they never could revoke because they lost the revocation certificate (all examples purely fictional).
In any case, a keyservers job is not even to be some kind of source of trust, so all that really should matter is that it has a user's most up-to-date keys on it. Validating a key should come from web-of-trust or some secure second channel verification method (like listing your key ID on a TLS-enabled website).
> No one wants to look at their entries from 2005 from when they screwed up while learning about subkeys
Yeah, it's interesting that even platforms such as Keybase that allow changing or removing data timestamp them in irremovable ways sometimes (e.g. following someone snapshots their profile in the follower's sigchain).
> In any case, a keyservers job is not even to be some kind of source of trust, so all that really should matter is that it has a user's most up-to-date keys on it. Validating a key should come from web-of-trust or some secure second channel verification method.
Actually having a key on a keyserver was never relevant as web-of-trust runs parallel to the key origin (that is if you're connected to WoT you'd know which key is the real one). But it's not practical in general.
> like listing your key ID on a TLS-enabled website
Web Key Directory is something like that and it's trivial to set up: https://spacekookie.de/blog/usable-gpg-with-wkd/
I assume a bigger justification for not running an append-only keyserver was this dumpster fire[1] in which the append-only nature of the SKS keyserver ecosystem was abused as a DOS vector.
---
I'm not fluent in modern JS, but I think the level of callbacks nesting makes it hard to see what else/catch belong to what if/then, especially: https://github.com/tdjsnelling/dat-keyserver/blob/12fa3e8389...
You could try splitting this big function into smaller functions to reduce the length of the code largest callbacks.
It also makes commits harder to read. eg. for https://github.com/tdjsnelling/dat-keyserver/commit/12fa3e83... a reader can't see easily what changed in the function, as every line's indentation was changed. (And the commit message does not explain what the bug was)
Ha, omg this is impossible to read. Using async/await here would go a long way to helping. But honestly this just seems like a work in progress, so code cleanliness and ability for others to read and contribute might not be the focus.
Also, generally people do not put all their logic in the same file , in express apps. Just breaking out the business logic from the express app setup would also help to make it more readable/understandable.
> Ha, omg this is impossible to read
Please edit swipes like this out of your HN comments, particularly in Show HN threads: https://news.ycombinator.com/showhn.html.
It's also incorrect usage of promises; idiomatically no part of that code should be nested more than two levels deep. But yeah, async/await would be both cleaner to read and easier to write correctly.
(Ultimately a minor issue at this stage of what's otherwise a really neat project!)
Appreciate all the feedback, the code is certainly pretty hacky right now and I intend to clean it up in future. Glad you like the project!
I can greatly recommend
for that problem. There are weaker options to only ignore indentation, iirc.git log -p --color-words git show --color-words <commut-ref>Diff-so-fancy [1] is even better at visualizing this, in my opinion.
Would agree, my personal preference with this would be functions for each of the if/else cases for some extra readability/documentation.
The linked code block is basically the poster child of why async/await was made.
Even without async/await, it could be better if some unnecessary nesting would be cleaned up
could beopenpgp.cleartext .readArmored(req.body.message) .then(message => { openpgp.key .readArmored(nodes[0].value.key) .then(key => { ... }) })openpgp.cleartext .readArmored(req.body.message) .then(message => openpgp.key.readArmored(nodes[0].value.key) ) .then(key => { ... })Did you try running the code with the change you suggested? The lattermost closure in your snippet (`key => { ... }`) needs to be able to reference `message`, but you've moved it out of scope.
This and similar subthreads in this Show HN entry are a good example of pointless, hostile, obnoxiously-pedantic-without-even-being-technically-correct wankery that makes HN an often terrible place for conversations.
Ugh you're right, it wouldn't work then. For what it's worth I was mostly commenting on the "the poster child of why async/await was made" comment above with the idea that async/await wouldn't necessarily be needed to make the code cleaner. But as you pointed out, in this case it wouldn't work. I'm sorry if the commment came out as wankery.
I'm concerned about the key-removal functionality. The website (https://keys.tdjs.tech) reads: "Enter a message clearsigned with the key you wish to remove (message content is not important)"
My understanding of this is that anyone with a copy of anything you've ever signed can revoke your key. I hope I'm misunderstanding.
That is something glaringly obvious that I should have considered, thanks for pointing it out. I'll make the change to require a specific message.
Probably glaringly obvious (but in this case also), but you should make sure that the specific message starts with something highly conspicuous like "DAT KEYSERVER KEY REVOCATION REQUEST ID#<token>" to avoid social engineering someone with "Hey can you sign this token so I know you're you?".
The message has to be signed with the private key of the key so no one but the real owner can delete it. In fact, in the readme:
> If a user can prove that a key belongs to them (by signing a message with their private key) then they are able to remove their public key with no interaction needed from the server operator. Once a key is removed, it is removed from all servers in the pool.
This is a pretty fucking awesome idea.
To prove that you have the private key, they should send you a challenge message and make you send back that message with a signature. If you can send any signed message, anyone who has ever received a signed message from a person can upload it and revoke that person's key.
> If you can send any signed message
Reading the code, it does indeed allow any signed message: https://github.com/tdjsnelling/dat-keyserver/blob/12fa3e8389...
The attack that I believe fwip is concerned about is:
Alice sends an email to Bob and clearsigns the message. Bob, or anyone else who intercepts the email is now able to paste that message into the form and remove Alice's key from the keyserver.
This could be mitigated by requiring it to be a specific message.
GnuPG generates a revocation certificate when you're creating a new key, import that and it requires no interaction from you or the keyserver operator. And it works even if you've forgotten the passphrase of your GPG key. Obviously you don't want to put that file anywhere public, and if it gets compromised you can just publish the file to the keyservers yourself and generate a new key.
Project looks definitely interesting. Too bad the code looks like callback-hell from 10 years ago: https://github.com/tdjsnelling/dat-keyserver/commit/12fa3e83...
Still, an interesting alternative for people who consider https://keys.openpgp.org too radical.
OP, please Google "promise chaining", it will make your life so much better.
At that point, why not go straight to async functions?
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Refe...
At that point, why not go straight to monads?
Because one is a language feature and the other is yet another JS library made by some person.
I don't really get why people need to use async methods (regardless if they are promises or something else) when in fact they are implementing a blocking workflow where one method call needs the results of a preceding method call. It would be much easier to just use synchronized/blocking functions because then you get the desired behaviour for free and need not jump through hoops like promises chaining.
I get that there are APIs that are only available in an async fashion these days and I was forced to bolt on an blocking workflow on an async API myself and didn't particularly enjoy it.
Not sure I understand your complaint. Don't promises ultimately wait upon external events, like a connection receiving data or a timeout being triggered? Given that Javascript is single threaded it is the only way of not blocking the event loop.