Skip to content
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

Decode OIDs from ASN.1 BER for display #2

Merged
merged 2 commits into from Dec 19, 2014
Merged

Decode OIDs from ASN.1 BER for display #2

merged 2 commits into from Dec 19, 2014

Conversation

sigmaris
Copy link
Contributor

The bytes in the gss_OID_desc structure are the ASN.1 BER encoded form of the OID. When displaying the OID we should show the human-readable form of the OID. In my python-gssapi library I added a dependency on pyasn1 just for decoding OIDs, but in the interest of keeping dependencies simple, I wrote a basic decoder for the BER encoded OID data. This makes the repr() of OID objects match the human-readable form:

before:

>>> MechType.kerberos
<OID [42, 134, 72, 134, 247, 18, 1, 2, 2]>

after:

>>> MechType.kerberos
<OID [1, 2, 840, 113554, 1, 2, 2]>

@simo5
Copy link
Contributor

simo5 commented Nov 30, 2014

Shouldn't we show the OID in the canonical form used everywhere which uses dots ?
I guess something like: <OID: 1.2.840.113554.1.2.2>

@sigmaris
Copy link
Contributor Author

Yeah, I have just changed it to use that form:

>>> MechType.kerberos
<OID 1.2.840.113554.1.2.2>

I feel that we should make it possible to construct an OID from a dot-separated string, too. I'd propose changing the elements parameter to the OID constructor to accept a sequence of integers (or maybe a dot-separated string) and to do the BER encoding when constructing the OID. At the moment using the elements parameter would require the caller to do BER encoding themselves which seems odd.

@simo5
Copy link
Contributor

simo5 commented Dec 1, 2014

On Sun, 30 Nov 2014 09:15:17 -0800
Hugh Cole-Baker notifications@github.com wrote:

I feel that we should make it possible to construct an OID from a
dot-separated string, too. I'd propose changing the elements
parameter to the OID constructor to accept a sequence of integers
(or maybe a dot-separated string) and to do the BER encoding when
constructing the OID. At the moment using the elements parameter
would require the caller to do BER encoding themselves which seems
odd.

Makes sense.

@DirectXMan12
Copy link
Member

Can you incorporate #3 (tests for this functionality), please?
Otherwise, generally LGTM. 👍

As for constructing an OID from not-BER-encoded-form, I would create a separate from_asn1 method that took a sequence of integers (or dot-separated string, or both, your choice), and did the BER encoding on that, and then returned the corresponding OID.

I would leave the elements parameter as accepting BER form: it minimizes the code that goes on in cinit, we retain the option of passing BER bytes (in case someone has them from some other source), and it makes it easy to subclass in Python and add extra conversion logic there.

@frozencemetery
Copy link
Member

If we're going to do a lot of ASN.1 parsing, we might want to switch to a dedicated ASN.1 module, but I think given the (relatively) small of what we need to parse, that'd be overkill for now.

I think this is good to have for clarity reasons; please be sure this gets merged when #3 does.

@simo5
Copy link
Contributor

simo5 commented Dec 7, 2014

LGTM

@sigmaris
Copy link
Contributor Author

sigmaris commented Dec 8, 2014

Before merging these changes I wanted to run the tests and check that they actually passed. I'm working on Mac OS X, so I thought I'd use Vagrant to start a Linux VM to run the tests in. What Linux distro is best to use for testing? I was going to use Debian but I see there's no Debian package for Python 3.3, only Python 3.4.

Related to this, would a Vagrantfile be a useful addition to the repository? The Vagrantfile would contain scripts for provisioning a VM with Vagrant for working on the project. If so, I can add one when I've got my VM set up.

This commit causes `repr` for OIDs to display the OID in dotted
form instead of as the raw ASN.1 BER-encoded bytes.

Also-Authored-By: Simo Sorce <simo@redhat.com>
This commit introduces support for constructing OIDs using integers
(instead of having to pass the BER-encoded bytes to the constructor).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants