Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpcserver: Support dynamic cert reload. #3153

Merged
merged 1 commit into from Jul 24, 2023

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Jul 10, 2023

This modifies the RPC server to support dynamically reloading (aka hot reload) the RPC certificate/key pair as well as the client CAs (when configured with --authtype=clientcert).

In other words, dcrd will now notice when the certificates have been changed on the file system on new connections and reload and cache the new ones.

In terms of deciding when to reload the files, this implementation opts for a highly portable stat-based approach that does not require any additional dependencies over using platform specific file change notifications such as inotify on Linux.

This implementation also aims to provide nice error handling semantics and includes additional logic to minimize the amount of disk accesses needed to determine when the files have changed.

The following is an overview of the semantics:

  • All connections used a cached TLS config
  • Certs are only tested for changes and reloaded when:
    • A new connection is established
    • At least 5 seconds have passed since the last check
    • The file modification times and/or sizes have changed
  • The existing working certs are retained if any errors are encountered when loading the new ones in order to avoid breaking a working config
  • Only a single error will be shown for attempt at loading an invalid config as opposed to spamming the same error on every new connection

@davecgh davecgh added this to the 1.9.0 milestone Jul 10, 2023
server.go Outdated Show resolved Hide resolved
@davecgh davecgh force-pushed the rpcserver_reloadable_certs branch from 07dbf1a to ab47b11 Compare July 10, 2023 17:50
server.go Outdated Show resolved Hide resolved
Comment on lines +3302 to +3306
changed := fi.Size() != f.curSize || fi.ModTime() != f.curTime
if changed {
f.curSize = fi.Size()
f.curTime = fi.ModTime()
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No big deal, but shouldn't the modified time be enough?

Copy link
Member Author

@davecgh davecgh Jul 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the time, yes, but it isn't always.

For example, it is quite common on some shared file systems to preserve the modification time when copying files. Another example is cp -p which various sync methods use.

server.go Outdated Show resolved Hide resolved
@davecgh davecgh force-pushed the rpcserver_reloadable_certs branch from ab47b11 to a00201a Compare July 24, 2023 06:28
This modifies the RPC server to support dynamically reloading (aka hot
reload) the RPC certificate/key pair as well as the client CAs (when
configured with --authtype=clientcert).

In other words, dcrd will now notice when the certificates have been
changed on the file system on new connections and reload and cache the
new ones.

In terms of deciding  when to reload the files, this implementation opts
for a highly portable stat-based approach that does not require any
additional dependencies over using platform specific file change
notifications such as inotify on Linux.

This implementation also aims to provide nice error handling semantics
and includes additional logic to minimize the amount of disk accesses
needed to determine with the files have changed.

The following is an overview of the semantics:

- All connections used a cached TLS config
- Certs are only tested for changes and reloaded when:
  - A new connection is established
  - At least 5 seconds have passed since the last check
  - The file modification times and/or sizes have changed
- The existing working certs are retained if any errors are encountered
  when loading the new ones in order to avoid breaking a working config
- Only a single error will be shown for attempt at loading an invalid
  config as opposed to spamming the same error on every new connection
@davecgh davecgh force-pushed the rpcserver_reloadable_certs branch from a00201a to 488b816 Compare July 24, 2023 16:52
@davecgh davecgh merged commit 488b816 into decred:master Jul 24, 2023
2 checks passed
@davecgh davecgh deleted the rpcserver_reloadable_certs branch July 24, 2023 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants