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

mbedtls_pk_write_pubkey_der expects a mbedtls_pk_context that is non-const #396

Closed
sithglan opened this issue Jan 13, 2016 · 5 comments
Closed
Labels
bug component-crypto Crypto primitives and low-level interfaces

Comments

@sithglan
Copy link

This bug comes from Ray Satiro from the libcurl malilinglist:

  • mbedtls_pk_write_pubkey_der expects a mbedtls_pk_context that is non-const
    [3], but mbedtls_ssl_get_peer_cert returns a const certificate. I don't see a
    way to make a non-const copy of the public key but there is probably a way to
    do this, or copy the cert or some parent container.

Answer from Manuel:

Finally, regarding the fact that mbedtls_pk_write_pubkey_der() expects
a non-const pk_context, I think it is a bug on our side: that function
should work well with a const public key, and I'm afraid we just
forgot the const in the prototype :( So I think it should work if you
just cast the const away. If you want a more correct solution, then
the easiest thing to make a non-const copy of is the certificate, by
calling mbedtls_x509_crt_parse_der() on c->raw.p, c->raw.len if c is
the pointer returned by mbedtls_ssl_get_peer_cert().

References:
https://tls.mbed.org/discussions/generic/resumed-tls-handshake
http://article.gmane.org/gmane.comp.web.curl.library/46767

@ciarmcom
Copy link

ARM Internal Ref: IOTSSL-601

@aaronmdjones
Copy link
Contributor

Bump. I just got bitten by this, and newer versions of Clang (3.8+) will warn even if you have an explicit cast, as it should.

warning: cast from 'const mbedtls_pk_context *' to 'mbedtls_pk_context *' drops const qualifier [-Wcast-qual]
                if ((ret = mbedtls_pk_write_pubkey_der((mbedtls_pk_context *) &certificate->pk, dk, sizeof dk)) < 0)
                                                                              ^
1 warning generated.

Adding const to the function prototype cannot change the ABI, and foo *bar can always be implicitly converted to const foo *bar (when bar is not a typedef to an array type), so it will not break the API either. In other words, you don't have to wait for the release of a new library version to fix this.

@jay
Copy link

jay commented Mar 17, 2017

FYI we worked around this in libcurl by making the copy.
https://github.com/curl/curl/blob/curl-7_53_1/lib/vtls/mbedtls.c#L550-L562

@RonEld
Copy link
Contributor

RonEld commented Sep 13, 2018

#1194 addresses this issue

Unfortunately, since this is an API breaking change, it will probably take time to merge it

@RonEld RonEld added the component-crypto Crypto primitives and low-level interfaces label Feb 17, 2019
@RonEld
Copy link
Contributor

RonEld commented Jun 11, 2019

The structure mbedtls_pk_context is planned to be removed with the introduction of mbed-crypto, therefore closing this issue as it won't be fixed.

@RonEld RonEld closed this as completed Jun 11, 2019
ronald-cron-arm added a commit to ronald-cron-arm/mbedtls that referenced this issue Jul 22, 2022
…5698

Merge of development PRs from v3.1.0 to 5698
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-crypto Crypto primitives and low-level interfaces
Projects
None yet
Development

No branches or pull requests

6 participants