dr|z3d
zzz or eyedeekay, around or about?
dr|z3d
looking at core/java/src/net/i2p/crypto/RSAConstants.java .. it seems our key sizes are in some cases too small..
dr|z3d
// standard specs
dr|z3d
public static final RSAKeyGenParameterSpec F4_1024_SPEC = genSpec(1024, F4);
dr|z3d
public static final RSAKeyGenParameterSpec F4_2048_SPEC = genSpec(2048, F4);
dr|z3d
public static final RSAKeyGenParameterSpec F4_3072_SPEC = genSpec(3072, F4);
dr|z3d
public static final RSAKeyGenParameterSpec F4_4096_SPEC = genSpec(4096, F4);
dr|z3d
these are the current recommended values:
dr|z3d
public static final RSAKeyGenParameterSpec F4_2048_SPEC = genSpec(2048, F4);
dr|z3d
public static final RSAKeyGenParameterSpec F4_3072_SPEC = genSpec(3072, F4);
dr|z3d
public static final RSAKeyGenParameterSpec F4_4096_SPEC = genSpec(4096, F4);
dr|z3d
public static final RSAKeyGenParameterSpec F4_8192_SPEC = genSpec(8192, F4);
dr|z3d
> Use a key of the recommended size or larger. The key size should be at least 128 bits for AES encryption, 256 bits for elliptic-curve cryptography (ECC), and 2048 bits for RSA, DSA, or DH encryption.
dr|z3d
also possible issues with static IV here: core/java/src/net/i2p/crypto/CryptixAESEngine.java:288
dr|z3d
> When a cipher is used in certain modes such as CBC or GCM, it requires an initialization vector (IV). Under the same secret key, IVs should be unique and ideally unpredictable. If the same IV is used with the same secret key, then the same plaintext results in the same ciphertext. This can let an attacker learn if the same data pieces are transferred or stored, or help the attacker run a dictionary attack.
dr|z3d
Also: router/java/src/net/i2p/router/transport/udp/SSUHMACGenerator.java:108 looks potentially problematic.
dr|z3d
> Ensure that you use a strong, modern cryptographic algorithm. Use at least AES-128 or RSA-2048. Do not use the ECB encryption mode since it is vulnerable to replay and other attacks.
eyedeekay
I'm here, probably have to look at how we use each thing first but I don't see anything that actually uses the too-short RSA key sizes in our code, we just have the constants for it
eyedeekay
Less certainty from me on the other two things though
dr|z3d
just flagging some of the github code scanning alerts that I've neglected for too long.
eyedeekay
Yeah we get those from time to time too but there's a ton of false-positive stuff
dr|z3d
> Arbitrary file access during archive extraction ("Zip Slip")
dr|z3d
> core/java/src/net/i2p/util/FileUtil.java:155
dr|z3d
> Unsanitized archive entry, which may contain '..', is used in a .
dr|z3d
> Extracting files from a malicious zip file, or similar type of archive, is at risk of directory traversal attacks if filenames from the archive are not properly validated.
eyedeekay
I've seen that one before, that one's probably a true-positive
eyedeekay
But I didn't see it on i2p.i2p, it was on i2p.android.base IIRC...
dr|z3d
seems like that one's actionable.
dr|z3d
XSS issue in the wizard.
dr|z3d
> apps/i2ptunnel/jsp/WEB-INF/classes/net/i2p/i2ptunnel/jsp/wizard_jsp.java:702
dr|z3d
public class XSS extends HttpServlet {
dr|z3d
protected void doGet(HttpServletRequest request, HttpServletResponse response)
dr|z3d
throws ServletException, IOException {
dr|z3d
// BAD: a request parameter is written directly to the Servlet response stream
dr|z3d
response.getWriter().print(
dr|z3d
"The page \"" + request.getParameter("page") + "\" was not found.");
dr|z3d
}
dr|z3d
}
dr|z3d
looks like we've got those all over, including susimail.
dr|z3d
appears to be a boilerplate chunk of code, so probably easy enough to fix globally.
eyedeekay
Probably, but I'm never assuming anything is easy ever again :)
dr|z3d
wise move :)
eyedeekay
I'll try and sort out false from true positives on the zip slip and the XSSs in a day or two, will need some help/time with the crypto ones though
dr|z3d
I've switched out the standards in the first issue with those listed.
dr|z3d
so removed, 1024, added 8192.
dr|z3d
there are more of less severity rating, but those listed are probably the ones that merit attention. maybe storm in a tea cup, but better to get eyes on them than ignore them.
eyedeekay
Understood, thanks for bringing them up
dr|z3d
are you working on anything at the moment, or just letting the dust settle?
eyedeekay
I'm working on getting the Debian repository replaced and fixed now that I've got a little time for it
eyedeekay
Then I'm on a whole mess of broken links on the website
eyedeekay
bunch of dangling submissions(PRs/MRs) on SAMv3 libraries to review
eyedeekay
Roadmap updates
eyedeekay
The rare worthwhile bug report on Reddit
dr|z3d
busy busy then.. the reddit bug report looked to me like an unstable wireless connection, but there's not a huge amount of info there last time I looked.
dr|z3d
hmm, if he's not routing over i2p and accessing his own b32, then, hmm.
dr|z3d
reading teddit, now there's plenty of info.
dr|z3d
might be vaguely related to slow tranfer speeds on QBittorrent/libtorrent and i2pchat, maybe.
dr|z3d
on i2pchat, I don't think I've ever seen >15KB/s transfer speed for file sends, ever.
dr|z3d
if running some file transfer tests with i2pchat helps identify the issue, let me know if you want to test.
eyedeekay
What I keep seeing right now is an that clienttps for SAM sockets can be null indefinitely in some circumstances
dr|z3d
(the debug logging in i2pchat is pretty verbose, so it might shed some light)
eyedeekay
I'll try it but it seems to be reproducible by running the goSam tests
eyedeekay
the weird thing is that I see the log event but fail to see the concomitant losses
eyedeekay
So I think there might be more than one thing going on
zzz
dr|z3d, you can ignore abuot 99% of that
zzz
or put in the work and do your own research
zzz
if we are using RSA 1024, you just broke it; if not, you wasted your time
zzz
you're hacking our low-level crypto without thought to make github happy? seems bad
dr|z3d
nothing committed, zzz, and I figured you'd have something to say :)
zzz
yeah who's the other guy who used to post linter/checker output, it's not very helpful
dr|z3d
different linter, this one seems a bit more on point.
dr|z3d
the other guy was using a web application scanner which doesn't look at the source code.
zzz
sure but every one could take hours, if you want to help then do the work
zzz
anonymousmaybe
dr|z3d
I was just bringing some of flagged issues to this channel for discussion and consideration. there aren't a huge number of flagged issues that merit attention.
zzz
for example, SSUHMACGenerator is SSU 1, known issue, and was replaced for SSU 2 as documented in the SSU2 proposal. At least get that far
dr|z3d
so nuke the code if it's obsolete!
zzz
SSU 1 is off by default for the next release but the code ripout hasn't happened yet
dr|z3d
ok
zzz
that part will be messy and is low priority
dr|z3d
what about the "zip slip" exploit, that looks actionable?
dr|z3d
References: snyk.io/research/zip-slip-vulnerability | owasp.org/www-community/attacks/Path_Traversal | cwe.mitre.org/data/definitions/22.html
zzz
eyedeekay and I have both looked at it. we have some defenses but not enough to make github happy
zzz
we're not unzipping untrusted content really, the su3's are signed by trusted people
dr|z3d
yeah, trusted until they're not, muwire, case in point.
dr|z3d
and sure, you can remove the certs from the console, but there's still a window of opportunity there.
dr|z3d
better to work on the assumption nothing can be truly trusted than assume everything is.
zzz
sure but it all goes into assigning a priority
zzz
still not sure if it's a real issue or a false positive
dr|z3d
if ((entry.getName().endsWith(".jar.pack") || entry.getName().endsWith(".war.pack")) && !entry.getName().contains("..")) { not a good enough fix?
zzz
I haven't looked at it in a long time.
zzz
If you want to help, construct an attack zip and test it, then if it works, design a fix and test that
zzz
and then file a ticket
dr|z3d
here's what the alert says by way of mitigation: A common way to check that a user-supplied path SUBDIR falls inside a directory DIR is to use getCanonicalPath() to remove any path-traversal elements and then check that DIR is a prefix. However, if DIR is not slash-terminated, this can unexpectedly allow accessing siblings of DIR.