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

Incorrest OER encoding of default BIT STRING value #293

Open
DanyaFilatov opened this issue Sep 10, 2018 · 9 comments
Open

Incorrest OER encoding of default BIT STRING value #293

DanyaFilatov opened this issue Sep 10, 2018 · 9 comments

Comments

@DanyaFilatov
Copy link
Contributor

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
Copy link
Contributor

brchiu commented Sep 11, 2018

@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
Copy link

@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
Copy link
Contributor Author

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.

mouse07410 added a commit to mouse07410/asn1c that referenced this issue Sep 24, 2018
@brchiu
Copy link
Contributor

brchiu commented Sep 27, 2018

With da56723, 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
Copy link

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

What is your opinion?

@mouse07410
Copy link

mouse07410 commented Sep 29, 2018

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
Copy link

mouse07410 commented Sep 30, 2018

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
Copy link
Contributor

brchiu commented Sep 30, 2018

adding bf70e37

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

mouse07410 added a commit to mouse07410/asn1c that referenced this issue Sep 30, 2018
@mouse07410
Copy link

mouse07410 commented Sep 30, 2018

@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>
$ 

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

3 participants