Certificate verification for DCPlusPlus SelfSigned certificates fails when connecting to some users

Bug #1981899 reported by eMTee
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
DC++
Confirmed
Low
Unassigned

Bug Description

...yet the transfer is always allowed and successful in these cases. The error appears in the system log.

[2022-04-03 14:47] <eMTee> So I think I figured out the cause of client cert expiration warnings appearing for certain users in the log at C-C connection time. DC++ generates self signed client certs that are valid for 90 days, simply checks for expiration and regenerates the cert if the cert is expired. However, if you start your client with a valid cert but it expires *while* the client is running then others connecting to you get the message. It can easily happen to people who running the client for days/weeks.
[2022-04-03 14:57] <eMTee> What was confusing that AirDC++ doesn't show this message for peers for which DC++ does. It is because they delibarately suppress this message if the allowUntrusted setting is on. Moreover they are generating certs valid for 1 year and starting to regenerate them at start if the cert is about to expire in 90 days. In this approach a client can only have an expired self signed cert if it's running for more than 90 days.

[2022-04-05 19:56:29] <cologic> And as for the cert findings, okay, why not generate longer-lasting certs? I know LE has advocated for the virtues of short-lived, renewed certs, but I don't see how that helps much with DC++'s use case. There's no cert revocation etc
[2022-04-05 19:56:55] <cologic> There'd still be edge cases for people leaving clients running longer, but less common
[2022-04-05 20:25:09] <cologic> It's not a proper fix, but it's expedient and low-risk
[2022-04-06 08:53] <eMTee> Or leave the certs as is and just silence those cryptic error messages for self signed certs by default (when Allow untrusted certs for C-C is enabled).
[2022-04-06 10:22] <cologic> also reasonable. They're not really working to authenticate anyone anyway. Bad crypto protocol design, I guess, but at least better not to have spurious warnings as a result
[2022-04-06 10:32] <eMTee> Yeah I agree and it is also a minimal change.
[2022-04-06 11:16] <cologic> and works with already-created 90-day certs, so better backwards compatibility

AirDC++'s fix for this issue is in https://github.com/airdcpp/airdcpp-windows/blob/8c359424d883ba836b344383c862ba0b386fc30b/airdcpp/airdcpp/CryptoManager.cpp#L504 but the code there is pretty different / more advanced compared to ours.
Question is where to place this fix in our code or is it useful to get more things / all the improvements from there.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.