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

os/include/tinyara/security_hal.h: Add AES-GCM mode #6178

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jylee9613
Copy link
Contributor

Add AES-GCM mode and parameter to support AES-GCM encryption/decryption. AES-GCM mode needs 4 additional parameter.

  • AAD (Additional Authentication Data)
  • AAD length
  • TAG (Authentication Tag)
  • TAG length

Add AES-GCM mode and parameter to support AES-GCM encryption/decryption.
AES-GCM mode needs 4 additional parameter.
- AAD (Additional Authentication Data)
- AAD length
- TAG (Authentication Tag)
- TAG length
Comment on lines +222 to +232
#ifdef CONFIG_HW_AES_GCM_ENC
#define HAL_AES_PARAM_INITIALIZER \
{ \
HAL_AES_UNKNOWN, NULL, 0, NULL, NULL, NULL, NULL, NULL, 0, NULL, 0 \
}
#else
#define HAL_AES_PARAM_INITIALIZER \
{ \
HAL_AES_UNKNOWN, NULL, 0, NULL, NULL, NULL, NULL \
}
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

How about below? You don't need to maintain duplicated values and it's easy to know AES_GCM requries 4 more params I think.

#define HAL_AES_PARAM_INITIALIZER                                          \
	{                                                                      \
		HAL_AES_UNKNOWN, NULL, 0, NULL, NULL, NULL, NULL   \
#ifdef CONFIG_HW_AES_GCM_ENC
		, NULL, 0, NULL, 0 \
#endif
	}

Comment on lines +188 to +189
/* Additional authentication data
* to generate tag */
Copy link
Contributor

Choose a reason for hiding this comment

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

	/* Additional authentication data 
	 * to generate tag
	 * /

Copy link
Contributor

Choose a reason for hiding this comment

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

But It looks short enough. Do we need to use 2 lines?

Comment on lines +47 to +48
config HW_AES_GCM_ENC
bool "HW AES GCM encryption decryption"
Copy link
Contributor

Choose a reason for hiding this comment

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

The config name is ENC but explanation says encryption descryption. What is correct?

Comment on lines +106 to +108
#ifdef CONFIG_HW_AES_GCM_ENC
HAL_AES_GCM,
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

How about adding that without the conditional?

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.

None yet

2 participants