Skip to content

Ed25519 test cases#610

Merged
jforissier merged 1 commit intoOP-TEE:masterfrom
sa-kib:ed25519_tests
Oct 11, 2022
Merged

Ed25519 test cases#610
jforissier merged 1 commit intoOP-TEE:masterfrom
sa-kib:ed25519_tests

Conversation

@sa-kib
Copy link
Copy Markdown
Contributor

@sa-kib sa-kib commented Aug 15, 2022

Add test cases to generate ED25519 key, sign/verify for ED25519, ED25519ctx and ED25519ph.
Companion PR for Ed25519 OP-TEE implementation: #5486

0x76, 0xf0, 0x9b, 0x3c, 0x1e, 0x16, 0x17, 0x42
};


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment describing where you got these test vectors from.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in 21de224

TEEC_Session session = { };
TEE_OperationHandle op = TEE_HANDLE_NULL;
TEE_ObjectHandle key_handle = TEE_HANDLE_NULL;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty line not needed between the declarations of stack variables.

Copy link
Copy Markdown
Contributor Author

@sa-kib sa-kib Aug 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in 4ad27fe


if (!ADBG_EXPECT_TEEC_SUCCESS(c,
xtest_teec_open_session(&session, &crypt_user_ta_uuid,
NULL, &ret_orig)))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please align with the ( on the line above. Same below where possible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in 21de224

@sa-kib sa-kib changed the title [RFC] ED25519 test cases ED25519 test cases Aug 29, 2022
@sa-kib sa-kib changed the title ED25519 test cases Ed25519 test cases Aug 29, 2022
Copy link
Copy Markdown
Contributor

@jenswi-linaro jenswi-linaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With my comment addressed:
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

tv->ptx_len, out, &out_size)))
goto out;

ADBG_EXPECT_COMPARE_UNSIGNED(c,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use ADBG_EXPECT_BUFFER() instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

varder pushed a commit to varder/optee_test that referenced this pull request Sep 28, 2022
The test vectors originate from RFC 8032 sections 7.1, 7.2, 7.3

This patch is not expected to be merged.
The pull request will be rebased as soon as the
pull request with these data vectors is merged:
OP-TEE#610

Signed-off-by: Valerii Chubar <valerii_chubar@epam.com>
@varder varder mentioned this pull request Sep 29, 2022
Copy link
Copy Markdown
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are minor indentation issues.
Acked-by: Etienne Carriere <etienne.carriere@linaro.org> once addressed.

const uint8_t flag;
const uint8_t *context;
size_t context_len;
} eddsa;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix indentation

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

TEEC_CloseSession(&session);
}
ADBG_CASE_DEFINE(regression, 4016_ed25519, xtest_tee_test_4016_ed25519,
"Test TEE Internal API ED25519 sign/verify");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix indenation

 ADBG_CASE_DEFINE(regression, 4016_ed25519, xtest_tee_test_4016_ed25519,
-"Test TEE Internal API ED25519 sign/verify");
+		  "Test TEE Internal API ED25519 sign/verify");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

xtest_add_attr(&num_attrs, attrs,
TEE_ATTR_EDDSA_PREHASH, NULL, 0);


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove one of the 2 empty lines

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

@jforissier
Copy link
Copy Markdown
Contributor

There is a problem with the key generation test

  2067:  * regression_4007_ed25519 Test TEE Internal API Generate ed25519 key
  2068:  o regression_4007_ed25519.1 Generate Ed25519 key
  2069:  regression_4000.c:4350: ta_crypt_cmd_generate_key(c, s, key, key_size, params, param_count) has an unexpected value: 0xffff0006 = TEEC_ERROR_BAD_PARAMETERS, expected 0x0 = TEEC_SUCCESS
  2070:  regression_4000.c:4835: generate_and_test_key(c, &session, 0xA1000043, 0, 256, ((void *)0), 0) has an unexpected value: 0x0 = false, expected 0x1 = true
  2071:    regression_4007_ed25519.1 FAILED
  2072:    regression_4007_ed25519 FAILED

varder pushed a commit to varder/optee_test that referenced this pull request Oct 3, 2022
The test vectors originate from RFC 8032 sections 7.1, 7.2, 7.3

This patch is not expected to be merged.
The pull request will be rebased as soon as the
pull request with these data vectors is merged:
OP-TEE#610

Signed-off-by: Valerii Chubar <valerii_chubar@epam.com>
generate_and_test_key(c, &session,
TEE_TYPE_ED25519_KEYPAIR, 0, 256,
NULL, 0)))
return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouold remove instruction return here:

	Do_ADBG_BeginSubCase(c, "Generate Ed25519 key");

	ADBG_EXPECT_TRUE(c, generate_and_test_key(c, &session,
						   TEE_TYPE_ED25519_KEYPAIR,
						   0, 256, NULL, 0));

	Do_ADBG_EndSubCase(c, "Generate Ed25519 key");
out:
	TEEC_CloseSession(&session);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, thank you!

@jforissier
Copy link
Copy Markdown
Contributor

There is a problem with the key generation test

  2067:  * regression_4007_ed25519 Test TEE Internal API Generate ed25519 key
  2068:  o regression_4007_ed25519.1 Generate Ed25519 key
  2069:  regression_4000.c:4350: ta_crypt_cmd_generate_key(c, s, key, key_size, params, param_count) has an unexpected value: 0xffff0006 = TEEC_ERROR_BAD_PARAMETERS, expected 0x0 = TEEC_SUCCESS
  2070:  regression_4000.c:4835: generate_and_test_key(c, &session, 0xA1000043, 0, 256, ((void *)0), 0) has an unexpected value: 0x0 = false, expected 0x1 = true
  2071:    regression_4007_ed25519.1 FAILED
  2072:    regression_4007_ed25519 FAILED

OP-TEE/optee_os#5569

@jforissier
Copy link
Copy Markdown
Contributor

Yet another problem with the key generation test, this time hopefully fixed by OP-TEE/optee_os#5573.

@jforissier
Copy link
Copy Markdown
Contributor

@sa-kib would you mind addressing @etienne-lms's comments and applying his Acked-by? Thanks!

Add test cases to generate ED25519 key, sign/verify for
ED25519, ED25519ctx and ED25519ph.
The test vectors originate from RFC 8032 sections 7.1, 7.2, 7.3

Signed-off-by: Valerii Chubar <valerii_chubar@epam.com>
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
@sa-kib
Copy link
Copy Markdown
Contributor Author

sa-kib commented Oct 7, 2022

@sa-kib would you mind addressing @etienne-lms's comments and applying his Acked-by? Thanks!

patch is updated now, thank you!

@jforissier
Copy link
Copy Markdown
Contributor

CI is now happy, thanks all.

@jforissier jforissier merged commit ab9863c into OP-TEE:master Oct 11, 2022
@sa-kib sa-kib deleted the ed25519_tests branch October 12, 2022 09:40
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 this pull request may close these issues.

4 participants