Incorrest OER encoding of default BIT STRING value

DanyaFilatov commented 5 years ago

Asn1c produces incorrect OER encoding when the value of the bit string field is equal to it default value. According to the specification, when value of the field is equal to it default value, this field shall be treated as omitted.

For this type:

S1 ::= SEQUENCE  {
    fn INTEGER,
    fb BIT STRING DEFAULT '00'H
}

the S1 { 5, '00'H } is encoded as 80 01 05 00, but shall be 80 00 05

The issue is very urgent and important because out of this the COER encoding of IEEE 1609.2 certificate is invalid, and asn1c cannot be used to generate these certificates.

The actual reason is because the asn1c doesn't generate code for default values for BIT STRING sequence members:

generated code S1.c:

static asn_TYPE_member_t asn_MBR_S1_1[] = {
    { ATF_NOFLAGS, 0, offsetof(struct S1, fn),
        (ASN_TAG_CLASS_CONTEXT | (0 << 2)),
        -1, /* IMPLICIT tag at current level */
        &asn_DEF_NativeInteger,
        0,
        { 0, 0, 0 },
        0, 0, /* No default value */
        "fn"
        },
    { ATF_POINTER, 1, offsetof(struct S1, fb),
        (ASN_TAG_CLASS_CONTEXT | (1 << 2)),
        -1, /* IMPLICIT tag at current level */
        &asn_DEF_BIT_STRING,
        0,
        { 0, 0, 0 },
        0, 0, /* No default value */
        "fb"
        },
};

The 'fb' member definition doesn't have functions to compare and set default values. Providing these function manually, I was managed to solve the issue. The same behavior was found for OCTET STRING.

brchiu commented 5 years ago

@DanyaFilatov ,

You might have an trial with https://github.com/brchiu/asn1c/tree/fix_issue_293_incorrest_OER_encoding_of_default_BIT_STRING_value , if you can do more tests with different length conditions are welcomed.

mouse07410 commented 5 years ago

@DanyaFilatov any feedback?

@velichkov what is your opinion - is the proposed fix safe to integrate into the master branch? If so, should @brchiu create a similar patch for OCTET STRING?

DanyaFilatov commented 5 years ago

For my purposes it works but I didn't have enough time to fully test other lengths, etc.. Anyway it would be great to merge it because potentially partial solution is better than no solution. Would be also great to implement OCTET STRINGS as well.

brchiu commented 5 years ago

With da56723a3279a3cd1d011a79a1119197030d33de, the following example asn.1 module

TEST

DEFINITIONS AUTOMATIC TAGS ::=

BEGIN

S1 ::= SEQUENCE  {
    fn INTEGER,
    fb BIT STRING DEFAULT '00'H,
    fc OCTET STRING DEFAULT 'FFFF'H
}

END

The generated S1 member info is :

static asn_TYPE_member_t asn_MBR_S1_1[] = {
    { ATF_NOFLAGS, 0, offsetof(struct S1, fn),
        (ASN_TAG_CLASS_CONTEXT | (0 << 2)),
        -1, /* IMPLICIT tag at current level */
        &asn_DEF_NativeInteger,
        0,
        { 0, 0, 0 },
        0, 0, /* No default value */
        "fn"
        },
    { ATF_POINTER, 2, offsetof(struct S1, fb),
        (ASN_TAG_CLASS_CONTEXT | (1 << 2)),
        -1, /* IMPLICIT tag at current level */
        &asn_DEF_BIT_STRING,
        0,
        { 0, 0, 0 },
        &asn_DFL_3_cmp, /* Compare DEFAULT "" */
        &asn_DFL_3_set, /* Set DEFAULT "" */
        "fb"
        },
    { ATF_POINTER, 1, offsetof(struct S1, fc),
        (ASN_TAG_CLASS_CONTEXT | (2 << 2)),
        -1, /* IMPLICIT tag at current level */
        &asn_DEF_OCTET_STRING,
        0,
        { 0, 0, 0 },
        &asn_DFL_4_cmp, /* Compare DEFAULT "ÿÿ" */
        &asn_DFL_4_set, /* Set DEFAULT "ÿÿ" */
        "fc"
        },
};

You'd better check whether the default values are correctly set or not.

mouse07410 commented 5 years ago

You'd better check whether the default values are correctly set or not

What is your opinion?

mouse07410 commented 5 years ago

You'd better check whether the default values are correctly set or not.

@brchiu , I did check - and found a problem with APER. In particular - if {BIT,OCTET} STRING DEFAULT ... is specified, but the actual input provides a value different from the default, the default value gets used/displayed (aka, for APER the code incorrectly ignores the provided non-default value). I cannot tell for sure whether it happens when APER output is written, or when it's read, but I suspect it's when it's written...

Here are the files, and the screen log:

TEST

DEFINITIONS AUTOMATIC TAGS ::=

BEGIN

S1 ::= SEQUENCE {
    fn INTEGER,
    fb BIT STRING DEFAULT '01'H,
    fc OCTET STRING DEFAULT 'FFEF'H
}

END
$ cat s1.xer
<S1>
  <fn>45</fn>
</S1>
$ cat s2.xer
<S1>
  <fn>9845</fn>
  <fb>0101</fb>
</S1>
$ ./converter-example -p S1 -ixer -o per s2.xer > s2.uper
$ ./converter-example -p S1 -iper -otext s2.uper
S1 ::= {
    fn: 9845
    fb: 50 (4 bits unused)
    fc: FF EF
}
$ ./converter-example -p S1 -ixer -oaper s2.xer > s2.aper
$ ./converter-example -p S1 -iaper -otext s2.aper
S1 ::= {
    fn: 9845
    fb: 01
    fc: FF EF
}
$

OER and UPER appear fine:

$ ./converter-example -p S1 -ooer -ixer s2.xer > s2.oer
$ ./converter-example -p S1 -oper -ixer s2.xer > s2.uper
$ ./converter-example -p S1 -oder -ixer s2.xer > s2.der
$
$ ./converter-example -p S1 -otext -iper s2.uper
S1 ::= {
    fn: 9845
    fb: 50 (4 bits unused)
    fc: FF EF
}
$ ./converter-example -p S1 -otext -ioer s2.oer
S1 ::= {
    fn: 9845
    fb: 50 (4 bits unused)
    fc: FF EF
}

When both are omitted - all the codecs seem to do the right thing:

$ cat s1.xer
<S1>
  <fn>45</fn>
</S1>
$ ./converter-example -p S1 -ooer -ixer s1.xer > s1.oer
$ ./converter-example -p S1 -oper -ixer s1.xer > s1.uper
$ ./converter-example -p S1 -oaper -ixer s1.xer > s1.aper
$ ./converter-example -p S1 -oder -ixer s1.xer > s1.der
$
$ ./converter-example -p S1 -otext -ioer s1.oer
S1 ::= {
    fn: 45
    fb: 01
    fc: FF EF
}
$ ./converter-example -p S1 -otext -iper s1.uper
S1 ::= {
    fn: 45
    fb: 01
    fc: FF EF
}
$ ./converter-example -p S1 -otext -iaper s1.aper
S1 ::= {
    fn: 45
    fb: 01
    fc: FF EF
}
$ ./converter-example -p S1 -otext -iber s1.der
S1 ::= {
    fn: 45
}

Same thing happens with OCTET STRING DEFAULT xxxx: APER ignores explicitly provided non-default value, and sticks default value instead:

$ cat s3.xer
<S1>
  <fn>9845</fn>
  <fc>DEAD9</fc>
</S1>
$ ./converter-example -p S1 -ooer -ixer s3.xer > s3.oer
$ ./converter-example -p S1 -oper -ixer s3.xer > s3.uper
$ ./converter-example -p S1 -oaper -ixer s3.xer > s3.aper
$ ./converter-example -p S1 -oder -ixer s3.xer > s3.der
$
$ ./converter-example -p S1 -otext -iber s3.der
S1 ::= {
    fn: 9845
    fc: DE AD 90
}
$ ./converter-example -p S1 -otext -ioer s3.oer
S1 ::= {
    fn: 9845
    fb: 01
    fc: DE AD 90
}
$ ./converter-example -p S1 -otext -iper s3.uper
S1 ::= {
    fn: 9845
    fb: 01
    fc: DE AD 90
}
$ ./converter-example -p S1 -otext -iaper s3.aper
S1 ::= {
    fn: 9845
    fb: 01
    fc: FF EF
}

Update Also worth observing:

$ cat s3.xer
<S1>
  <fn>9812</fn>
  <fb>0010111010001</fb>
  <fc>DADDEAD91234</fc>
</S1>
$ ./converter-example -p S1 -ixer -ooer s3.xer > s3.oer
$ ./converter-example -p S1 -ixer -per-nopad -oper s3.xer > s3.per
$ ./converter-example -p S1 -ixer -per-nopad -oaper s3.xer > s3.aper
$ ./converter-example -p S1 -ixer -per-nopad -oder s3.xer > s3.der
$
$ ./converter-example -p S1 -iaper -otext s3.aper 
S1 ::= {
    fn: 9812
    fb: 00
    fc: FF FF
}
$ ./converter-example -p S1 -iper -otext s3.uper 
s3.uper: No such file or directory
$ ./converter-example -p S1 -iper -otext s3.per 
S1 ::= {
    fn: 9812
    fb: 2E 88 (3 bits unused)
    fc: DA DD EA D9 12 34
}
$ ./converter-example -p S1 -ioer -otext s3.oer 
S1 ::= {
    fn: 9812
    fb: 2E 88 (3 bits unused)
    fc: DA DD EA D9 12 34
}
$ ./converter-example -p S1 -iber -otext s3.der 
S1 ::= {
    fn: 9812
    fb: 2E 88 (3 bits unused)
    fc: DA DD EA D9 12 34
}
$ ./converter-example -p S1 -ixer -otext s3.xer 
S1 ::= {
    fn: 9812
    fb: 2E 88 (3 bits unused)
    fc: DA DD EA D9 12 34
}

mouse07410 commented 5 years ago

The proposed PR seems to work fine with OER and UPER, but screws up with APER - sticking default values regardless of whether the field value is or isn't present.

Help debugging this would be appreciated.

@brchiu please feel free to check my fork, as that's what I have applied your PR to.

brchiu commented 5 years ago

adding bf70e3748c5c72dcb114d849174b17d0aafbfed1

I am not able to add this commit to #297 because origin/master does not have APER.

mouse07410 commented 5 years ago

@brchiu, thank you!! I've tested your commit, and it worked fine with APER, as well with UPER and OER. Please feel free to check against my fork, where both of your commits are merged.

Question: does DER respect DEFAULT? If so, we need another commit to fix that too...

Update Or maybe it's OK...? Could you comment please:

$ cat s2.xer
<S1>
  <fn>9845</fn>
  <fb>0101</fb>
</S1>
$ ./converter-example -p S1 -ixer -oder s2.xer > s2.der
$ ./converter-example -p S1 -iber -otext s2.der
S1 ::= {
    fn: 9845
    fb: 50 (4 bits unused)
}
$ ./converter-example -p S1 -iber -oxer s2.der
<S1>
    <fn>9845</fn>
    <fb>
        0101
    </fb>
    <fc>FF EF</fc>
</S1>
$

vlm / asn1c

Incorrest OER encoding of default BIT STRING value #293