I am glad to hear that.
Regards,
Tomas Mraz, OpenSSL
On Fri, 2022-09-30 at 17:18 +0000, GonzalezVillalobos, Diego wrote:
> [AMD Official Use Only - General]
>
> Hello Tomas,
>
> There was a logic error in my code, I did not realize that the first
> iteration of the verification was supposed to fail. The verification
> is working correctly! I apologize for my last response. I really
> appreciate all your help!
>
> Thank you very much,
>
> Diego Gonzalez
> ---------------------------------------------------------------------
> -------------------------------------------------------------
>
>
> -----Original Message-----
> From: Tomas Mraz <tomas@xxxxxxxxxxx>
> Sent: Friday, September 30, 2022 1:22 AM
> To: GonzalezVillalobos, Diego <Diego.GonzalezVillalobos@xxxxxxx>;
> openssl-users@xxxxxxxxxxx
> Subject: Re: Updating RSA public key generation and signature
> verification from 1.1.1 to 3.0
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> Hi,
>
> unfortunately I do not see anything wrong with the code. Does the
> EVP_DigestVerifyFinal return 0 or negative value? I do not think this
> is a bug in OpenSSL as this API is thoroughly tested and it is highly
> improbable that there would be a bug in the ECDSA verification
> through this API.
>
> I am currently out of ideas on what could be wrong or how to
> investigate further. Perhaps someone else can chime in on what can be
> wrong?
>
> Tomas
>
> On Thu, 2022-09-29 at 19:22 +0000, GonzalezVillalobos, Diego wrote:
> > [AMD Official Use Only - General]
> >
> > Hello Tomas,
> >
> > So, I made sure that px_size and py_size are equal to the group
> > order
> > (48). I was able to verify successfully using our previous method
> > (deprecated) with the new key generation method, but I'm still not
> > able to get the digestverify to work successfully. As a reminder
> > this
> > is how we were verifying before:
> >
> > // Determine if SHA_TYPE is 256 bit or 384 bit if
> > (parent_cert->pub_key_algo == SEV_SIG_ALGO_RSA_SHA256 ||
> > parent_cert->pub_key_algo == SEV_SIG_ALGO_ECDSA_SHA256
> > ||parent_cert-
> > > pub_key_algo == SEV_SIG_ALGO_ECDH_SHA256)
> > {
> > sha_type = SHA_TYPE_256;
> > sha_digest = sha_digest_256;
> > sha_length = sizeof(hmac_sha_256);
> > }
> > else if (parent_cert->pub_key_algo == SEV_SIG_ALGO_RSA_SHA384 ||
> > parent_cert->pub_key_algo == SEV_SIG_ALGO_ECDSA_SHA384 ||
> > parent_cert->pub_key_algo == SEV_SIG_ALGO_ECDH_SHA384)
> > {
> > sha_type = SHA_TYPE_384;
> > sha_digest = sha_digest_384;
> > sha_length = sizeof(hmac_sha_512);
> > }
> > else
> > {
> > break;
> > }
> >
> > // 1. SHA256 hash the cert from Version through pub_key
> > parameters
> > // Calculate the digest of the input message rsa.c ->
> > rsa_pss_verify_msg()
> > // SHA256/SHA384 hash the cert from the [Version:pub_key]
> > params
> > uint32_t pub_key_offset = offsetof(sev_cert, sig_1_usage);
> > //
> > 16 + sizeof(SEV_PUBKEY)
> > if (!digest_sha((uint8_t *)child_cert, pub_key_offset,
> > sha_digest, sha_length, sha_type)) {
> > break;
> > }
> > if ((parent_cert->pub_key_algo == SEV_SIG_ALGO_ECDSA_SHA256) ||
> > (parent_cert->pub_key_algo ==
> > SEV_SIG_ALGO_ECDSA_SHA384) ||
> > (parent_cert->pub_key_algo ==
> > SEV_SIG_ALGO_ECDH_SHA256) ||
> > (parent_cert->pub_key_algo ==
> > SEV_SIG_ALGO_ECDH_SHA384)) { // ecdsa.c -> sign_verify_msg
> > ECDSA_SIG *tmp_ecdsa_sig = ECDSA_SIG_new();
> > BIGNUM *r_big_num = BN_new();
> > BIGNUM *s_big_num = BN_new();
> >
> > // Store the x and y components as separate BIGNUM
> > objects. The values in the
> > // SEV certificate are little-endian, must reverse
> > bytes before storing in BIGNUM
> > r_big_num = BN_lebin2bn(cert_sig[i].ecdsa.r,
> > sizeof(sev_ecdsa_sig::r), r_big_num); // LE to BE
> > s_big_num = BN_lebin2bn(cert_sig[i].ecdsa.s,
> > sizeof(sev_ecdsa_sig::s), s_big_num);
> >
> > // Calling ECDSA_SIG_set0() transfers the memory
> > management of the values to
> > // the ECDSA_SIG object, and therefore the values
> > that
> > have been passed
> > // in should not be freed directly after this
> > function
> > has been called
> > if (ECDSA_SIG_set0(tmp_ecdsa_sig, r_big_num,
> > s_big_num) != 1) {
> > BN_free(s_big_num); // Frees
> > BIGNUMs manually here
> > BN_free(r_big_num);
> > ECDSA_SIG_free(tmp_ecdsa_sig);
> > continue;
> > }
> > EC_KEY *tmp_ec_key =
> > EVP_PKEY_get1_EC_KEY(parent_signing_key); // Make a local key so
> > you
> > can free it later
> > if (ECDSA_do_verify(sha_digest,
> > (uint32_t)sha_length,
> > tmp_ecdsa_sig, tmp_ec_key) != 1) {
> > EC_KEY_free(tmp_ec_key);
> > ECDSA_SIG_free(tmp_ecdsa_sig); // Frees
> > BIGNUMs too
> > continue;
> > }
> >
> > found_match = true;
> > EC_KEY_free(tmp_ec_key);
> > ECDSA_SIG_free(tmp_ecdsa_sig); // Frees
> > BIGNUMs
> > too
> > break;
> > }
> >
> >
> > Digest sha function:
> > bool digest_sha(const void *msg, size_t msg_len, uint8_t *digest,
> > size_t digest_len, SHA_TYPE sha_type) {
> > bool ret = false;
> >
> > do { //TODO 384 vs 512 is all a mess
> > if ((sha_type == SHA_TYPE_256 && digest_len !=
> > SHA256_DIGEST_LENGTH)/* ||
> > (sha_type == SHA_TYPE_384 && digest_len !=
> > SHA384_DIGEST_LENGTH)*/)
> > break;
> >
> > if (sha_type == SHA_TYPE_256) {
> > SHA256_CTX context;
> >
> > if (SHA256_Init(&context) != 1)
> > break;
> > if (SHA256_Update(&context, (void *)msg, msg_len) != 1)
> > break;
> > if (SHA256_Final(digest, &context) != 1)
> > break;
> > }
> > else if (sha_type == SHA_TYPE_384) {
> > SHA512_CTX context;
> >
> > if (SHA384_Init(&context) != 1)
> > break;
> > if (SHA384_Update(&context, (void *)msg, msg_len) != 1)
> > break;
> > if (SHA384_Final(digest, &context) != 1)
> > break;
> > }
> >
> > ret = true;
> > } while (0);
> >
> > return ret;
> > }
> >
> > This works using the new EC EVP key generation.
> > The current verification method keeps failing:
> >
> > if ((parent_cert->pub_key_algo == SEV_SIG_ALGO_ECDSA_SHA256) ||
> > (parent_cert->pub_key_algo ==
> > SEV_SIG_ALGO_ECDSA_SHA384) ||
> > (parent_cert->pub_key_algo ==
> > SEV_SIG_ALGO_ECDH_SHA256) ||
> > (parent_cert->pub_key_algo ==
> > SEV_SIG_ALGO_ECDH_SHA384)) { // ecdsa.c -> sign_verify_msg
> >
> > ECDSA_SIG *tmp_ecdsa_sig = ECDSA_SIG_new();
> > BIGNUM *r_big_num = BN_new();
> > BIGNUM *s_big_num = BN_new();
> > uint32_t sig_len;
> > unsigned char* der_sig = NULL;;
> >
> > // Store the x and y components as separate BIGNUM
> > objects. The values in the
> > // SEV certificate are little-endian, must reverse
> > bytes before storing in BIGNUM
> > r_big_num = BN_lebin2bn(cert_sig[i].ecdsa.r,
> > sizeof(sev_ecdsa_sig::r), r_big_num); // LE to BE
> > s_big_num = BN_lebin2bn(cert_sig[i].ecdsa.s,
> > sizeof(sev_ecdsa_sig::s), s_big_num);
> >
> > // Calling ECDSA_SIG_set0() transfers the memory
> > management of the values to
> > // the ECDSA_SIG object, and therefore the values
> > that
> > have been passed
> > // in should not be freed directly after this
> > function
> > has been called
> > if (ECDSA_SIG_set0(tmp_ecdsa_sig,
> > r_big_num,s_big_num) != 1) {
> > BN_free(s_big_num); // FreesBIGNUMs manually
> > here
> > BN_free(r_big_num);
> > ECDSA_SIG_free(tmp_ecdsa_sig);
> > break;
> > }
> >
> > int der_sig_len = i2d_ECDSA_SIG(tmp_ecdsa_sig,
> > &der_sig);
> > // der_sig = static_cast<unsigned
> > char*>(OPENSSL_malloc(der_sig_len));
> > // unsigned char* der_iter = der_sig;
> > // der_sig_len = i2d_ECDSA_SIG(tmp_ecdsa_sig,
> > &der_iter); // <= bugfix here
> >
> >
> > if (der_sig_len == 0) {
> > cout << "sig length invalid" << endl;
> > break;
> > }
> >
> > if (der_sig == NULL) {
> > cout << "sig generation failed" << endl;
> > break;
> > }
> >
> > // loop through the array elements
> > for (size_t i = 0; i < der_sig_len; i++) {
> > cout << der_sig[i] << ' ';
> > }
> >
> > verify_md_ctx = EVP_MD_CTX_new();
> >
> >
> > if (!verify_md_ctx) {
> > cout << "Error md verify context " << endl;;
> > break;
> > }
> >
> > if (EVP_DigestVerifyInit(verify_md_ctx, NULL,
> > (parent_cert->pub_key_algo == SEV_SIG_ALGO_ECDSA_SHA256 ||
> > parent_cert->pub_key_algo == SEV_SIG_ALGO_ECDH_SHA256) ?
> > EVP_sha256(): EVP_sha384(), NULL, parent_signing_key) <= 0) {
> > cout << "Init fails " << endl;
> > break;
> > }
> >
> > if (EVP_DigestVerifyUpdate(verify_md_ctx, (uint8_t
> > *)child_cert, pub_key_offset) <= 0){ // Calls SHA256_UPDATE
> > cout << "updating digest fails" << endl;
> > break;
> > }
> >
> > int ret = EVP_DigestVerifyFinal(verify_md_ctx,
> > der_sig, der_sig_len);
> > if (ret == 0) {
> > cout << "EC Verify digest fails" << endl;
> > break;
> > } else if (ret < 0) {
> > printf("Failed Final Verify
> > %s\n",ERR_error_string(ERR_get_error(),NULL));
> > cout << "EC Verify error" << endl;
> > break;
> > }
> >
> > found_match = true;
> > cout << "SEV EC verification Succesful" << endl;
> >
> > if (verify_md_ctx)
> > EVP_MD_CTX_free(verify_md_ctx);
> >
> > break;
> > }
> >
> > The only difference still is using the der signature; besides that,
> > it
> > is the same. Could it be a bug?
> >
> > Thank you,
> >
> > Diego Gonzalez
> > -------------------------------------------------------------------
> > --
> > -------------------------------------------------------------
> >
> >
> > -----Original Message-----
> > From: Tomas Mraz <tomas@xxxxxxxxxxx>
> > Sent: Thursday, September 29, 2022 1:12 AM
> > To: GonzalezVillalobos, Diego <Diego.GonzalezVillalobos@xxxxxxx>;
> > openssl-users@xxxxxxxxxxx
> > Subject: Re: Updating RSA public key generation and signature
> > verification from 1.1.1 to 3.0
> >
> > Caution: This message originated from an External Source. Use
> > proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > Hi,
> >
> > comments below.
> >
> > On Wed, 2022-09-28 at 22:12 +0000, GonzalezVillalobos, Diego wrote:
> > > [AMD Official Use Only - General]
> > >
> > > Hello Tomas,
> > >
> > > I generated the key as you suggested, and I am no longer getting
> > > an
> > > error message! Thank you for that. Here is how I'm generating the
> > > key
> > > now:
> > >
> > > // SEV certificate are little-endian, must reverse bytes before
> > > generating key
> > > if ((cert->pub_key_algo == SEV_SIG_ALGO_ECDSA_SHA256)
> > > > >
> > > (cert->pub_key_algo ==
> > > SEV_SIG_ALGO_ECDSA_SHA384)) {
> > > //Grab x param and flip bytes to BE
> > > memcpy(px, &cert->pub_key.ecdsa.qx,
> > > ec_group_order);
> > > if(!sev::reverse_bytes(px, sizeof(px)))
> > > break;
> > > //Grab y param and flip bytes to BE
> > > memcpy(py, &cert->pub_key.ecdsa.qy,
> > > ec_group_order);
> > > if(!sev::reverse_bytes(py, sizeof(py)))
> > > break;
> > > }
> > > else if ((cert->pub_key_algo ==
> > > SEV_SIG_ALGO_ECDH_SHA256) ||
> > > (cert->pub_key_algo ==
> > > SEV_SIG_ALGO_ECDH_SHA384))
> > > {
> > > //Grab x param and flip bytes to BE
> > > memcpy(px, &cert->pub_key.ecdh.qx,
> > > ec_group_order);
> > > if(!sev::reverse_bytes(px, sizeof(px)))
> > > break;
> > > //Grab y param and flip bytes to BE
> > > memcpy(py, &cert->pub_key.ecdh.qy,
> > > ec_group_order);
> > > if(!sev::reverse_bytes(py, sizeof(py)))
> > > break;
> > > }
> > >
> > > int px_size = sizeof(px)/sizeof(*px);
> > > int py_size = sizeof(py)/sizeof(*py);
> > >
> > > // Will contain x and y components
> > > unsigned char public_key_xy[1 + px_size + py_size] =
> > > {0};
> > >
> > > //Add point conversion as first value
> > > public_key_xy[0] = POINT_CONVERSION_UNCOMPRESSED;
> > >
> > > //Add x components after point compression
> > > memcpy(public_key_xy + 1, px, px_size);
> > > //Add y components after x
> > > memcpy(public_key_xy + px_size + 1, py ,py_size);
> > >
> > > // int nid = EC_curve_nist2nid("P-384"); //
> > > NID_secp384r1
> > >
> > > OSSL_PARAM_BLD *params_build = OSSL_PARAM_BLD_new();
> > >
> > > if ( params_build == NULL ) {
> > > cout << "Params build fails" << endl;
> > > break;
> > > }
> > >
> > > if (!OSSL_PARAM_BLD_push_utf8_string(params_build,
> > > OSSL_PKEY_PARAM_GROUP_NAME, "P-384", 0)) {
> > > cout<< "Push EC curve to build fails" << endl;
> > > break;
> > > }
> > >
> > > if (!OSSL_PARAM_BLD_push_octet_string(params_build,
> > > OSSL_PKEY_PARAM_PUB_KEY, public_key_xy, sizeof(public_key_xy))) {
> > > cout << "Error: failed to push qx into param
> > > build."
> > > << endl;
> > > break;
> > > }
> > >
> > > OSSL_PARAM *params =
> > > OSSL_PARAM_BLD_to_param(params_build);
> > >
> > > if ( params == NULL ) {
> > > cout << "Error: building parameters." << endl;
> > > break;
> > > }
> > >
> > > OSSL_PARAM_BLD_free(params_build);
> > >
> > > key_gen_ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_EC, NULL);
> > >
> > > if(EVP_PKEY_fromdata_init(key_gen_ctx) != 1) {
> > > cout << "failed to initialize key creation." <<
> > > endl;
> > > break;
> > > }
> > >
> > > if(EVP_PKEY_fromdata(key_gen_ctx, &evp_pub_key,
> > > EVP_PKEY_PUBLIC_KEY, params) != 1) {
> > > cout << "key generation breaks" << endl;
> > > printf("Failed Final Verify
> > > %s\n",ERR_error_string(ERR_get_error(),NULL));
> > > break;
> > > }
> > >
> > > if (EVP_PKEY_get_base_id(evp_pub_key) != EVP_PKEY_EC)
> > > {
> > > cout << "wrong key type" << endl;
> > > break;
> > > }
> > > }
> > >
> > > if (!evp_pub_key) {
> > > cout << "no evp pkey" << endl;
> > > break;
> > > }
> > > cout << "compile key succesful" << endl;
> > > cmd_ret = STATUS_SUCCESS;
> > >
> > > Although the key generation works and I'm not getting a verify
> > > error
> > > anymore, I am still unsuccessful on verifying the digest. It
> > > keeps
> > > failing (returning 0). Here is how I'm currently trying to do the
> > > verification.
> >
> > Are you sure the px_size and py_size is equal to the group order?
> > The
> > x and y values must be padded to the group order with 0 (at the
> > start
> > because the values need to be BE).
> >
> > > ECDSA_SIG *tmp_ecdsa_sig = ECDSA_SIG_new();
> > > BIGNUM *r_big_num = BN_new();
> > > BIGNUM *s_big_num = BN_new();
> > > uint32_t sig_len;
> > > unsigned char* der_sig;
> > >
> > > // Store the x and y components as separate
> > > BIGNUM
> > > objects. The values in the
> > > // SEV certificate are little-endian, must
> > > reverse
> > > bytes before storing in BIGNUM
> > > r_big_num = BN_lebin2bn(cert_sig[i].ecdsa.r,
> > > sizeof(sev_ecdsa_sig::r), r_big_num); // LE to BE
> > > s_big_num = BN_lebin2bn(cert_sig[i].ecdsa.s,
> > > sizeof(sev_ecdsa_sig::s), s_big_num);
> > >
> > > // Calling ECDSA_SIG_set0() transfers the memory
> > > management of the values to
> > > // the ECDSA_SIG object, and therefore the values
> > > that have been passed
> > > // in should not be freed directly after this
> > > function has been called
> > > if (ECDSA_SIG_set0(tmp_ecdsa_sig,
> > > r_big_num,s_big_num) != 1) {
> > > BN_free(s_big_num); // FreesBIGNUMs manually
> > > here
> > > BN_free(r_big_num);
> > > ECDSA_SIG_free(tmp_ecdsa_sig);
> > > break;
> > > }
> >
> >
> > > int der_sig_len = i2d_ECDSA_SIG(tmp_ecdsa_sig,
> > > NULL);
> > > der_sig = static_cast<unsigned
> > > char*>(OPENSSL_malloc(der_sig_len));
> > > unsigned char* der_iter = der_sig;
> > > der_sig_len = i2d_ECDSA_SIG(tmp_ecdsa_sig,
> > > &der_iter); // <= bugfix here
> > >
> > > if (der_sig_len == 0) {
> > > cout << "sig length invalid" << endl;
> > > break;
> > > }
> > >
> > > if (der_sig == NULL) {
> > > cout << "sig generation failed" << endl;
> > > break;
> > > }
> > >
> >
> > You do not need to call i2d_ECDSA_SIG() twice. Just assign NULL to
> > der_iter and i2d_ECDSA_SIG(tmp_ecdsa_sig, &der_iter) call will
> > allocate the buffer for the encoded signature for you.
> >
> >
> > > verify_md_ctx = EVP_MD_CTX_new();
> > >
> > >
> > > if (!verify_md_ctx) {
> > > cout << "Error md verify context " << endl;;
> > > break;
> > > }
> > >
> > > if (EVP_DigestVerifyInit(verify_md_ctx, NULL,
> > > (parent_cert->pub_key_algo == SEV_SIG_ALGO_ECDSA_SHA256 ||
> > > parent_cert->pub_key_algo == SEV_SIG_ALGO_ECDH_SHA256) ?
> > > EVP_sha256(): EVP_sha384(), NULL, parent_signing_key) <= 0) {
> > > cout << "Init fails " << endl;
> > > break;
> > > }
> > >
> > > if (EVP_DigestVerifyUpdate(verify_md_ctx,
> > > (uint8_t
> > > *)child_cert, pub_key_offset) <= 0){ // Calls SHA256_UPDATE
> > > cout << "updating digest fails" << endl;
> > > break;
> > > }
> > >
> > > int ret = EVP_DigestVerifyFinal(verify_md_ctx,
> > > der_sig, der_sig_len);
> > > cout << ret << endl;
> > > if (ret == 0) {
> > > cout << "EC Verify digest fails" << endl;
> > > break;
> > > } else if (ret < 0) {
> > > printf("Failed Final Verify
> > > %s\n",ERR_error_string(ERR_get_error(),NULL));
> > > cout << "EC Verify error" << endl;
> > > break;
> > > }
> > >
> > > found_match = true;
> > > cout << "SEV EC verification Succesful" << endl;
> > >
> > > Could it be because I'm creating a ECDSA SIG object and then
> > > turning
> > > it into a der format to verify? Again, suggestions would be
> > > appreciated.
> >
> > No, that should be correct. The signature as produced or verified
> > by
> > the EVP_DigestSignFinal or EVP_DigestVerifyFinal should be in the
> > DER
> > encoding of the ECDSA_SIG. So you have that correct.
> >
> > I do not see any apparent problem with your code apart of the
> > remarks
> > above.
> >
> > --
> > Tomáš Mráz, OpenSSL
>
> --
> Tomáš Mráz, OpenSSL
--
Tomáš Mráz, OpenSSL
