Implement Bag encryption/decryption. #1206

jwon02 · 2017-10-27T19:28:00Z

This PR adds plugins to the Bag class to encrypt/decrypt chunks and connection records in a bag file. By default, the NoEncryptor plugin is loaded to generate backward-compatible unencrypted bag files. Load AesCbcEncrytpro to encrypt the bag contents using a GPG key installed in the system.

ABI compatibility is broken by two new members added to Bag.

dirk-thomas · 2017-10-27T20:50:21Z

tools/rosbag_storage/CMakeLists.txt

@@ -7,7 +7,7 @@ if(NOT WIN32)
 endif()

 find_package(console_bridge REQUIRED)
-find_package(catkin REQUIRED COMPONENTS cpp_common roscpp_serialization roscpp_traits rostime roslz4)
+find_package(catkin REQUIRED COMPONENTS cpp_common pluginlib roscpp_serialization roscpp_traits rostime roslz4)


The dependency on pluginlib needs to be declared in the package manifest in order to be installed on Jenkins (and become a dependency of the Debian package). Therefore CI currently fails.

In general a dependency on pluginlib will be a bit problematic since currently pluginlib is only part of the higher level group "ROS base". When required here it will need to be moved to "ROS core" (see REP 142).

I'm in favour of that change, as I'd like to use plugins (or at least plugin discovery) in tools like rostopic.

That would be "challenging" since rostopic is written in Python and pluginlib in C++ 😉

Yeah, I think I was thinking that the python plugin discovery stuff that rqt uses was part of pluginlib itself, but it's totally not; it's all just right there in the rqt repo. Well never mind that then. :)

In any case, pluginlib is itself a lightweight dependency, isn't it? Is the pocoo stuff it depends on heavy?

It's not heavy: mostly pluginlib and class_loader.

The "problem" is that pluginlib depends on rosconsole which is in the ros_comm repo.
So if rosbag_storage would depend on pluginlib that would result in a repository-level circular dependency. To avoid that some parts would likely need to be split out into a separate repo. (In general splitting this repo into multiple smaller ones is beneficial - but also a lot of effort...)

Declared build/run dependency to libgpgme11-dev, libssl-dev, and pluginlib in package.xml.

The repo-level circular dependency doesn't impact our builds, so we'd be happy to proceed with this path if the proposed change is otherwise acceptable, and plan to assist with splitting out rosconsole in time for the ROS M release of this repository.

Alternatively, I could look at breaking pluginlib's dependency on rosconsole, potentially by just changing its logging over to use the non-ROS console_bridge API instead. Is that acceptable?

If not, we can retain the idea of encryption being a pluggable capability while not actually allowing external encryption modules to be plugged in (eg, setEncryptorPlugin would take a pointer to an instance rather a plugin name and params).

Let us know how you'd like to proceed here.

if the proposed change is otherwise acceptable

I will add some higher level comment to the PR. But the general feature sounds like a good idea to me.

... splitting out rosconsole in time for the ROS M release of this repository.

I don't think we want the repository level circular dependency. So splitting out rosconsole before accepting this feature would be a reasonable path forward.

Alternatively, I could look at breaking pluginlib's dependency on rosconsole, potentially by just changing its logging over to use the non-ROS console_bridge API instead. Is that acceptable?

It might be but you might want to check with the pluginlib maintainer on this. There might be problems which I don't foresee atm.

mikepurvis · 2017-10-27T20:51:02Z

tools/rosbag_storage/src/bag.cpp

@@ -217,6 +218,14 @@ void Bag::setCompression(CompressionType compression) {
    compression_ = compression;
 }

+void Bag::setEncryptorPlugin(std::string const& plugin_name, std::string const& plugin_param) {


How much overlap is there between the encryption functions and the compression functions? My question about this would be whether it would make more sense to have this as a more generic setStoragePlugin, where bzip2 and lzma compression plugins can be loaded similar to the encryption scheme.

Obviously I don't want the scope to get away from us, but it's just a thought.

Would setStoragePlugin cover (i) encryption, (ii) compression, and (iii) the combination of the two?

Honestly, I haven't given it that much thought. If there isn't an obvious path on that, disregard— a plugin hook which applies only to encryption is fine with me.

Thank you for your input @mikepurvis. Nothing is wrong with having a single 'storage' plugin (except that we might end up with mxn plugin classes for m compression methods and n encryptors). Refactoring existing compression stream classes and combining them with encryptors would require lots of efforts, and I would like leave that as a future task.

dirk-thomas · 2017-10-30T16:24:18Z

In general this feature is a great addition. I would have a few comments / questions beside the considerations of the circular dependencies discussed above:

During recording resources are often sparse. Therefore I think it would make sense if the encryption could optionally also be performed after recording the data - similar as the compress option.
How about Windows support? Since some users are building ROS core on Windows it might be worth to consider if this would work or break their build. "Worst case" the feature would gracefully fall back and not be available on that platform.
Please add unit test coverage for this new feature.
Please move all implementations to .cpp files rather than in .h files.

jwon02 · 2017-10-30T16:39:34Z

It makes perfect sense to me to have a rosbag command-line tool to encrypt/decrypt a bag file. Let me work on that.
I have overlooked Windows support. Let me take a look, and come back to this thread.
Let me add unit-tests.
Let me move implementations to .cpp.

jwon02 · 2017-10-31T18:27:59Z

Added a test case, and move implementations to .cpp. Still WIP for Windows support and command-line tools.

jwon02 · 2017-11-01T17:54:04Z

Build on CI fails due to missing library libgpgme11-dev. How can I fix this?

gavanderhoorn · 2017-11-01T17:58:23Z

libgpgme11-dev is not a valid rosdep key, so rosdep can't install it. Contribute the rule, that should make it work (I typically skip the whole forking part).

jwon02 · 2017-11-01T18:26:31Z

Thank you @gavanderhoorn.

jwon02 · 2017-11-02T19:43:02Z

Added command-line tools to en/decrypt bag files. Encrypting a large bag file takes a long time, and a progress meter should be implemented in future.
WIP for Windows support.

jwon02 · 2017-11-03T16:16:55Z

@dirk-thomas All of your initial comments have been addressed, and the PR is ready for rereivew:

Added rosbag command-line tool encrypt and decrypt.
Only rosbag/NoEncryptor is supported for Windows, which means bags cannot be encrypted on Windows, and trying to open an encrypted bag file on Windows would throw an exception saying rosbag/AesCbcEncryptor is not found.
Added a unit test.
Moved implementation to .cpp.

dirk-thomas

I added few more comments inline but haven't taken a too close look to the patch yet.

Since it breaks ABI and should therefore only be considered for the next ROS release (Melodic) we can't merge it until we have a melodic-devel branch. Usually we branch as late as possible (maybe around February) to avoid extra porting effort.

It would be great if other could try this feature in the meantime and comment with feedback here.

dirk-thomas · 2017-11-03T16:19:28Z

tools/rosbag/CMakeLists.txt

+    ARCHIVE DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION}
+    LIBRARY DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION}
+    RUNTIME DESTINATION ${CATKIN_PACKAGE_BIN_DESTINATION})
+endif()


Please put the generic part outside of this conditional block and only create a conditional block for encrypt.

dirk-thomas · 2017-11-03T16:20:27Z

tools/rosbag/src/encrypt.cpp

+      ("help,h",   "produce help message")
+      ("quiet,q",  "suppress console output")
+      ("plugin,p", po::value<std::string>()->default_value("rosbag/AesCbcEncryptor"), "encryptor name")
+      ("param,r",  po::value<std::string>()->default_value("*"),                      "encryptor parameter")


Please avoid such long blocks of spaces for aligning arguments.

Same below.

dirk-thomas · 2017-11-03T16:21:48Z

tools/rosbag/src/encrypt.cpp

+};
+
+void EncryptorOptions::buildOutbagName() {
+    if (!outbag.empty()) {


Please use a consistent style for positioning curly braces. In ROS 1 they are generally on a separate line. But most important is consistency.

Across the code.

I tried to make the style consistent with sibling files, "play.cpp" and "record.cpp", though the style in those two files are problematic. Let me use consistent separate-line braces in encrypt.cpp.

dirk-thomas · 2017-11-03T16:23:32Z

tools/rosbag_storage/CMakeLists.txt

+    src/uncompressed_stream.cpp
+  )
+  target_link_libraries(rosbag_storage ${catkin_LIBRARIES} ${Boost_LIBRARIES} ${BZIP2_LIBRARIES} ${console_bridge_LIBRARIES} crypto gpgme)
+endif()


Please do not duplicate large chunks of code like this. Instead create a variable for the optional files, fill that variable based on the platform, and then use it as one argument to the add_library call.

Addressed. Thank you.

jwon02 · 2017-11-03T20:49:49Z

I'm okay with releasing encryption functionality along with Melodic.

jwon02 · 2017-11-06T18:10:00Z

All review comments and compiler warnings so far have been addressed.

dirk-thomas · 2017-11-06T18:59:00Z

tools/rosbag_storage/CMakeLists.txt

+
+    catkin_add_gtest(test_aes_encryptor test/test_aes_encryptor.cpp
+      WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/test)
+    target_link_libraries(test_aes_encryptor rosbag_storage ${catkin_LIBRARIES} ${Boost_LIBRARIES})


This line needs to be wrapped in a conditional: if(TARGET test_aes_encryptor).

Addressed. Thank you.

dirk-thomas · 2017-11-06T18:59:05Z

tools/rosbag_storage/CMakeLists.txt

+if(NOT WIN32)
+  set(AES_ENCRYPT_SOURCE "src/aes_encryptor.cpp")
+  set(AES_ENCRYPT_LIBRARIES "crypto" "gpgme")
+endif()


The variables should be set as empty outside of the conditional. Otherwise newer CMake versions will report a warning for using an uninitialized variable.

Addressed. Thank you.

…r warnings.

…ncrypted bags. (ros#1206)

* Implement Bag encryption/decryption. (ros#1206) Ported to 1.12.12 * Python Bag class supports encryption; all the rosbag tools work for encrypted bags. (ros#1206) * Improve exception messages raised when a public key is missing. * Randomize initialization vectors for encrypt/decrypt. * Fix bag encryption routine (ros#1310) Bag encryption routine was truncating the recorded block by the size of the IV. * Drop const qualifier from *::decryptChunk methods Since decryption can change EVP's context, these methods can't be const anymore. * Move encryption to from openssl software aes to EVP API * Check EVP API results * Add EncryptionOptions to Recorder * Parse encryption and encryption-param options in the record executable * Fix gtests * Fix rostests - 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)

mikepurvis · 2018-04-23T20:34:00Z

@ros-pull-request-builder retest this please

mikepurvis · 2018-04-23T21:00:47Z

ROS Melodic CI failing due to missing rosdep. Should be resolved by ros/rosdistro#17588.

mikepurvis · 2018-04-24T00:46:24Z

@ros-pull-request-builder retest this please

mikepurvis · 2018-04-24T02:01:00Z

@jwon02 Can you investigate the CI failure which occurred building on Ubuntu Bionic?

18:01:41 ==> make -j1 in '/tmp/catkin_workspace/build_isolated/rosbag_storage'
18:01:41 Scanning dependencies of target rosbag_storage
18:01:41 [  7%] Building CXX object CMakeFiles/rosbag_storage.dir/src/aes_encryptor.cpp.o
18:01:42 In file included from /tmp/catkin_workspace/src/ros_comm/tools/rosbag_storage/include/rosbag/bag.h:65:0,
18:01:42                  from /tmp/catkin_workspace/src/ros_comm/tools/rosbag_storage/src/aes_encryptor.cpp:35:
18:01:42 /opt/ros/melodic/include/pluginlib/class_loader.h:36:2: warning: #warning Including header <pluginlib/class_loader.h> is deprecated, include <pluginlib/class_loader.hpp> instead. [-Wcpp]
18:01:42  #warning Including header <pluginlib/class_loader.h> is deprecated, \
18:01:42   ^~~~~~~
18:01:43 In file included from /tmp/catkin_workspace/src/ros_comm/tools/rosbag_storage/src/aes_encryptor.cpp:40:0:
18:01:43 /opt/ros/melodic/include/pluginlib/class_list_macros.h:36:2: warning: #warning Including header <pluginlib/class_list_macros.h> is deprecated, include <pluginlib/class_list_macros.hpp> instead. [-Wcpp]
18:01:43  #warning Including header <pluginlib/class_list_macros.h> is deprecated, \
18:01:43   ^~~~~~~
18:01:45 In file included from /opt/ros/melodic/include/pluginlib/./class_loader.hpp:350:0,
18:01:45                  from /opt/ros/melodic/include/pluginlib/class_loader.h:39,
18:01:45                  from /tmp/catkin_workspace/src/ros_comm/tools/rosbag_storage/include/rosbag/bag.h:65,
18:01:45                  from /tmp/catkin_workspace/src/ros_comm/tools/rosbag_storage/src/aes_encryptor.cpp:35:
18:01:45 /opt/ros/melodic/include/pluginlib/././class_loader_imp.hpp:68:26: warning: ‘std::vector<std::__cxx11::basic_string<char> > {anonymous}::catkinFindLib()’ defined but not used [-Wunused-function]
18:01:45  std::vector<std::string> catkinFindLib()
18:01:45                           ^~~~~~~~~~~~~
18:01:46 [ 14%] Building CXX object CMakeFiles/rosbag_storage.dir/src/bag.cpp.o
18:01:47 In file included from /tmp/catkin_workspace/src/ros_comm/tools/rosbag_storage/include/rosbag/bag.h:65:0,
18:01:47                  from /tmp/catkin_workspace/src/ros_comm/tools/rosbag_storage/src/bag.cpp:28:
18:01:47 /opt/ros/melodic/include/pluginlib/class_loader.h:36:2: warning: #warning Including header <pluginlib/class_loader.h> is deprecated, include <pluginlib/class_loader.hpp> instead. [-Wcpp]
18:01:47  #warning Including header <pluginlib/class_loader.h> is deprecated, \
18:01:47   ^~~~~~~
18:01:49 /tmp/catkin_workspace/src/ros_comm/tools/rosbag_storage/src/bag.cpp: In constructor ‘rosbag::Bag::Bag(rosbag::Bag&&)’:
18:01:49 /tmp/catkin_workspace/src/ros_comm/tools/rosbag_storage/src/bag.cpp:73:21: error: no matching function for call to ‘pluginlib::ClassLoader<rosbag::EncryptorBase>::ClassLoader()’
18:01:49  Bag::Bag(Bag&& other) {
18:01:49                      ^
18:01:49 In file included from /opt/ros/melodic/include/pluginlib/./class_loader.hpp:350:0,
18:01:49                  from /opt/ros/melodic/include/pluginlib/class_loader.h:39,
18:01:49                  from /tmp/catkin_workspace/src/ros_comm/tools/rosbag_storage/include/rosbag/bag.h:65,
18:01:49                  from /tmp/catkin_workspace/src/ros_comm/tools/rosbag_storage/src/bag.cpp:28:
18:01:49 /opt/ros/melodic/include/pluginlib/././class_loader_imp.hpp:90:1: note: candidate: pluginlib::ClassLoader<T>::ClassLoader(std::__cxx11::string, std::__cxx11::string, std::__cxx11::string, std::vector<std::__cxx11::basic_string<char> >) [with T = rosbag::EncryptorBase; std::__cxx11::string = std::__cxx11::basic_string<char>]
18:01:49  ClassLoader<T>::ClassLoader(
18:01:49  ^~~~~~~~~~~~~~
18:01:49 /opt/ros/melodic/include/pluginlib/././class_loader_imp.hpp:90:1: note:   candidate expects 4 arguments, 0 provided
18:01:50 make[2]: *** [CMakeFiles/rosbag_storage.dir/src/bag.cpp.o] Error 1
18:01:50 CMakeFiles/rosbag_storage.dir/build.make:86: recipe for target 'CMakeFiles/rosbag_storage.dir/src/bag.cpp.o' failed
18:01:50 CMakeFiles/Makefile2:67: recipe for target 'CMakeFiles/rosbag_storage.dir/all' failed
18:01:50 make[1]: *** [CMakeFiles/rosbag_storage.dir/all] Error 2
18:01:50 Makefile:140: recipe for target 'all' failed
18:01:50 make: *** [all] Error 2

mikepurvis · 2018-04-24T18:02:05Z

Remaining failure is known to be flaky. Thanks for the contribution here, @jwon02 and @guillaumeautran!

dirk-thomas · 2018-04-24T19:04:30Z

tools/rosbag_storage/test/test_aes_encryptor.cpp

+    // Test the message decrypted from the bag file
+    bag.open(bag_file_name, rosbag::bagmode::Read);
+    rosbag::View view(bag);
+    EXPECT_EQ(view.size(), 1);


This line might be responsible for the new compiler warning: http://build.ros.org/job/Mdev__ros_comm__ubuntu_bionic_amd64/20/warnings22Result/package.-1053464660/

PS: the warning was also visible in the PR build...

Should I create another PR to remove the warning?

That would be great. Thanks.

dirk-thomas · 2018-05-18T21:35:50Z

I just noticed that my previous raised concern about the pluginlib dependency hasn't been addressed. So currently we have a circular dependency between this repository and pluginlib.

@jwon02 @mikepurvis This needs to be resolved as soon as possible.

mikepurvis · 2018-05-19T17:26:41Z

Oh blah, that is a fail on my part. The three options given in the original discussion were:

Split rosconsole out of this repo. This should be relatively clean, as rostime and rosunit are already in non-ros_comm repos.
Break pluginlib's dep on rosconsole. Proposed and rejected in Depend on libconsole-bridge-dev instead of rosconsole? pluginlib#81.
Hardcode the list of supported plugins, thus dropping (at least for now) the need to explicitly use pluginlib. The only external work I'm aware of which ties into any of this is the hardware-accelerated encryption work discussed in Add hardware-accelerated encryption to rosbags shield-ai/ros_comm#1, but it's not actually implemented as a separate plugin. So this is doable, but limits future expansion. Plus I have other places in ros_comm where I'd like to be able to introduce plugin hooks...

So I don't think I have the necessary permissions to create a new repo for rosconsole, but that would be my preferred resolution to this. If such a repo can be created, I can put up the PRs to drop rosconsole from this repo and add it to the new one.

dirk-thomas · 2018-05-19T20:27:11Z

I will double check the hierarchy first thing Monday morning but I agree that it seems possible to move rosconsole out of this repo since it doesn't depend on anything in this repo. If other packages in this repo depend on it though that would imply that pluginlib needs to be in the "ros_core" variant rather than in "ros_base" (no problem though).

If all is good I will duplicate this repo and remove rosconsole and on the clone remove all other packages. And then do new Melodic releases of both repos.

dirk-thomas · 2018-05-21T17:15:57Z

See ros/rosdistro#17919 for tracking the migration.

clalancette · 2018-05-21T22:54:11Z

There's an additional piece of fallout from this PR that we are now seeing on the buildfarm: http://build.ros.org/job/Mbin_ubhf_uBhf__swri_console__ubuntu_bionic_armhf__binary/2/console (look near the end for the error). Looking at the documentation for GPGME, it looks like we'll probably have to add -D_FILE_OFFSET_BITS=64 to make anything that does #include <bag.h> successfully compile on armhf.

clalancette · 2018-05-21T23:41:27Z

I'm testing a potential fix over on https://github.com/ros/ros_comm/tree/armhf-file-offset-bits

tools/rosbag/src/rosbag/bag.py

dirk-thomas added the consider-future-release label Oct 27, 2017

dirk-thomas reviewed Oct 27, 2017

View reviewed changes

mikepurvis reviewed Oct 27, 2017

View reviewed changes

jwon02 force-pushed the add_bag_encryption_plugin branch 6 times, most recently from eafecb1 to 515189f Compare October 30, 2017 15:59

mikepurvis mentioned this pull request Oct 30, 2017

Depend on libconsole-bridge-dev instead of rosconsole? ros/pluginlib#81

Closed

jwon02 mentioned this pull request Nov 1, 2017

Contribute rosdep rule for libgpgme11-dev ros/rosdistro#16229

Merged

Implement Bag encryption/decryption.

a2c0545

jwon02 force-pushed the add_bag_encryption_plugin branch from 932513e to a2c0545 Compare November 3, 2017 16:12

dirk-thomas reviewed Nov 3, 2017

View reviewed changes

dirk-thomas added the enhancement label Nov 3, 2017

mikepurvis added the hitlist label Nov 3, 2017

mikepurvis added a commit that referenced this pull request Nov 3, 2017

Implement Bag encryption/decryption. (#1206)

321e977

jwon02 force-pushed the add_bag_encryption_plugin branch from 9fccb54 to dffec15 Compare November 6, 2017 17:02

dirk-thomas reviewed Nov 6, 2017

View reviewed changes

Add missing CATKIN_DEPEND to pluginlib; code refactoring; fix compile…

3b583b4

…r warnings.

Burgos pushed a commit to Burgos/ros_comm that referenced this pull request Feb 18, 2018

Python Bag class supports encryption; all the rosbag tools work for e…

305afd2

…ncrypted bags. (ros#1206)

madsciencetist mentioned this pull request Feb 20, 2018

Add hardware-accelerated encryption to rosbags shield-ai/ros_comm#1

Merged

mikepurvis changed the base branch from lunar-devel to melodic-devel April 21, 2018 21:04

jwon02 force-pushed the add_bag_encryption_plugin branch 3 times, most recently from 69030f2 to cd924cc Compare April 24, 2018 15:52

Replace deprecated .h files with .hpp

ac8ebd9

jwon02 force-pushed the add_bag_encryption_plugin branch from cd924cc to ac8ebd9 Compare April 24, 2018 16:02

mikepurvis merged commit 50b377a into ros:melodic-devel Apr 24, 2018

dirk-thomas reviewed Apr 24, 2018

View reviewed changes

mikepurvis mentioned this pull request Apr 24, 2018

debug failing RosAssert tests on Debian Stretch #1358

Closed

dirk-thomas mentioned this pull request May 21, 2018

separate rosconsole ros/rosdistro#17919

Closed

12 tasks

clalancette mentioned this pull request May 22, 2018

Add in -D_FILE_OFFSET_BITS=64 on machines less than 64-bits. #1406

Merged

kartikmohta referenced this pull request in KumarRobotics/multicam_calibration Aug 6, 2018

added node to make work on ubuntu 18.04

e2ab1ab

kartikmohta mentioned this pull request Aug 6, 2018

Declaring a rosbag::Bag member in a nodelet class leads to failure to launch the nodelet #1474

Closed

dirk-thomas mentioned this pull request Aug 9, 2018

changes between 1.14.3 and 1.13.6 for backporting #1477

Merged

dirk-thomas mentioned this pull request Jan 25, 2019

rosbag: why use the Crypto package ? ros/ros#201

Closed

dirk-thomas reviewed Feb 3, 2020

View reviewed changes

tools/rosbag/src/rosbag/bag.py Show resolved Hide resolved

Implement Bag encryption/decryption. #1206

Implement Bag encryption/decryption. #1206

Conversation

jwon02 commented Oct 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwon02 Oct 30, 2017 • edited Loading

Choose a reason for hiding this comment

mikepurvis Oct 30, 2017 • edited Loading

Choose a reason for hiding this comment

dirk-thomas Oct 30, 2017 • edited by mikepurvis Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dirk-thomas commented Oct 30, 2017

jwon02 commented Oct 30, 2017

jwon02 commented Oct 31, 2017

jwon02 commented Nov 1, 2017

gavanderhoorn commented Nov 1, 2017 • edited Loading

jwon02 commented Nov 1, 2017

jwon02 commented Nov 2, 2017

jwon02 commented Nov 3, 2017

dirk-thomas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwon02 commented Nov 3, 2017

jwon02 commented Nov 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikepurvis commented Apr 23, 2018

mikepurvis commented Apr 23, 2018

mikepurvis commented Apr 24, 2018

mikepurvis commented Apr 24, 2018

mikepurvis commented Apr 24, 2018

dirk-thomas Apr 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dirk-thomas commented May 18, 2018

mikepurvis commented May 19, 2018

dirk-thomas commented May 19, 2018

dirk-thomas commented May 21, 2018

clalancette commented May 21, 2018

clalancette commented May 21, 2018

jwon02 Oct 30, 2017 •

edited

Loading

mikepurvis Oct 30, 2017 •

edited

Loading

dirk-thomas Oct 30, 2017 •

edited by mikepurvis

Loading

gavanderhoorn commented Nov 1, 2017 •

edited

Loading

dirk-thomas Apr 24, 2018 •

edited

Loading