-
Notifications
You must be signed in to change notification settings - Fork 44
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
Use XML and XSLT to perform permission transform #72
Conversation
So at this point, this xml profile format is working in generating complaint permission files. Before I go an add some more tests, perhaps folks would like to give some feedback on the profile structure, or play around with making profiles. The commands work the same way, only it takes in an xml file now for generating permissions, and the node name given should be FQN: e.g This should allow the permission generation for a given transport to be less coupled with the heigherleve API and CLI. We can add more transforms for future transports and version them as well to support multiple versions/releases of the same transport. ping @jacobperron @mjcarroll and perhaps @ross-desmond. |
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.
Sorry for the delayed response.
I'm not able to run through the demo. When attempting to generate keys for talker/listener:
$ ros2 security create_key demo_keys talker
Namespace(NAME='talker', ROOT='demo_keys', _command=<sros2.command.security.SecurityCommand object at 0x7f33e0b4b390>, _verb=<sros2.verb.create_key.CreateKeyVerb object at 0x7f33e0067278>, **{' command': 'security', ' verb': 'create_key'})
creating key for node name: talker
creating ECDSA param file: demo_keys/talker/ecdsaparam
running command in path [None]: openssl ecparam -name prime256v1 > demo_keys/talker/ecdsaparam
creating key and cert request
running command in path [demo_keys]: openssl req -nodes -new -newkey ec:talker/ecdsaparam -config talker/request.cnf -keyout talker/key.pem -out talker/req.pem
Generating an EC private key
writing new private key to 'talker/key.pem'
-----
creating cert
running command in path [demo_keys]: openssl ca -batch -create_serial -config ca_conf.cnf -days 3650 -in talker/req.pem -out talker/cert.pem
Using configuration from ca_conf.cnf
Can't open ./index.txt.attr for reading, No such file or directory
139862021646144:error:02001002:system library:fopen:No such file or directory:../crypto/bio/bss_file.c:74:fopen('./index.txt.attr','r')
139862021646144:error:2006D080:BIO routines:BIO_new_file:no such file:../crypto/bio/bss_file.c:81:
Check that the request matches the signature
Signature ok
Certificate Details:
Serial Number: 4096 (0x1000)
Validity
Not Before: Dec 19 22:24:36 2018 GMT
Not After : Dec 16 22:24:36 2028 GMT
Subject:
commonName = talker
X509v3 extensions:
X509v3 Basic Constraints:
CA:FALSE
Certificate is to be certified until Dec 16 22:24:36 2028 GMT (3650 days)
Write out database with 1 new entries
Data Base Updated
Traceback (most recent call last):
File "/root/tmp_ws/install/ros2cli/bin/ros2", line 11, in <module>
load_entry_point('ros2cli==0.6.2', 'console_scripts', 'ros2')()
File "/root/tmp_ws/install/ros2cli/lib/python3.6/site-packages/ros2cli/cli.py", line 69, in main
rc = extension.main(parser=parser, args=args)
File "/root/tmp_ws/install/sros2/lib/python3.6/site-packages/sros2/command/security.py", line 37, in main
return extension.main(args=args)
File "/root/tmp_ws/install/sros2/lib/python3.6/site-packages/sros2/verb/create_key.py", line 34, in main
success = create_key(args)
File "/root/tmp_ws/install/sros2/lib/python3.6/site-packages/sros2/api/__init__.py", line 454, in create_key
create_permission_file(permissions_path, name, domain_id, {'topics': None})
File "/root/tmp_ws/install/sros2/lib/python3.6/site-packages/sros2/api/__init__.py", line 319, in create_permission_file
_permissions_xml = policy_xsl(policy_element)
File "src/lxml/xslt.pxi", line 513, in lxml.etree.XSLT.__call__
File "src/lxml/apihelpers.pxi", line 41, in lxml.etree._documentOrRaise
TypeError: Invalid input object: dict
package.xml
Outdated
@@ -13,6 +13,7 @@ | |||
<depend>ros2cli</depend> | |||
|
|||
<exec_depend>openssl</exec_depend> | |||
<exec_depend>python3-lxml</exec_depend> |
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.
AFAICT, there is no rosdep rule for python3-lxml
:
sros2: Cannot locate rosdep definition for [python3-lxml]
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.
I'm not sure how to add a package key to the ros2/rosdistro listing as one would normally do for ros1, as there doesn't seem to be a rosdep/python.yaml
in the ros2 fork given the structure has changed. How would I go about doing the ros2 equivalent of this: ros/rosdistro#12314
python3-lxml is already released in the official ubuntu, debian, and fedora distros:
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.
We're still using the ROS 1 repo for rosdep: https://github.com/ros/rosdistro/tree/master/rosdep
You can add the key there.
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.
@jacobperron , please see ros/rosdistro#19824
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.
Does python3-lxml
also need to be a <test_depend>
or just a <depend>
?
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.
No, I don't think so. According to http://wiki.ros.org/catkin/package.xml#Dependencies:
They should never duplicate any dependencies already mentioned as build or run dependencies.
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 the built in xml
module be used instead of lxml
? https://docs.python.org/3.6/library/xml.etree.elementtree.html
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 the built in
xml
module be used instead oflxml
?
If all we needed was to parse XML markup, then yes the standard python module xml
would suffice. However in the interest of keeping the templating pipeline implementation agnostic (so that SROS2 security profiles may be consumed by other tools and languages), I've elected to use XML templating as well. This necessitates the use of both schema validation and template processing for the complete profile-to-permission pipeline. There are few python modules that provide XSD validation, and fewer still that also support XST templating. At the moment, lxml
seems to be the only game in town.
https://stackoverflow.com/questions/299588/validating-with-an-xml-schema-in-python
For a while with a previous work, I was using xmlschema
; a python native XSD engine (lacks a XST processor). Yet I was somewhat misusing it as a XST engine as well for pipelining permission translation; something a that proper XST files are much more adapt as solving.
https://pypi.org/project/xmlschema/
Also, I'm not sure if the latest fix to the standard modul's Xincludes
processor as been backported to the version of python we target:
https://bugs.python.org/issue20928
Thus the case for lxml
.
@jacobperron , I've updated the key generation, as well as the governance templating, so I no longer encounter there error above you've show. Please give it another try, just be sure to provide the names of the nodes as a FQN with a root slash: ros2 security create_keystore demo_keys
ros2 security create_key demo_keys /talker
ros2 security create_key demo_keys /talker
ros2 security create_permission demo_keys /talker <path_to>/sample_policy.xm You should also be able to create keys for nodes nested under deeping namespaces: ros2 security create_key demo_keys /foo/talker
ros2 security create_key demo_keys /foo/bar/talker
ros2 security create_permission demo_keys /foo/bar/talker <path_to>/sample_policy.xm Note: you'd have to modify the sample_policy.xml example to include the |
ping @jacobperron @mjcarroll and perhaps @ross-desmond Services and actions for Crystal are also ported. |
ping @jacobperron @mjcarroll , could I get some feedback or CI triggered? This is ready for review.. |
Sorry for the delay. |
@ruffsl I guess our CI doesn't install deps based on the package.xml. I'll look into it. |
Currently, it does not, it's based on a fixed set of packages. Many deps are installed in this neighborhood: https://github.com/ros2/ci/blob/master/linux_docker_resources/Dockerfile#L106 If we can get a branch on |
Check that, we could probably inject it here for all platforms: https://github.com/ros2/ci/blob/master/ros2_batch_job/__main__.py#L49 |
I think you'll have to fix the CI branch again. ros2/ci@5790679#r31903461 |
ping @jacobperron @mjcarroll and perhaps @ross-desmond I've just refactored the PR to the new repo structure and have updated the generate_profile verb to create profiles using the new xml format. Please take a look. |
SROS2_Windows.md
Outdated
``` | ||
|
||
And now we will use it to generate the XML permission files expected by the middleware: | ||
|
||
```bat | ||
ros2 security create_permission demo_keys talker demo_keys/policies.yaml | ||
ros2 security create_permission demo_keys listener demo_keys/policies.yaml | ||
ros2 security create_permission demo_keys /talker demo_keys/policy.xml |
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.
These commands are no longer working for me:
ros2 security create_permission demo_keys /talker demo_keys/policy.xml
Namespace( command='security', verb='create_permission', NAME='/talker', POLICY_FILE_PATH='demo_keys/policy.xml', ROOT='demo_keys', _command=<sros2.command.security.SecurityCommand object at 0x7effc05900b8>, _verb=<sros2.verb.create_permission.CreatePermissionVerb object at 0x7effbfbd4d68>)
key_dir demo_keys/talker
unable to find profile "/talker"
I've copied the new default policy.xml. Is the <topic>/*</topic>
tag not not working as expected?
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.
What is a good way to store and locate a ros2 package asset via command line?
https://answers.ros.org/question/288501/ros2-equivalent-of-rospackagegetpath/
Sortof something like this:
ros2 security create_permission demo_keys /talker $(ros2 pkg prefix sros2)/test/policies/sample_policy.xml
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.
I updated the readme to use the simple_example.xml instead. The default.xml
is only used to bootstrap the permission before any policy profile is specified. It simple allows for all ros interfaces, yet is more conservative than simple <topic>*</topic>
.
@ruffsl I asked a few days ago, but it appears lost to the internet. Are you trying to land this in the patch release? |
That would be nice. I'd like to get the profile format out there and get some feedback from real users. I'd like to eventually overhaul the security CLI, as I don't think the positional arguments are intuitive, and we should refactor the crypto to use python APIs rather than openssl via subrosses, buth those are out of scope for the current PR. @mjcarroll , did you figure out how to get the CI to use the package.xml for dependency resolution? |
for when generate_policies doesn't necessarily add permissions to a generated profile.
Crystal seems to require permission for parameters though is should be unnecessary
Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>
@ruffsl is this still ready to merge after that force push? Just got the LXML dep in. |
I rebased wrt master, as I misread a remark elsewhere. Suppose I didn't need to, my bad. I can push the previous branch back if you'd like. Nothing's changed though. |
Nope just verifying! Thanks, and sorry that it has taken so long. |
fixing incomplete rebase from #72
fixing incomplete rebase from #72
* Correct sros2 cli test folder location (#83) * Update test folder location fixing incomplete rebase from #72 * Remove old yaml profile examples fixing incomplete rebase from #72 * add reference to schema in generated permission files (#84) Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> * Add missing attributes to test permissions XML file Signed-off-by: Jacob Perron <jacob@openrobotics.org> * fix status print to match commands invoked Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com> * Fix bug preventing generate_policy verb from working with publishers and services Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Add CMake lint test to sros2_cmake (#90) Fixed lint errors accordingly. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
This profile is language agnostic, and transforms can be added for more transports.
TODO: