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

Avoid segmentation fault in __pkcs11h_crypto_mbedtls_certificate_is_issuer #68

Closed
ko-maren opened this issue Sep 17, 2024 · 6 comments
Closed

Comments

@ko-maren
Copy link

I'm using pkcs11-helper v2.28.0 and mbedtls 3.6.0. There I'm observing a segmentation fault occurring in __pkcs11h_crypto_mbedtls_certificate_is_issuer. This segmentation fault comes from mbedtls (see Mbed-TLS/mbedtls#9570), however it could prevented in pkcs11-helper as well/additional.

Instead of using the memset shortly before calling mbedtls_x509_crt_parse, the segmentation fault does not happened if defining x509_issuer and x509_cert at the beginning of the function.

Possible fix in pkcs11-helper:

static
int
__pkcs11h_crypto_mbedtls_certificate_is_issuer (
	IN void * const global_data,
	IN const unsigned char * const issuer_blob,
	IN const size_t issuer_blob_size,
	IN const unsigned char * const cert_blob,
	IN const size_t cert_blob_size
) {
	mbedtls_x509_crt x509_issuer;
	mbedtls_x509_crt x509_cert;
	memset(&x509_issuer, 0, sizeof(x509_issuer));
	memset(&x509_cert, 0, sizeof(x509_cert));

	uint32_t verify_flags = 0;
	PKCS11H_BOOL is_issuer = FALSE;

	(void)global_data;

	/*_PKCS11H_ASSERT (global_data!=NULL); NOT NEEDED*/
	_PKCS11H_ASSERT (issuer_blob!=NULL);
	_PKCS11H_ASSERT (cert_blob!=NULL);

	if (0 != mbedtls_x509_crt_parse (&x509_issuer, issuer_blob, issuer_blob_size)) {
		goto cleanup;
	}

	if (0 != mbedtls_x509_crt_parse (&x509_cert, cert_blob, cert_blob_size)) {
		goto cleanup;
	}

	if ( 0 == mbedtls_x509_crt_verify(&x509_cert, &x509_issuer, NULL, NULL,
		&verify_flags, NULL, NULL )) {
		is_issuer = TRUE;
	}

cleanup:
	mbedtls_x509_crt_free(&x509_cert);
	mbedtls_x509_crt_free(&x509_issuer);

	return is_issuer;
}
@ko-maren
Copy link
Author

Regarding of the response in the MBedTLS issue, mbedtls_x509_crt_init needs to be called before using x509_issuer and x509_cert. So this needs to be adjusted in __pkcs11h_crypto_mbedtls_certificate_is_issuer

@saper
Copy link
Member

saper commented Oct 8, 2024

Do you have some simple code to demonstrate the crash?

@alonbl
Copy link
Member

alonbl commented Oct 9, 2024

Hi @ko-maren,
Somehow I missed this notification.
Can you please test #70 ?
Thanks,
Alon

@ko-maren
Copy link
Author

@alonbl I also missed the notification.
I will check it, and let you know.
Thanks

@ko-maren ko-maren changed the title Avoid sementation fault in __pkcs11h_crypto_mbedtls_certificate_is_issuer Avoid segmentation fault in __pkcs11h_crypto_mbedtls_certificate_is_issuer Oct 22, 2024
@ko-maren
Copy link
Author

LGTM

@alonbl
Copy link
Member

alonbl commented Oct 28, 2024

Thanks!

@alonbl alonbl closed this as completed Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants