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

Add missing mbedtls_ecp_import function since mbedtls_ecp_keypair became opaque #8652

Closed
awakecoding opened this issue Dec 21, 2023 · 2 comments · Fixed by #7815
Closed

Add missing mbedtls_ecp_import function since mbedtls_ecp_keypair became opaque #8652

awakecoding opened this issue Dec 21, 2023 · 2 comments · Fixed by #7815

Comments

@awakecoding
Copy link

awakecoding commented Dec 21, 2023

Suggested enhancement

I am currently trying to migrate code which looks like this from mbedtls 2.x to mbedtls 3.x, which loads an ECDSA key pair from ASN.1 data, and struggling to find a suitable method since the old code would access mbedtls_ecdsa_context members directly, and those are now made opaque. I noticed that mbedtls_ecp_export does exactly what I need... except in the wrong direction: I would need mbedtls_ecp_import instead, like this:

int mbedtls_ecp_import(mbedtls_ecp_keypair *key, const mbedtls_ecp_group *grp,
                       const mbedtls_mpi *d, const mbedtls_ecp_point *Q)
{
    int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;

    if ((ret = mbedtls_ecp_group_copy(&key->grp, grp)) != 0) {
        return ret;
    }

    if ((ret = mbedtls_mpi_copy(&key->d, d)) != 0) {
        return ret;
    }

    if ((ret = mbedtls_ecp_copy(&key->Q, Q)) != 0) {
        return ret;
    }

    return 0;
}

mbedtls_ecdsa_from_keypair could almost work, but I can't find a way to initialize the 'Q' (public key) parameter from input binary data:

/*
 * Set context from an mbedtls_ecp_keypair
 */
int mbedtls_ecdsa_from_keypair( mbedtls_ecdsa_context *ctx, const mbedtls_ecp_keypair *key )
{
    int ret;

    if( ( ret = mbedtls_ecp_group_copy( &ctx->grp, &key->grp ) ) != 0 ||
        ( ret = mbedtls_mpi_copy( &ctx->d, &key->d ) ) != 0 ||
        ( ret = mbedtls_ecp_copy( &ctx->Q, &key->Q ) ) != 0 )
    {
        mbedtls_ecdsa_free( ctx );
    }

    return( ret );
}

The group can be initialized at the same time as private key (d) is loaded into the keypair using mbedtls_ecp_read_key. My understanding is that the public key can be derived from the private key alone, but I could not find a way to initialize Q from external functions this way. As for loading Q, I didn't find the corresponding function.

Justification

This looks like an oversight, and it's the last thing I need to fix to complete my mbedtls 3.x migration. I'm thinking of doing a downstream patch with a simple mbedtls_ecp_import function to get unblocked, unless someone can point me to what I've missed that could be used to migrate the older code. Here's a simplified version of one of the functions affected by this:

int KeyParseEcdsaPkcs8(uint8_t* blob, int blobSize, int groupId, mbedtls_ecdsa_context* ecdsa)
{
    int result = 0;
    uint8_t* p = blob;
    uint8_t* end = blob + blobSize;
    size_t length = 0;
 
    mbedtls_ecdsa_init(ecdsa);
    result = mbedtls_ecp_group_load(&ecdsa.grp, groupId);

    // Enter containing sequence.
    result = mbedtls_asn1_get_tag(&p, end, &length, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE);

    int version; // Get version
    result = mbedtls_asn1_get_int(&p, end, &version);

    if (version != 1)
        return FAILURE; // Unsupported PKCS#8 version
    
    // Get the private key.
    result = mbedtls_asn1_get_tag(&p, end, &length, MBEDTLS_ASN1_OCTET_STRING);
    result = mbedtls_mpi_read_binary(&ecdsa.d, p, length);
    p += length;

    result = mbedtls_asn1_get_tag(&p, end, &length, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_CONTEXT_SPECIFIC | 1);
    end = p + length;

    result = mbedtls_asn1_get_tag(&p, end, &length, MBEDTLS_ASN1_BIT_STRING);
    p++; // Skip the unused bit field of the bit string.
    length--;

    // Read the point which should be a normal point uncompressed format, since the writer of this data is most likely OpenSSH
    result = mbedtls_ecp_point_read_binary(&ecdsa.grp, &ecdsa.Q, p, length);
    
    return SUCCESS;
}
@awakecoding
Copy link
Author

Linking related issues and pull request for mbedlts_ecp_export, this is just the same problem but with a missing mbedtls_ecp_import function:

#4838
#5005
#5642

@gilles-peskine-arm
Copy link
Contributor

This will be resolved by #7815.

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

Successfully merging a pull request may close this issue.

2 participants