Remove unnecessary signature algorithms

[coneiric]

---

By submitting this pull-request, I confirm the following:

• I have read and understood the contributor guide in kovri-docs.
• I have checked that another pull-request for this purpose does not exist.
• I have considered and confirmed that this submission will be valuable to others.
• I accept that this submission may not be used and that this pull-request may be closed by the will of the maintainer.
• I give this submission freely under the BSD 3-clause license.

---

Remove signatures according to #498

[anonimal]

Have you ran a live test yet and tested a client/server tunnel? Like I said in IRC, simply removing the signatures won't work. I can finish the remainder of the review this week but please do continue development in the meantime.

[anonimal]

MaxBuffer = core::RSASHA5124096_KEY_LENGTH

This quickly breaks functionality. See the use-case for MaxBuffer.

[coneiric]

I didn't realize this breaks functionality. I'll revert MaxBuffer, and do some reading. Is the MinBuffer value still fine?

[coneiric]

Have you ran a live test yet and tested a client/server tunnel?

I haven't checked with a full live test yet. Was able to start Kovri, and got it to connect to peers.
Will test out if tunnel creation works.

Any tips for writing integration tests for Kovri?

[anonimal]

Any tips for writing integration tests for Kovri?

Aside from the prelim Boost.Python work and testnet, there's nothing currently in-place aside from running the router and testing tunnels manually.

[coneiric]

Just ran into a connection bug with a pull from the latest master. Thought it might have been something from my changes, so cloned a fresh copy, and same bug came up with building tunnels.

A few different errors keep popping up, (some rejections, timeouts, missing parts, etc.). The project builds fine, and runs without crashing.

Here is a gist of the error logs

Should I open an issue for this?

[anonimal]

Just ran into a connection bug with a pull from the latest master

Those are all known and actual error reporting needs fine-tuning (tunnel building works regardless). Not related to this PR, no need for a new issue.

[anonimal]

@anonimal       oneiric: I'm doing a hefty amount of related review + related work on your PR. I'll end up committing something to your branch instead of sending diffs.
@anonimal       Essentially one of the problems is how we don't properly handle silent fails (somewhat silent) when encountering older routers in our netdb + when we want to build tunnels.
@anonimal       These older routers are signing with DSA-SHA1 and we never remove them from the netdb once parsed/created.
@anonimal       That and more. I've patched the other areas you missed but there's still more to do and I haven't caught it all yet.
@anonimal       This work would've been easier to finish if that identity rewrite was resolved first. Oh well, here we are.
@anonimal       s/how we don't/how we don't with this PR/
@anonimal       I've added much needed trace logging too, really makes things easier.
@anonimal       Dumping the RIs in hex and running kovri-util should be reduced to simply running GetDescription during runtime. I'll make the changes and more.
@anonimal       Anyway, next week (Monday).

The above reference was regarding nightmare code #366.

[anonimal]

An update/clarifications:

1. I've put this PR on the back-burner and will probably continue to do so for the rest of the month because its resolution is not a requisite for release. We'll see.
2. After reseed, most (but not all) of the time, we remove NetDb RI entries once they are confirmed DSA (set as unreachable, the NetDb eventually removes) but even when they are successfully removed, this isn't done quickly enough (saved ~1 minute later, deleted ~3/4 minutes after that) so, transports attempt to connect anyway - later failing during signature verification.
3. When both client / server tunnel Ed25519 keys are in use, and we attempt to use tunnels, there are silent failures that need to be logged and fixed
4. When server tunnel keys are DSA, obvious errors:
   • When HandlingDatabaseStoreMessage / creating remote LeaseSet from DatabaseStore, lease(s) is(are) DSA (keytype 0). We don't handle appropriately in LeaseSet's ReadFromBuffer (patched to catch, but not PR'd; if any lease is DSA, the entire leaseset is null - this is not a real solution), nor elsewhere (not patched).
     • Other non-Ed25519 behavior is TBD but it should be behave similar so long as the aforementioned isn't fixed.

[coneiric]

Thanks for your notes and work on this PR. Apologies for lack of reply, was unable to access my accounts.

I've put this PR on the back-burner

Totally understand. I'll keep working on the points you brought up, hopefully ready by release after this one :)

After reseed, most (but not all) of the time, we remove NetDb RI entries

Will look for areas in code to shorten the gap between confirming DSA reseed entries, and deleting them from NetDb. Is there a minimum amount of time the entries need to stay in the NetDb?

When both client / server tunnel Ed25519 keys are in use ... there are silent failures that need to be logged and fixed

Will look into sections of the code dealing with this scenario, and get an understanding of what those silent failures are.

When HandlingDatabaseStoreMessage / creating remote LeaseSet from DatabaseStore, lease(s) is(are) DSA (keytype 0) ... if any lease is DSA, the entire leaseset is null - this is not a real solution)

Is it possible to separate the DSA entry from the rest of the LeaseSet? Does one member rely on all others in the set? Like in the catch scenario, was thinking about carving out the DSA entry(s), and replacing them with valid entries from elsewhere / doing a DatabaseStore reorg to only include ed25519 in LeaseSet. Not sure how that would work in code, or if it is even a good idea on the conceptual level.

Many thanks for the pointers on where to go from here.

Side note: ran another live test after merging latest master, and now getting SIGILL on kovri::core::IdentityEx::GetSigningKeyType ... /kovri/src/core/router/identity.cc:355. This doesn't happen in master build, so tracking down how to catch this properly with changes from this PR.

[anonimal]

Is it possible to separate the DSA entry from the rest of the LeaseSet?

IIRC, should be if implemented.

Like in the catch scenario, was thinking about carving out the DSA entry(s), and replacing them with valid entries from elsewhere / doing a DatabaseStore reorg to only include ed25519 in LeaseSet.

Ok. Let's see what you come up with.

[anonimal]

Likely not the correct solution, only fixes SIGILL runtime
interrupt.
std::unique_ptrstd::uint8_t[] null_key = std::make_uniquestd::uint8_t[](m_ExtendedLen - 2);
return bufbe16toh(null_key.get()); // null crypto key

Not a real fix. If you drop this commit and rebase the branch, I can start sending in the patches I mentioned earlier.

[anonimal]

Likely not the correct solution, only fixes SIGILL runtime
interrupt.
std::unique_ptrstd::uint8_t[] null_key = std::make_uniquestd::uint8_t[](m_ExtendedLen);
return bufbe16toh(null_key.get()); // signing key

Not a real fix. If you drop this commit and rebase the branch, I can start sending in the patches I mentioned earlier.

[coneiric]

I can drop that commit no problem, where should I rebase the branch to? Just don't want all the comments to get deleted, unless that's not a problem.

[coneiric]

wtf, just tried to reset the last two commits, and Github auto-close the PR...

[anonimal]

Hmm, can you reopen?

[coneiric]

Phew, think that worked. Let me know if anything still looks screwy.

Re-did the deletions in `router/identity.cc`, and omitted the "fix" for `GetSigningKeyType()` and `GetCryptoKeyType()`,

[anonimal]

Is there a minimum amount of time the entries need to stay in the NetDb?

@coneiric I missed this question. See Time::RouterExpiration and its implementation (both are in src/core/router/net_db). Also see the docs for details. Also note LeaseSet behavior and expiration time.

[coneiric]

@anonimal, after researching docs and code a bit, found the m_ExcludedPeers vector of IdentHash(es).

Wasn't sure if it was the right bag to put newly found Destinations when they have unsupported signing key types.

What I was thinking was somehow store IdentHashes known to have unsupported SigningKeyTypes, and check against that list similar to how the m_ExcludedPeers list works when doing DatabaseLookups.

For example, here in the RouterIdentity code, we could calculate the IdentHash and remove it / ban it for one floodfill round (60min).

The "remove it" part is where I'm struggling. Not sure that the m_ExcludedPeers list was intended for something like what I'm attempting, or if there is a better way.

Edit Bad idea, could be used to de-anonymize clients: ~~~Would it make more sense to store unsupported destinations with a "blacklist" flag in the NetDB, and check against that?~~~

Or possibly using peer profiling to exclude invalid peers? Edit <- Researching best way to do this using RouterInfo and RouterContext.

Similar sections in the datagram api, where we can match on all unsupported signature types.

[anonimal]

#498 will be vastly easier to resolve once #366 and #643 and resolved.

[coneiric]

Closing until #366 and #643 are resolved.