Error storing a new trust anchor
Open
Error storing a new trust anchor
When I store a new trust anchor, I sometimes receive an error using the same PEM encoded certificate.
I call "OCPki.addTrustAnchor" method with a PEM encoded certificate and it works but it sometimes returns an error using the same PEM certificate.
DEBUG: ../../security/oc_pki.c <pki_add_trust_anchor:265>: attempting to add a trust anchor
ERROR: ../../security/oc_pki.c <pki_add_trust_anchor:281>: could not parse the provided trust anchor: -8576
I have attached the PEM encoded certificate and the log file for a success storing and the PEM encoded certificate and the log file for a failed storing.
certificate_error.log
certificate_success.log
store_certificate_error.log
store_certificate_success.log
Drop or upload designs to attach
Linked issues
0
Trishia Yun
Trishia Yun @Trishia · 7 months ago
Developer
@KISHEN
I have witnessed same issue related to pki certificates installing mfgcert, trust anchor and so on. I guess this is because of these code on several functions in oc_pki.c : if (cert[cert_size - 1] != '\0') { cert_size += 1; } which is expending byte array to cert_size + 1 without array rellocation/re-sizing. So sometimes cert[cert_size-1] after cert_size += 1 can be wrong address's data (i.e. 0x4a ...) even if you are expecting '\0'. It makes parsing error in mbedtls_x509_crt_parse().
In many case of apps on android or java, the last data of byte array in pem cert is commonly '\r'(0x0a) when apps read a pem file.
To avoid this problem, apps should add '\0' via resizing byte array but I think it should be handled or resized in oc_pki.c
pki_add_trust_anchor(size_t device, const unsigned char *cert, size_t cert_size,
oc_sec_credusage_t credusage)
{
OC_DBG("attempting to add a trust anchor");
mbedtls_x509_crt cert1, cert2;
mbedtls_x509_crt_init(&cert1);
size_t c_size = cert_size;
/* Parse root cert */
if (oc_certs_is_PEM((const unsigned char *)cert, cert_size) != 0) {
OC_ERR("provided cert is not in PEM format");
return -1;
}
if (cert[cert_size - 1] != '\0') {
c_size += 1;
}
int ret = mbedtls_x509_crt_parse(&cert1, (const unsigned char *)cert, c_size);
if (ret < 0) {
OC_ERR("could not parse the provided trust anchor: %d", ret);
return -1;
}
OC_DBG("parsed the provided trust anchor");
/* Pass through all known trust anchors looking for a match */
oc_sec_creds_t *creds = oc_sec_get_creds(device);
oc_sec_cred_t *c = oc_list_head(creds->creds);
for (; c != NULL; c = c->next) {
if (c->credusage != credusage) {
continue;
}
mbedtls_x509_crt_init(&cert2);
ret = mbedtls_x509_crt_parse(
&cert2, (const unsigned char *)oc_string(c->publicdata.data),
oc_string_len(c->publicdata.data) + 1);
if (ret < 0) {
OC_ERR("could not parse stored certificate: %d", ret);
mbedtls_x509_crt_free(&cert2);
continue;
}
mbedtls_x509_crt *trustca = &cert2;
for (; trustca != NULL; trustca = trustca->next) {
if (trustca->raw.len == cert1.raw.len &&
memcmp(trustca->raw.p, cert1.raw.p, cert1.raw.len) == 0) {
break;
}
}
mbedtls_x509_crt_free(&cert2);
if (trustca) {
mbedtls_x509_crt_free(&cert1);
OC_DBG("found trust anchor in cred with credid %d", c->credid);
return c->credid;
}
}
OC_DBG("adding a new trust anchor entry to /oic/sec/cred");
ret = oc_sec_add_new_cred(device, false, NULL, -1, OC_CREDTYPE_CERT,
credusage, "*", 0, 0, NULL, OC_ENCODING_PEM,
c_size - 1, (const uint8_t *)cert, NULL, NULL);
if (ret != -1) {
OC_DBG("added new trust anchor entry to /oic/sec/cred");
oc_sec_dump_cred(device);
} else {
OC_ERR("could not add trust anchor entry to /oic/sec/cred");
}
mbedtls_x509_crt_free(&cert1);
return ret;
}
Kishen Maloor
Kishen Maloor @KISHEN · 7 months ago
Maintainer
@Trishia
Yes, mbedTLS expects a string with the certificate(s) that is terminated by a NULL character. The underlying string matching functions it internally uses to locate the certificate header/footer stop at a NULL. So, I think they've just set a terminating NULL character convention for the input to their parse API to prevent out-of-bounds access. As such, I'd prefer to follow that same convention for input to the oc_pki APIs as well rather than reallocating/resizing the provided input string to add a NULL character. We should clarify this in the API documentation.
Trishia Yun
Trishia Yun @Trishia · 7 months ago
Developer
@KISHEN
Ok I got it. Thank you for your clear explanation.
But these code if (cert[cert_size - 1] != '\0') { cert_size += 1; } in oc_pki.c makes a lot of confusion to app developer who missed Null character at the end of byte array for pem data. That code works well accidentally but sometimes it fails. So I think you'd better remove that code for more clear convention if it is not essential code. In my case, that issue makes me confusion for a long time because there is no sample code to get byte data added null character on android sample apps.
Kishen Maloor
Kishen Maloor @KISHEN · 7 months ago
Maintainer
@Trishia
But these code if (cert[cert_size - 1] != '\0') { cert_size += 1; } in oc_pki.c makes a lot of confusion to app developer
Ideally, the API user shouldn't have to look inside the API implementation to know how to use it :). I think we should clarify this detail (i.e. to include a terminating NULL byte) in the API docs for oc_pki APIs.
As for why those lines you referred to exist, it is because mbedTLS's parse API needs the length of the cert + 1 (for the NULL byte). If a C app passes in the cert as a C string and happens to send strlen(cert) as the cert length to the API, then this line bumps up that length before handing the string to mbedTLS. That's all. The API implementation still requires that the cert string have a NULL byte at the end.
Diego Bartolome
Diego Bartolome @dbartolome · 7 months ago
Developer
@miguel has also verified the certificate in OTGC (by printing it) before calling the API and the content is exactly the same, when the operation fails and when goes well.
Kishen Maloor
Kishen Maloor @KISHEN · 7 months ago
Maintainer
@dbartolome @miguel
If you find an error log that looks different than the one above, please post it in this issue. The error log documented above is returned by mbedTLS.
ERROR: ../../security/oc_pki.c <pki_add_trust_anchor:281>: could not parse the provided trust anchor: -8576
Wouter van der Beek
Wouter van der Beek @wouter · 27 minutes ago
Developer
solution: provide javadoc info that a cert needs to be zero terminated in java before calling the C libarary.