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

error.c does not include asn1.h #3328

Closed
davidhuziji opened this issue May 18, 2020 · 7 comments · Fixed by #3350
Closed

error.c does not include asn1.h #3328

davidhuziji opened this issue May 18, 2020 · 7 comments · Fixed by #3350
Assignees
Labels
bug component-platform Portability layer and build scripts

Comments

@davidhuziji
Copy link

Hi all,

I'm redirected from Mbed Crypto repo as it suggests to bring up new issues here. Please gently kick me out if I misunderstood something.

I'm working on a use case in which the memory footprint is very sensitive. Therefore I only uncomment essential options (I thought) in mbed-crypto configuration file.
However, ASN.1 related definitions cannot be found during mbed-crypto build. The error log looks like the code block below.

mbed-crypto/library/error.c:465:22: error: use of undeclared identifier 'MBEDTLS_ERR_ASN1_OUT_OF_DATA'

mbed-crypto/library/error.c:467:22: error: use of undeclared identifier 'MBEDTLS_ERR_ASN1_UNEXPECTED_TAG'

mbed-crypto/library/error.c:469:22: error: use of undeclared identifier 'MBEDTLS_ERR_ASN1_INVALID_LENGTH'

mbed-crypto/library/error.c:471:22: error: use of undeclared identifier 'MBEDTLS_ERR_ASN1_LENGTH_MISMATCH'

mbed-crypto/library/error.c:473:22: error: use of undeclared identifier 'MBEDTLS_ERR_ASN1_INVALID_DATA'

mbed-crypto/library/error.c:475:22: error: use of undeclared identifier 'MBEDTLS_ERR_ASN1_ALLOC_FAILED'

mbed-crypto/library/error.c:477:22: error: use of undeclared identifier 'MBEDTLS_ERR_ASN1_BUF_TOO_SMALL'

It seems that asn1.h is not directly included in error.c. But it is included in other modules whose header files are included in error.c. It looks like asn1.h include depends on other modules.
So I guess another module is expected to be enabled by default when ASN.1 is selected. Right? May I know which modules should be enabled together with ASN.1, by default?

Thanks a lot.

@danh-arm
Copy link
Contributor

Hi @davidhuziji. For questions like this it's best to use the Mbed TLS mailing list: https://lists.trustedfirmware.org/mailman/listinfo/mbed-tls. This issue tracker is mainly for reporting bugs and possible enhancements. Regards, Dan.

@davidhuziji
Copy link
Author

Thx a lot, Dan! Thx for saving this orphan thread. :)

@gilles-peskine-arm
Copy link
Contributor

That's actually a bug. ASN.1 doesn't depend on anything, so it should be possible to build the library with it and nothing else. We won't fix it in Mbed Crypto, which is no longer maintained, but it's a bug in Mbed TLS as well.

We probably never noticed before because if you include ASN.1, you probably include something else that uses it and that will pull in asn1.h indirectly: RSA, ECDSA, X.509. So check your configuration, it's likely that either you're including ASN.1 when you don't need to or you aren't including something you need.

If you write your own config.h, you should #include "mbedtks/check_config.h". It will #error out if a module is missing one of its dependencies.

@gilles-peskine-arm gilles-peskine-arm added bug component-platform Portability layer and build scripts help-wanted This issue is not being actively worked on, but PRs welcome. labels May 22, 2020
@gilles-peskine-arm gilles-peskine-arm changed the title ASN.1 macros reported as undeclared identifiers during Mbed Crypto build error.c does not include asn1.h May 22, 2020
@davidhuziji
Copy link
Author

davidhuziji commented May 25, 2020

Thx @gilles-peskine-arm. I'm building up a minimal set of configurations to support TLS with ECDHE_ECDSA and therefore ASN.1 is selected.

It seems that asn1.h is only included in header files of PKCS#5, PKCS#12 and OID. But I haven't enable any of the three in Mbed Crypto.

Mbed Crypto is running in secure world in my use case. I'd prefer to put Crypto stuff as little as possible to optimize the secure binary size. If a feature belongs to common protocol level, I'd enable it in mbedTLS in non-secure world.

But due to my limited knowledge of TLS, I'm not sure whether the 3 configs (PKCS#5, PKCS#12, OID) should be enabled by default in Mbed Crypto if ECDHE_ECDSA is selected for TLS. Or should I rely on those 3 configs to get include of asn1.h?

@gilles-peskine-arm
Copy link
Contributor

You probably don't need PKCS#5 or PKCS#12, those are for loading encrypted key files. You do need OID for X.509, but that would be in the normal world for you.

You do need ASN1_PARSE_C for ECDSA_C, but the dependency is only internal, so ecdsa.h doesn't include asn1.h. And anyway ecdsa.h is not included in error.c because it doesn't define its own error codes.

@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented May 25, 2020

git blame shows that scripts/generate_errors.pl explicitly excludes asn1.h, and has done so from the start (9d78140). That's because there was no asn1.h then.

I think the fix is to delete the line $include_name = "" if ($include_name eq "asn1"); from scripts/generate_errors.pl and run scripts/generate_errors.pl. As a temporary workaround, you can do that, or add #include "mbedtls/asn1.h" to error.c manually.

@gilles-peskine-arm gilles-peskine-arm removed the help-wanted This issue is not being actively worked on, but PRs welcome. label May 25, 2020
@gilles-peskine-arm gilles-peskine-arm self-assigned this May 25, 2020
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue May 25, 2020
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@davidhuziji
Copy link
Author

Thx! @gilles-peskine-arm

gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue May 28, 2020
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue May 28, 2020
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-platform Portability layer and build scripts
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants