Commit 89389bc7a (asn1: Fix long-standing IMPLICIT tagging brokenness)
was incomplete. Removing the hacks in lib/asn1/cms.asn1 revealed this.
Now the ASN.1 compiler generates enums to indicate what is the class and
tag of each type. This is needed so the decoder functions generated by
the compiler can know what tag to restore.
Now, too, the compiler does handle IMPLICIT tags whose encoded length is
different from that of the underlying type.
However, we now don't handle indefinite BER and non-DER definite lengths
(DCE) following IMPLICIT tags. This would affect only CMS in-tree.
This commit _mostly_ fixes the Heimdal ASN.1 compiler to properly
support IMPLICIT tagging in most if not all the many cases where it
didn't already, as you could see in lib/asn1/canthandle.asn1 prior to
this commit.
This fix is a bit of a hack in that a proper fix would change the
function prototypes of the encode/decode/length functions generated by
the compiler to take an optional IMPLICIT tag to tag with instead of the
type they code. That fix would not be localized to lib/asn1/ however,
and would change the API and ABI of generated code (which is mostly not
an ABI for Heimdal, but still, some external projects would have to make
changes).
Instead, for IMPLICIT tags we currently depend on the IMPLICIT tag and
the sub-type's tag having the same size -- this can be fixed with extra
allocation on the encoder side as we do on the decoder side, but we
might leave it for later.
The issue we're fixing manifested as:
-- The [CONTEXT 0] tag in Bar below was turned into an EXPLICIT tag
-- instead of an IMPLICIT one, netting the DER encoding for the `foo`
-- member as:
-- [CONTEXT 0] [UNIVERSAL Seq] [UNIVERSAL Int] <encoding of i>
-- instead of the correct:
-- [CONTEXT 0] [UNIVERSAL Int] <encoding of i>
Foo ::= SEQUENCE { i INTEGER }
Bar ::= SEQUENCE { foo [0] IMPLICIT Foo }
or
Foo ::= INTEGER
Bar ::= SEQUENCE { foo [0] IMPLICIT Foo } -- tag context 0 marked
-- constructed!
I've reviewed this in part by reviewing the output of the compiler
before and after this change using this procedure:
- Run an earlier version of the ASN.1 compiler output for all
modules in lib/asn1/. Save these in a different location.
- Run this (or later) version of the ASN.1 compiler output for
the same modules, adding --original-order for modules that
have been manually sorted already (e.g., rfc2459.asn1).
- Run clang-format on the saved and newest generated C source
and header files.
- Diff the generated output. Substantial differences will
relate to handling of IMPLICIT tagging. These are
particularly evident in the tcg.asn1 module, which uses a lot
of those.
Later commits add test data (certificates with extensions that use
IMPLICIT tagging) taken from external specifications as well, which
exercise this fix.
Non-urgent brokenness yet to be fixed:
- When the IMPLICIT tag and the tag of the underlying type require
differing numbers of bytes to encode, the encoding and decoding will
fail. The prototypes of generated length_*() functions make it
impossible to do much better.
- SET OF <primitive> still crashes the compiler (not a new bug).
Futures:
- Unwind hackery in cms.asn1 that worked around our lack of proper
IMPLICIT tagging support.
Here are some of the generated code deltas one expects to see around
this commit:
$ git checkout $earlier_version
$ ./autogen.sh
$ mkdir build
$ cd build
$ ../configure ...
$ make -j4
$ make check
$ cd lib/asn1
$ for i in *.c; do
[[ $i = asn1parse.? || $i = lex.? || $i = *.h ]] && continue
clang-format -i $i $i
cmp /tmp/save/$i $i && echo NO DIFFS: $i && continue; echo DIFF: $i
done
NO DIFFS: asn1_cms_asn1.c
NO DIFFS: asn1_digest_asn1.c
NO DIFFS: asn1_err.c
NO DIFFS: asn1_krb5_asn1.c
/tmp/save/asn1_kx509_asn1.c asn1_kx509_asn1.c differ: byte 6433, line 264
DIFF: asn1_kx509_asn1.c
NO DIFFS: asn1_ocsp_asn1.c
NO DIFFS: asn1_pkcs10_asn1.c
/tmp/save/asn1_pkcs12_asn1.c asn1_pkcs12_asn1.c differ: byte 12934, line 455
DIFF: asn1_pkcs12_asn1.c
NO DIFFS: asn1_pkcs8_asn1.c
NO DIFFS: asn1_pkcs9_asn1.c
NO DIFFS: asn1_pkinit_asn1.c
/tmp/save/asn1_rfc2459_asn1.c asn1_rfc2459_asn1.c differ: byte 20193, line 532
DIFF: asn1_rfc2459_asn1.c
NO DIFFS: asn1_rfc4043_asn1.c
/tmp/save/asn1_rfc4108_asn1.c asn1_rfc4108_asn1.c differ: byte 595, line 26
DIFF: asn1_rfc4108_asn1.c
/tmp/save/asn1_tcg_asn1.c asn1_tcg_asn1.c differ: byte 31835, line 1229
DIFF: asn1_tcg_asn1.c
/tmp/save/asn1_test_asn1.c asn1_test_asn1.c differ: byte 384, line 21
DIFF: asn1_test_asn1.c
/tmp/save/test_template_asn1-template.c test_template_asn1-template.c differ: byte 650, line 20
DIFF: test_template_asn1-template.c
$
$ cd ../..
$ git checkout $newer_version
$ make -j4 && make check
$ cd lib/asn1
$ for i in *.[ch]; do
[[ $i = asn1parse.? || $i = lex.? || $i = *.h ]] && continue
clang-format -i $i $i
cmp /tmp/save/$i $i && echo NO DIFFS: $i && continue
diff -ubw /tmp/save/$i $i
done | $PAGER
and one should see deltas such as the following:
- a small enhancement to handling of OPTIONAL members:
(data)->macData = calloc(1, sizeof(*(data)->macData));
if ((data)->macData == NULL)
goto fail;
e = decode_PKCS12_MacData(p, len, (data)->macData, &l);
- if (e) {
+ if (e == ASN1_MISSING_FIELD) {
free((data)->macData);
(data)->macData = NULL;
+ } else if (e) {
+ goto fail;
} else {
p += l;
len -= l;
ret += l;
- more complete handling of DEFAULTed members:
e = decode_FWReceiptVersion(p, len, &(data)->version, &l);
- if (e)
+ if (e == ASN1_MISSING_FIELD) {
+ (data)->version = 1;
+ } else if (e) {
goto fail;
- p += l;
- len -= l;
- ret += l;
+ } else {
+ p += l;
+ len -= l;
+ ret += l;
+ }
{
- replacement of tags with implicit tags (encode side):
/* targetUri */
if ((data)->targetUri) {
size_t Top_tag_oldret HEIMDAL_UNUSED_ATTRIBUTE = ret;
ret = 0;
e = encode_URIReference(p, len, (data)->targetUri, &l);
if (e)
return e;
p -= l;
len -= l;
ret += l;
- e = der_put_length_and_tag(p, len, ret, ASN1_C_CONTEXT, PRIM, 4, &l);
+ e = der_replace_tag(p, len, ASN1_C_CONTEXT, CONS, 4);
if (e)
return e;
p -= l;
len -= l;
ret += l;
ret += Top_tag_oldret;
}
- replacement of tags with implicit tags (decode side):
strengthOfFunction_oldlen = len;
if (strengthOfFunction_datalen > len) {
e = ASN1_OVERRUN;
goto fail;
}
len = strengthOfFunction_datalen;
- e = decode_StrengthOfFunction(p, len, (data)->strengthOfFunction, &l);
- if (e)
- goto fail;
- p += l;
- len -= l;
- ret += l;
+ {
+ unsigned char *pcopy;
+ pcopy = calloc(1, len);
+ if (pcopy == 0) {
+ e = ENOMEM;
+ goto fail;
+ }
+ memcpy(pcopy, p, len);
+ e = der_replace_tag(pcopy, len, ASN1_C_UNIV, PRIM, 0);
+ if (e)
+ goto fail;
+ e = decode_StrengthOfFunction(p, len, (data)->strengthOfFunction, &l);
+ if (e)
+ goto fail;
+ p += l;
+ len -= l;
+ ret += l;
+ free(pcopy);
+ }
len = strengthOfFunction_oldlen - strengthOfFunction_datalen;
}
}
{
size_t profileOid_datalen, profileOid_oldlen;
- correct determination of implicit tag constructed vs no for IMPLICT-
tagged named primitive types:
{
size_t profileUri_datalen, profileUri_oldlen;
Der_type profileUri_type;
e = der_match_tag_and_length(p, len, ASN1_C_CONTEXT, &profileUri_type, 2,
&profileUri_datalen, &l);
- if (e == 0 && profileUri_type != PRIM) {
+ if (e == 0 && profileUri_type != CONS) {
e = ASN1_BAD_ID;
}
if (e) {
(data)->profileUri = NULL;
} else {
(data)->profileUri = calloc(1, sizeof(*(data)->profileUri));
if ((data)->profileUri == NULL) {
e = ENOMEM;
goto fail;
}
- correct determination of length of IMPLICT-tagged OIDs:
if ((data)->profileOid) {
size_t Top_tag_oldret = ret;
ret = 0;
ret += der_length_oid((data)->profileOid);
+ ret += 1 + der_length_len(ret);
ret += Top_tag_oldret;
}
These deltas should be examined with the corresponding ASN.1 module at
hand, cross-referencing the source code to the ASN.1 type definitions
and manually applying X.690 rules to double-check the choices of
primitive vs. constructed tag, and the choices of when to replace tags
and when not.
Many external ASN.1 modules that we have imported over time define types
like this:
Foo ::= SEQUENCE { bar Bar }
Bar ::= SEQUENCE { aMember INTEGER }
and before this change one had to re-order the definitions so that the
one for `Bar` came first. No more.
We can now have out of order definitions in ASN.1 modules and the
compiler will topologically sort output C type declarations so that one
no longer has to manually sort types in ASN.1 modules when importing
them.
Besides that, it is now possible to create circular data types using
OPTIONAL since we generate such fields as pointers (which can then be
pointers to incomplete struct declarations):
Circular ::= SEQUENCE {
name UTF8String,
next Circular OPTIONAL
}
Circular types aren't necessarily useful, but they have been used in the
past. E.g., the rpc.mountd protocol uses a circular type as a linked
list -- it should just have used an array, of course, as that's
semantically equivalent but more space efficient in its encoding, but
the point is that such types exist out there.
C enum labels have to be globally unique. ASN.1 module ENUMERATED and
INTEGER types with named values are not globally unique. This means
that ASN.1 integer type value names and enumerations can cause conflicts
when compiled to C.
This new option allows the user to specify a prefix to apply to such
names. Then this:
Foo ::= ENUMERATED { v1 (0) }
can generate:
typedef enum Foo {
prefix_v1 = 0,
} Foo;
instead of
typedef enum Foo {
v1 = 0,
} Foo;
which is very likely to conflict.
TBD: Add option to use the type name as the prefix?
TRUE/FALSE may not be defined, so emitting those symbols when generating
code for `... BOOLEAN DEFAULT TRUE -- or FALSE` causes the generated
code to fail to compile. We could move the definitions of TRUE/FALSE to
krb5-types.h, or maybe we could have an asn1_compile option to force
inclusion of more than one header file so we can have headers defining
such constants. But the simplest fix is to just emit 1/0 instead of
TRUE/FALSE.
This explains why some BOOLEAN DEFAULT usages in PKIX are made OPTIONAL
in Heimdal.
The code generators were shifting "1LU" by (<< 32) and (<< 63) which
are undefined operations for a 32-bit integer. To ensure the integer
is 64-bit use "1ULL".
Change-Id: I062cae5638139a9fe51563f64b1964f87e2f49e3
This commit adds functions for finding OIDs by symbolic name, meaning by
their symbolic names given in the ASN.1 modules that define them.
TBD:
- Resolve OIDs to names.
- Support a file in /etc for additional OID resolution.
- Add support for resolving OID arc names.
the code generated by asn1_compile.exe includes a large number
of unreferenced local variables. The resulting warnings drown
out other potentially more serious warnings.
This change suppresses the C4101 warnings in the generated
source files.
Change-Id: I17642ff427f457c885b1eb0e62436f3bc9057ee1
This reverts commit cb6f7ea40e.
stdint.h can be included everywhere now that the Windows
platform generates and installs a stdint.h when Visual
Studio does not provide one.
Change-Id: Ia3cab28d7f5806203cd45227765debda54ac7472
Looks like they defined basename() in string.h and ntohs/htonl are
implemented in terms of __bswap16() which is a macro with tmp
variables and so one cannot embed one call to ntohs/htons in another.
Not good but we workaround this limitation in glibc.
The source files generated by compile_et and asn1-compile must
begin with:
#ifdef HAVE_CONFIG_H
#include <config.h>
#endif
This permits conditional includes based on HAVE_STDINT_H and
HAVE_UNISTD_H to work.
Change-Id: Iefe25317ac3cb1970793748b8318174bcd7a087f
In most cases stdint.h should be inherited from roken.h.
In those cases where it cannot be, it must be protected by
#ifdef HAVE_STDINT_H
Change-Id: I46cbaeab1d65939468f84179aeeef7e4f898b0bb
Add strtoll()/strtoull() to lib/roken
Add stdint.h to lib/roken (Windows only)
Add logic to detect whether to use lib/roken's stdint.h based on
Visual Studio version
Add include of stdint.h in generated ASN.1 code
Export missing symbols for 64-bit integers in lib/asn1
Export missing symbols for FAST
Add missing sources to kdc/NTMakefile
Fix issue in kuserok
Fix bsearch issues
The 64-bit integer support changed the logic for deciding when an
INTEGER should map to a signed or unsigned 32- or 64-bit integer
type. The upshot is that two places where we had {0, INT_MAX}
ranges needed to be changed to be {0, UINT_MAX}.
We need to tweak the integer type mapping logic to have a bias for
unsigned integer types. Unsigned is better.
ASN.1 INTEGERs will now compile to C int64_t or uint64_t, depending
on whether the constraint ranges include numbers that cannot be
represented in 32-bit ints and whether they include negative
numbers.
Template backend support included. check-template is now built with
--template, so we know we're testing it.
Tests included.
