forked from Burgos/ros_comm
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add hardware-accelerated encryption to rosbags #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ported to 1.12.12
Bag encryption routine was truncating the recorded block by the size of the IV.
Since decryption can change EVP's context, these methods can't be const anymore.
r2dkennobi
approved these changes
Feb 21, 2018
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't find anything glaringly obvious. Would've liked to have kept the software AES option just in case but otherwise, looks good. Wish I knew more about the gpgme and openssl libraries.
madsciencetist
force-pushed
the
aes_ni
branch
from
February 21, 2018 20:36
f5003b8
to
d446111
Compare
- With ninja, when `_rostest_ARGS` is empty, the space right before it gets escaped, and the command that ultimately gets executed has a trailing slash. - rospy.log testing fails because our ROSCONSOLE_FORMAT does not print severity - bag.py had a bug in get_info_str() that has been fixed upstream - bz2 performs a few bytes better than expected, failing the rosbag compression test - roswtf tests had an outdated dependency list (TBH I don't understand what this list is)
madsciencetist
force-pushed
the
aes_ni
branch
from
February 22, 2018 00:12
d446111
to
a0d5d61
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Start with ros#1206, in which encryption was added to rosbags upstream. An asymmetric GPG public key encrypts a symmetric AES cipher, which encrypts the data itself. To decrypt, the corresponding private key is looked up and used to decrypt the AES cipher to decrypt the data.
The
Bag
class is inros_comm
, the lowest-level ROS package. ros#1206 added a dependency onpluginlib
and changed theBag
ABI, so it could not be added to ROS Kinetic and was instead targeted for ROS Lunar and later. We need it for Kinetic though, so @Burgos backported it to Kinetic. We will have to be careful about the ABI change. Anyros-kinetic-*
package that creates aBag
object in C++ will be break. Conveniently, I don't think there are any; the only nodes/tools I know of that work withBag
objects are in this repo.Benchmarking on my desktop, turning on encryption raised
record
CPU usage from 40% to 50%, which would be unacceptable given out lack of CPU headroom. @Burgos upgraded theaes_encryptor
to use AES-NI hardware-accelerated AES encryption instead of the default software implementation. With AES-NI, turning on encryption only raisesrecord
CPU usage from 40% to 41%.I created this
kinetic-release
branch off of tag 1.12.12, which is currently the most recently released version ofros-kinetic-ros-comm
.