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

IMPLICIT tags via DER_PACK_ANY fail on "" #17

Closed
vanrein opened this issue Jan 17, 2017 · 4 comments
Closed

IMPLICIT tags via DER_PACK_ANY fail on "" #17

vanrein opened this issue Jan 17, 2017 · 4 comments

Comments

@vanrein
Copy link
Owner

vanrein commented Jan 17, 2017

With the new support for IMPLICIT tags such as used by LDAP, many things now work better. But there are exceptions not yet solved. Take this LDAPMessage for instance,

SEQUENCE (OF): tag 0x30 #12 @0 ^0, Universal, Constructed
  INTEGER: tag 0x02 #1 @2 ^1, Universal, Primitive
     01 == 1
  [APPLICATION 0]: tag 0x60 #7 @5 ^1, Application, Constructed
    INTEGER: tag 0x02 #1 @7 ^2, Universal, Primitive
       03 == 3
    OCTETSTRING: tag 0x04 #0 @10 ^2, Universal, Primitive
    [0]: tag 0x80 #0 @12 ^2, Contextual, Primitive

Note how nothing is contained in the [0] tag on the last line. This is possible when the contained information is elementary, empty and implicitly tagged. This is indeed the case, if we look at the definitions:

       BindRequest ::= [APPLICATION 0] SEQUENCE {
             version                 INTEGER (1 ..  127),
             name                    LDAPDN,
             authentication          AuthenticationChoice }

        AuthenticationChoice ::= CHOICE {
             simple                  [0] OCTET STRING,
                                     -- 1 and 2 reserved
             sasl                    [3] SaslCredentials,
             ...  }

This however, translates to:

#define DER_PACK_rfc4511_BindRequest \
        DER_PACK_ENTER | DER_TAG_APPLICATION(0), \
        DER_PACK_STORE | DER_TAG_INTEGER, \
        DER_PACK_rfc4511_LDAPDN, \
        DER_PACK_rfc4511_AuthenticationChoice, \
        DER_PACK_LEAVE

#define DER_PACK_rfc4511_AuthenticationChoice \
        DER_PACK_CHOICE_BEGIN, \
        DER_PACK_ENTER | DER_TAG_CONTEXT(0), \
        DER_PACK_STORE | DER_PACK_ANY, \
        DER_PACK_LEAVE, \
        DER_PACK_ENTER | DER_TAG_CONTEXT(3), \
        DER_PIMP_rfc4511_SaslCredentials, \
        DER_PACK_LEAVE/* ...ASN.1 extensions... */, \
        DER_PACK_CHOICE_END

The problem lies in the DER_PACK_STORE | DER_PACK_ANY nested within the DER_PACK_ENTER | DER_TAG_CONTEXT(0) of DER_PACK_rfc4511_AuthenticationChoice. This does not seem to capture an empty string (and probably nothing that is not proper DER-encoded data, so header / length / value format.

What needs to be done, is replace these three lines

        DER_PACK_ENTER | DER_TAG_CONTEXT(0), \
        DER_PACK_STORE | DER_PACK_ANY, \
        DER_PACK_LEAVE, \

with the single line

        DER_PACK_STORE | DER_TAG_CONTEXT(0), \

This way, the structure is stored without any attempt to entering it -- which is the intention.

@vanrein vanrein changed the title IMPLICITly tagged DER_PACK_ANY and empty data IMPLICITly tagging through DER_PACK_ANY fails on empty data Jan 17, 2017
@vanrein vanrein changed the title IMPLICITly tagging through DER_PACK_ANY fails on empty data IMPLICIT tags via DER_PACK_ANY fails on "" Jan 17, 2017
@vanrein vanrein changed the title IMPLICIT tags via DER_PACK_ANY fails on "" IMPLICIT tags via DER_PACK_ANY fail on "" Jan 17, 2017
@vanrein
Copy link
Owner Author

vanrein commented Jan 18, 2017

Not completely sure how nested tags in constructions like [0] [1] INTEGER work under IMPLICIT TAGS?!?

Not sure it ever happens (that is, if it is permitted by ASN.1) but our DER_PIMP_ definitions may lead to strange things when they start with [1] and are called with the [0] as their implicit_tag parameter.

Ignoring this for now, who knows if we'll discover a bug due to it... unlikely to ever be used though.

vanrein added a commit that referenced this issue Jan 18, 2017
Not completely sure how nested tags in constructions like [0] [1] INTEGER work under IMPLICIT TAGS?!?

Not sure it ever happens (that is, if it is permitted by ASN.1) but our DER_PIMP_ definitions may lead to strange things when they start with [1] and are called with the [0] as their implicit_tag parameter.

Ignoring this for now, who knows if we'll discover a bug due to it... unlikely to ever be used though.
@vanrein
Copy link
Owner Author

vanrein commented Jan 18, 2017

Solved in 5056727, albeit with the remark above this one.

For what it's worth:

#define DER_PACK_rfc4511_BindRequest \
        DER_PACK_ENTER | DER_TAG_APPLICATION(0), \
        DER_PACK_STORE | DER_TAG_INTEGER, \
        DER_PACK_rfc4511_LDAPDN, \
        DER_PACK_rfc4511_AuthenticationChoice, \
        DER_PACK_LEAVE

#define DER_PACK_rfc4511_AuthenticationChoice \
        DER_PACK_CHOICE_BEGIN, \
        DER_PACK_STORE | DER_TAG_CONTEXT(0), \
        DER_PIMP_rfc4511_SaslCredentials(DER_TAG_CONTEXT(3))/* ...ASN.1 extensio
ns... */, \
        DER_PACK_CHOICE_END

So it looks like the requested change has been made.

@vanrein vanrein closed this as completed Jan 18, 2017
@vanrein vanrein reopened this Jan 18, 2017
@vanrein
Copy link
Owner Author

vanrein commented Jan 18, 2017

An extension to the latter / closing remarks...

In addition, note how the call to DER_PIMP_rfc4511_SaslCredentials is now parameterised with the implicit tag (which is pushed down into the definition) where it is used to replace the DER_PIMP_ definition's outer tag:

#define DER_PIMP_rfc4511_SaslCredentials(implicit_tag) \
        DER_PACK_rfc4511_LDAPString, \
        DER_PACK_OPTIONAL, \
        DER_PACK_STORE | DER_TAG_OCTETSTRING

This, however, is wrong. It lacks the SEQUENCE around these statements, which it should have replaced. Confusing, because we've also made initial code that used the DER_PIMP_ for COMPONENTS OF.

This seems like good grounds to re-open the issue.

vanrein added a commit that referenced this issue Jan 18, 2017
…open,

#17
We now do generate the DER_PIMP_ with surrounding tags replaced.
This means that any use for COMPONENTS OF will need one more def, DER_COMP_
@vanrein
Copy link
Owner Author

vanrein commented Jan 18, 2017

We still generate

#define DER_PACK_rfc4511_AuthenticationChoice \
        DER_PACK_CHOICE_BEGIN, \
        DER_PACK_STORE | DER_TAG_CONTEXT(0), \
        DER_PIMP_rfc4511_SaslCredentials(DER_TAG_CONTEXT(3))/* ...ASN.1 extensions... */, \
        DER_PACK_CHOICE_END

so storing the [0] tagged whatever-it-is, and we still push the implied [3] tag into the DER_PIMP_rfc4511_SaslCredentials, but now we generate a proper wrapper in the latter:

#define DER_PIMP_rfc4511_SaslCredentials(implicit_tag) \
        DER_PACK_ENTER | implicit_tag, \
        DER_PACK_rfc4511_LDAPString, \
        DER_PACK_OPTIONAL, \
        DER_PACK_STORE | DER_TAG_OCTETSTRING, \
        DER_PACK_LEAVE

Nice.

Note that the reference from DER_PACK_ to DER_PIMP_ is where you can see IMPLICIT TAGS. For Kerberos, you won't find that.

Something else worth noting is this distinction:

#define DER_PIMP_rfc4511_AuthenticationChoice(implicit_tag) \
        DER_PACK_ENTER | implicit_tag, \
        DER_PACK_CHOICE_BEGIN, \
        DER_PACK_STORE | DER_TAG_CONTEXT(0), \
        DER_PIMP_rfc4511_SaslCredentials(DER_TAG_CONTEXT(3))/* ...ASN.1 extensions... */, \
        DER_PACK_CHOICE_END, \
        DER_PACK_LEAVE

#define DER_PACK_rfc4511_AuthenticationChoice \
        DER_PACK_CHOICE_BEGIN, \
        DER_PACK_STORE | DER_TAG_CONTEXT(0), \
        DER_PIMP_rfc4511_SaslCredentials(DER_TAG_CONTEXT(3))/* ...ASN.1 extensions... */, \
        DER_PACK_CHOICE_END

Here the DER_PIMP_ has a CHOICE on the outside, which must never take over a tag; this is probably avoided by ASN.1 but in cases where it isn't we generate the extra outside wrapper for the pushed-down implicit_tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant