Skip to content

fix(net-tcp-server): give the generated TLS cert a relative expiry - #4091

Open
Github-Samuel wants to merge 1 commit into
citizenfx:masterfrom
Github-Samuel:fix/tls-cert-relative-expiry
Open

fix(net-tcp-server): give the generated TLS cert a relative expiry#4091
Github-Samuel wants to merge 1 commit into
citizenfx:masterfrom
Github-Samuel:fix/tls-cert-relative-expiry

Conversation

@Github-Samuel

Copy link
Copy Markdown

Goal of this PR

Stop the auto-generated server certificate from being expired the moment it is created.

TLSServer generates a self-signed certificate when there is no certificate on disk, and pins its notAfter to a fixed date:

Botan::X509_Cert_Options options;
options.country = "XX";
options.common_name = "do-not-trust.citizenfx.tls.invalid";
options.not_after("20260101000000Z");

That date is in the past now, so a certificate generated today is written out already expired.

How is this PR achieving the goal

X509_Cert_Options already takes a lifetime relative to the current clock, described in x509self.h as "the expiration time (from the current clock in seconds)":

X509_Cert_Options(const std::string& opts = "",
                  uint32_t expire_time = 365 * 24 * 60 * 60);

so the fixed date can just be handed to the constructor as a duration and the not_after call dropped:

Botan::X509_Cert_Options options("", 10 * 365 * 24 * 60 * 60);
options.country = "XX";
options.common_name = "do-not-trust.citizenfx.tls.invalid";

Ten years keeps the same "generate it once and forget about it" behaviour the fixed date was going for. Dropping the not_after call on its own would also work and would leave the constructor default of one year, but since nothing regenerates the certificate later that would just move the problem a year out.

What this does not change

Two things worth being clear about, since this looks more alarming than it is:

  • It is not a security fix. The client sets CURLOPT_SSL_VERIFYPEER/VERIFYHOST to 0, so nothing on the client side was checking this certificate's validity in the first place, and joins were not failing because of it. This is about the certificate being well-formed, not about it being trusted.
  • It only affects newly generated certificates. A server that already has server-tls.crt on disk keeps loading it, expired or not, because the load path takes the file whenever it exists. I deliberately did not add "regenerate when expired" here, since the same two paths are what an operator would use to supply their own certificate and silently overwriting that seemed worse than leaving it alone. Happy to add it if you would rather, it just felt like a separate decision.

This PR applies to the following area(s)

FXServer

Successfully tested on

Game builds: n/a, server-side only

Platforms: Windows, Linux

Checklist

  • Code compiles and has been tested successfully.
  • Code explains itself well and/or is documented.
  • My commit message explains what the changes do and what they are for.
  • No extra compilation warnings are added by these changes.

I could not do a full server build locally so I left the first box unchecked. I did check the call against the real X509_Cert_Options signature from the vendored Botan header, and that 10 * 365 * 24 * 60 * 60 is 315360000, well inside both int and uint32_t.

The self-signed certificate the server generates when no certificate is
present had its notAfter pinned to the literal 20260101000000Z. That date has
passed, so every certificate generated from now on is already expired the
moment it is written out.

Pass the lifetime to X509_Cert_Options instead, which takes it as an offset
from the current clock, so the certificate is valid from when it is actually
generated.

Signed-off-by: Samuel Nicol <99494967+Github-Samuel@users.noreply.github.com>
@github-actions github-actions Bot added the invalid Requires changes before it's considered valid and can be (re)triaged label Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

invalid Requires changes before it's considered valid and can be (re)triaged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant