openssl_* module_utils/crypto.py: add full list of OIDs known to current OpenSSL #54943

felixfontein · 2019-04-06T09:50:29Z

SUMMARY

Adds a full list of OIDs known to OpenSSL. Created from https://github.com/openssl/openssl/blob/master/crypto/objects/objects.txt by https://gist.github.com/felixfontein/376748017ad65ead093d56a45a5bf376

This is a large piece over 1050 lines, but then at least we can serve the same names for all items listed independent of the concrete OpenSSL library used by the module.

I hope I've attributed this part correctly. While the numbers and names in this collection are kind of public, the collection itself should be copyrightable.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

lib/ansible/module_utils/crypto.py

ansibot · 2019-04-06T09:58:48Z

cc @MarkusTeufelberger @Shaps @Spredzy @Xyon @puiterwijk @resmo
click here for bot help

lib/ansible/module_utils/crypto.py

MarkusTeufelberger · 2019-04-06T13:34:20Z

lib/ansible/module_utils/crypto.py

+    '0.9.2342.19200300': ('ucl', ),
+    '0.9.2342.19200300.100': ('pilot', ),
+    '0.9.2342.19200300.100.1': ('pilotAttributeType', ),
+    '0.9.2342.19200300.100.1.1': ('userId', 'UID'),


uid number 1

and...

MarkusTeufelberger · 2019-04-06T13:34:39Z

lib/ansible/module_utils/crypto.py

+    '0.9.2342.19200300.100.1.41': ('mobileTelephoneNumber', ),
+    '0.9.2342.19200300.100.1.42': ('pagerTelephoneNumber', ),
+    '0.9.2342.19200300.100.1.43': ('friendlyCountryName', ),
+    '0.9.2342.19200300.100.1.44': ('uniqueIdentifier', 'uid'),


uid number 2. Which one will end up mapping to the OID?

Hmmm. That's unfortunate. I was hoping I can ignore case, to work around the problem that OpenSSL uses userId, while cryptography uses userID (or was it vice versa?). Maybe we should remove the lower() everywhere and manually add the alias userID for userId. What do you think?

Removing lower() (and maybe adding a few mappings by hand) seems like the safer/cleaner option... uid != UID and mail != Mail is already bad enough in OpenSSL's OID mapping...

MarkusTeufelberger · 2019-04-06T13:39:23Z

lib/ansible/module_utils/crypto.py

-    for name in names[1:]:
-        _NORMALIZE_NAMES[name] = names[0]
+    for name in names:
+        _NORMALIZE_NAMES[name.lower()] = names[0]


Maybe something along these lines might be useful?

Suggested change

_NORMALIZE_NAMES[name.lower()] = names[0]

assert name.lower() not in _NORMALIZE_NAMES or _NORMALIZE_NAMES[name.lower()] == names[0]

_NORMALIZE_NAMES[name.lower()] = names[0]

Ah, those just map the shorter names to the longer ones...

Usually yes. If there aren't collisions, such as uid and UID...

There's also another collision: both 1.3.6.1.7 ('Mail') and 0.9.2342.19200300.100.1.3 ('rfc822Mailbox', 'mail') have the lowercase name mail. Besides these two, there are no more collisions.

MarkusTeufelberger · 2019-04-07T16:52:38Z

test/integration/targets/openssl_certificate_info/tasks/main.yml

@@ -27,7 +27,7 @@
      GN: First Name
      title: Chief
      pseudonym: test
-      UID: asdf
+      x500UniqueIdentifier: asdf


That would have been a "UserID" (OID 0.9.2342.19200300.100.1.1) before - I guess this change is intentional?

It was, though I'm not sure whether it is still necessary (or whether that problem was caused by lowercasing). I'll add a commit to revert this, let's see what happens.

Looks like it isn't necessary anymore. I've removed the same change from #54921.

MarkusTeufelberger · 2019-04-07T17:59:08Z

Looks great!

shipit

felixfontein · 2019-04-09T08:22:29Z

The new_module / new_plugin is incorrect; I've just rebased to update to current devel, where a new module was recently merged (and which this PR now also modifies; ansible/ansibullbot#1167).

gundalow

Red Hat legal have suggested two changes.

lib/ansible/module_utils/crypto.py

felixfontein · 2019-04-10T04:24:54Z

ready_for_review

felixfontein · 2019-04-10T10:12:35Z

@MarkusTeufelberger can you take another look? The only things I changed are the copyright notice (according to RedHat legal team wishes) and I rebased and applied the same changes needed for openssl_certificate_info to openssl_csr_info.

MarkusTeufelberger · 2019-04-10T11:20:25Z

🚢 it /

shipit

felixfontein · 2019-04-10T11:25:41Z

YMMD :D

gundalow · 2019-04-10T11:46:06Z

Red Hat legal have given this a +1, so merging

felixfontein · 2019-04-10T11:47:25Z

@MarkusTeufelberger thanks for reviewing!
@gundalow thanks for asking Red Hat legal, and thanks for merging!

felixfontein force-pushed the openssl-oids branch from eff2dcc to 08b28b0 Compare April 6, 2019 10:15

MarkusTeufelberger reviewed Apr 6, 2019

View reviewed changes

lib/ansible/module_utils/crypto.py Outdated Show resolved Hide resolved

MarkusTeufelberger reviewed Apr 6, 2019

View reviewed changes

MarkusTeufelberger reviewed Apr 7, 2019

View reviewed changes

ansibot added the owner_pr This PR is made by the module's maintainer. label Apr 7, 2019

felixfontein added 5 commits April 9, 2019 09:32

UID -> x500UniqueIdentifier

a3841ae

Reference actual version used.

94d686f

Don't normalize to lower-case.

63b4d03

Change test back.

a0cb028

Fix typo.

50f9785

felixfontein force-pushed the openssl-oids branch from 1956b6d to 50f9785 Compare April 9, 2019 07:35

ansibot added new_module This PR includes a new module. new_plugin This PR includes a new plugin. and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html owner_pr This PR is made by the module's maintainer. labels Apr 9, 2019

gundalow requested changes Apr 9, 2019

View reviewed changes

lib/ansible/module_utils/crypto.py Outdated Show resolved Hide resolved

lib/ansible/module_utils/crypto.py Show resolved Hide resolved

ansibot added owner_pr This PR is made by the module's maintainer. and removed new_module This PR includes a new module. labels Apr 9, 2019

Apply changes suggested by RedHat legal.

6b2f345

ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Apr 10, 2019

gundalow approved these changes Apr 10, 2019

View reviewed changes

ansibot added shipit This PR is ready to be merged by Core and removed community_review In order to be merged, this PR must follow the community review workflow. labels Apr 10, 2019

gundalow merged commit c411883 into ansible:devel Apr 10, 2019

felixfontein deleted the openssl-oids branch April 10, 2019 11:46

ansible locked and limited conversation to collaborators Jul 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

openssl_* module_utils/crypto.py: add full list of OIDs known to current OpenSSL #54943

openssl_* module_utils/crypto.py: add full list of OIDs known to current OpenSSL #54943

felixfontein commented Apr 6, 2019

ansibot commented Apr 6, 2019

MarkusTeufelberger Apr 6, 2019

MarkusTeufelberger Apr 6, 2019

felixfontein Apr 6, 2019

MarkusTeufelberger Apr 6, 2019

MarkusTeufelberger Apr 6, 2019

MarkusTeufelberger Apr 6, 2019

felixfontein Apr 6, 2019

felixfontein Apr 6, 2019

MarkusTeufelberger Apr 7, 2019

felixfontein Apr 7, 2019

felixfontein Apr 7, 2019

MarkusTeufelberger commented Apr 7, 2019

felixfontein commented Apr 9, 2019

gundalow left a comment

felixfontein commented Apr 10, 2019

felixfontein commented Apr 10, 2019

MarkusTeufelberger commented Apr 10, 2019

felixfontein commented Apr 10, 2019

gundalow commented Apr 10, 2019

felixfontein commented Apr 10, 2019

	_NORMALIZE_NAMES[name.lower()] = names[0]
	assert name.lower() not in _NORMALIZE_NAMES or _NORMALIZE_NAMES[name.lower()] == names[0]
	_NORMALIZE_NAMES[name.lower()] = names[0]

openssl_* module_utils/crypto.py: add full list of OIDs known to current OpenSSL #54943

openssl_* module_utils/crypto.py: add full list of OIDs known to current OpenSSL #54943

Conversation

felixfontein commented Apr 6, 2019

SUMMARY

ISSUE TYPE

COMPONENT NAME

ansibot commented Apr 6, 2019

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarkusTeufelberger commented Apr 7, 2019

felixfontein commented Apr 9, 2019

gundalow left a comment

Choose a reason for hiding this comment

felixfontein commented Apr 10, 2019

felixfontein commented Apr 10, 2019

MarkusTeufelberger commented Apr 10, 2019

felixfontein commented Apr 10, 2019

gundalow commented Apr 10, 2019

felixfontein commented Apr 10, 2019