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

[BUG] MbedTLS implementation cannot load all PAA Certificates #27194

Closed
nozols opened this issue Jun 12, 2023 · 0 comments · Fixed by #27207
Closed

[BUG] MbedTLS implementation cannot load all PAA Certificates #27194

nozols opened this issue Jun 12, 2023 · 0 comments · Fixed by #27207
Assignees
Labels
spec Mismatch between spec and implementation

Comments

@nozols
Copy link

nozols commented Jun 12, 2023

Reproduction steps

Build chip-tool with mbedtls

  1. cd example/chip-tool
  2. gn gen out/build --args='chip_crypto="mbedtls"'
  3. ninja -C out/build
  4. Go the certificate for subjectId 30:A7:FC:6C:D6:FA:5A:CB:82:7F:77:6E:32:62:07:6B:B8:11:E4:29 (found here) and save the pemCert field to a file (make sure to replace \n with newlines).
  5. Convert the certificate to der format
openssl x509 -in <PEM certificate file> -outform DER -out <output file>
  1. Run chip-tool pairing with --paa-trust-store-path set to the folder containing the der certificate.
cd out/build
chip-tool pairing code 12345 1234-567-8901 --paa-trust-store-path <path to certificate folder>
  1. The output will show the following line:
mbedTLS error: ASN1 - ASN1 tag was of an unexpected value

Bug prevalence

Whenever I do this

GitHub hash of the SDK that was being used

40fb7c2

Platform

core

Platform Version(s)

No response

Anything else?

It seems to be that the mbedtls implementation in chip expects the Basic Constraint extension to have a pathLen field to be set, while the matter specification marks this field as optional (see §6.2.2.5 in the v1.1 spec).

The certificate that was used in the reproduction path omits the pathLen field, as can be seen when reading the certificate using the following command:

openssl x509 -in cert.pem -text
...
X509v3 Basic Constraints: critical
                CA:TRUE
...

Certificates that do have the pathLen field set (e.g. D8:93:5A:88:DC:52:53:EA:35:4F:CE:D9:03:CE:D2:F6:2A:5C:AA:FF) can be used with mbedtls and will not show the ASN1 tag error.

@bzbarsky-apple bzbarsky-apple added the spec Mismatch between spec and implementation label Jun 12, 2023
tcarmelveilleux pushed a commit to tcarmelveilleux/connectedhomeip that referenced this issue Jun 13, 2023
Problem:
- Since Matter 1.1, PAAs are allowed to omit the pathlen optional
  field to basic constraints extension. The code did not do that
- Fixes project-chip#27194

Changes in this PR:

- Update all CryptoPAL backends to fix the path length checks
- Added unit tests covering valid and invalid basic constraints
  around path lengths.

Testing done:

- Unit tests pass on both mbedTLS and BoringSSL/OpenSSL CryptoPAL
tcarmelveilleux added a commit that referenced this issue Jun 15, 2023
* Fix PAA pathlen check

Problem:
- Since Matter 1.1, PAAs are allowed to omit the pathlen optional
  field to basic constraints extension. The code did not do that
- Fixes #27194

Changes in this PR:

- Update all CryptoPAL backends to fix the path length checks
- Added unit tests covering valid and invalid basic constraints
  around path lengths.

Testing done:

- Unit tests pass on both mbedTLS and BoringSSL/OpenSSL CryptoPAL

* Restyled by clang-format

* Address review comments

* Address review comments

* Improve logic with help from @bzbarsky-apple

* Restyled by clang-format

---------

Co-authored-by: [email protected] <[email protected]>
Co-authored-by: Restyled.io <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec Mismatch between spec and implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants