Open rst0git opened 11 months ago
Attention: 687 lines
in your changes are missing coverage. Please review.
Comparison is base (
b17a73b
) 70.62% compared to head (a58f851
) 69.24%.:exclamation: Current head a58f851 differs from pull request most recent head 42c52ff. Consider uploading reports for the commit 42c52ff to get more accurate results
Files | Patch % | Lines |
---|---|---|
criu/tls.c | 4.68% | 549 Missing :warning: |
criu/image.c | 29.78% | 33 Missing :warning: |
criu/protobuf.c | 25.00% | 30 Missing :warning: |
criu/page-xfer.c | 24.13% | 22 Missing :warning: |
criu/util.c | 22.72% | 17 Missing :warning: |
criu/mem.c | 26.31% | 14 Missing :warning: |
criu/pagemap.c | 16.66% | 5 Missing :warning: |
criu/cr-dump.c | 50.00% | 4 Missing :warning: |
criu/unittest/mock.c | 0.00% | 4 Missing :warning: |
criu/config.c | 25.00% | 3 Missing :warning: |
... and 3 more |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hello @ueno, @rst0git mentioned that he talked to you about the ciphers used in this PR. Can you take a look at this PR regarding the used ciphers and if they make sense. Unfortunately most of us here do not know that much about encryption and if you could take a look that would be great. Thanks.
@rst0git What about using constructs like cleanup_free
at some places? Could simplify some code paths a bit.
I think we are trying to avoid C++ comments with //
. I see a couple of them.
What happens if CRIU finds an encrypted image? Reading the code and the documentation it will look for a PEM file, right? How will CRIU react if the PEM file doesn't exist?
What about using constructs like cleanup_free at some places? Could simplify some code paths a bit.
Sounds like a good idea!
I think we are trying to avoid C++ comments with //. I see a couple of them.
Thanks, I've updated the comments to use /*
.
What happens if CRIU finds an encrypted image? Reading the code and the documentation it will look for a PEM file, right?
When criu dump
is used with --encrypt
, it generates an image file called cipher.img
. Subsequently, criu restore
checks if cipher.img
exists in the directory with image files. This file serves as an indicator that all other CRIU images are encrypted. In such case, CRIU loads a private key from the default location, which is /etc/pki/criu/private/key.pem
, or from an alternative path specified using the --tls-key
option.
How will CRIU react if the PEM file doesn't exist?
If the PEM file with private key doesn't exist, CRIU fails with the following error:
(00.001370) tls: Loading private key from /etc/pki/criu/private/key.pem
(00.001405) Error (criu/tls.c:585): tls: Can't stat /etc/pki/criu/private/key.pem: No such file or directory
A friendly reminder that this PR had no activity for 30 days.
This pull request extends CRIU with support for encrypted images. A new cli option,
-e|--encrypt
, is used to enable this functionality with thedump
command.The implementation is based on the existing integration with GnuTLS, using ChaCha20-Poly1305 for protobuf and raw images, and AES-XTS for memory pages. The symmetric keys used for encryption are randomly generated, encrypted with a public key loaded from X.509 certificate and stored in
cipher.img
. During restore, ifcipher.img
exists, CRIU will load a corresponding private key from a PEM file and decrypt the symmetric keys.Usage example:
The following figure shows the results of performance evaluation, where CRIUsec includes the changes in this pull request, CRIU is used without encryption as a baseline, and GnuPG, OpenSSL, age are alternative solutions used with post-dump action-script for comparison.