← Back to team overview

linuxdcpp-team team mailing list archive

[Bug 1981899] [NEW] Certificate verification for DCPlusPlus SelfSigned certificates fails when connecting to some users

 

Public bug reported:

...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.

** Affects: dcplusplus
     Importance: Low
         Status: Confirmed

-- 
You received this bug notification because you are a member of
Dcplusplus-team, which is subscribed to DC++.
https://bugs.launchpad.net/bugs/1981899

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

Status in DC++:
  Confirmed

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 manage notifications about this bug go to:
https://bugs.launchpad.net/dcplusplus/+bug/1981899/+subscriptions