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.

Sign up for GitHub

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

Jump to bottom

Move base64 constant-time functions to the new module #5236

Merged
merged 8 commits into from
Dec 9, 2021
Merged

Move base64 constant-time functions to the new module #5236

merged 8 commits into from
Dec 9, 2021

Conversation

gabor-mezei-arm
Copy link
Contributor

@gabor-mezei-arm gabor-mezei-arm commented Nov 26, 2021 •
edited
Loading

Description

The goal of this task is to move base64 constant-time functions to the new constant-time module.

Resolves #4926

Requires Backporting

Todos

  • Documentation
  • Backported

@gabor-mezei-arm gabor-mezei-arm self-assigned this Nov 26, 2021
@gabor-mezei-arm gabor-mezei-arm added component-crypto Crypto primitives and low-level interfaces enhancement needs-ci Needs to pass CI tests labels Nov 26, 2021
@gabor-mezei-arm gabor-mezei-arm force-pushed the 4926_base64_move_constant-time_functions branch from f2d1fe5 to 3ce063b Compare November 26, 2021 14:30
@gabor-mezei-arm
Rename functions to have suitable name
b8d7892 
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
@gabor-mezei-arm
Move mbedtls_ct_uchar_mask_of_range function to the constant-time module
28d6115 
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
@gabor-mezei-arm
Move mbedtls_ct_base64_enc_char function to the constant-time module
9a4074a 
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
@gabor-mezei-arm
Move mbedtls_ct_base64_dec_value function to the constant-time module
358829a 
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
@gabor-mezei-arm
Delete base64_invasive.h due to functions are moved to the constant-t…
d77b86c 
…ime module

Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
@gabor-mezei-arm
Make mbedtls_ct_uchar_mask_of_range function static
c0d8dda 
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
@gabor-mezei-arm
Unify function parameters
14d5fac 
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
@gabor-mezei-arm
Add documentation for the functions
a096975 
Signed-off-by: Gabor Mezei <gabor.mezei@arm.com>
@gabor-mezei-arm gabor-mezei-arm force-pushed the 4926_base64_move_constant-time_functions branch from 3ce063b to a096975 Compare November 26, 2021 16:33
@gabor-mezei-arm gabor-mezei-arm added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review and removed needs-ci Needs to pass CI tests labels Nov 29, 2021
@gabor-mezei-arm gabor-mezei-arm added the needs-backports Backports are missing or are pending review and approval. label Nov 29, 2021
davidhorstmann-arm
davidhorstmann-arm requested changes Nov 30, 2021
View reviewed changes
Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm left a comment

Choose a reason for hiding this comment

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

One question, LGTM otherwise.

library/constant_time_internal.h Show resolved Hide resolved
davidhorstmann-arm
davidhorstmann-arm approved these changes Nov 30, 2021
View reviewed changes
Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm left a comment

Choose a reason for hiding this comment

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

LGTM - thanks!

@mpg mpg self-requested a review December 8, 2021 11:53
@mpg mpg removed the needs-reviewer This PR needs someone to pick it up for review label Dec 8, 2021
mpg
mpg reviewed Dec 8, 2021
View reviewed changes
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

I understood the goal of the task differently, so I'd like this point to be resolved before I can approve this PR. #4926

@mpg mpg self-requested a review December 8, 2021 14:01
mpg
mpg approved these changes Dec 8, 2021
View reviewed changes
Copy link
Contributor

@mpg mpg left a comment

Choose a reason for hiding this comment

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

My misunderstanding about the intended scope of the task has been clarified. All looking good to me.

@mpg mpg added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Dec 8, 2021
@gabor-mezei-arm gabor-mezei-arm mentioned this pull request Dec 8, 2021
Merged
@mpg mpg removed the needs-backports Backports are missing or are pending review and approval. label Dec 9, 2021
@mpg mpg merged commit d7d740e into Mbed-TLS:development Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidhorstmann-arm davidhorstmann-arm davidhorstmann-arm approved these changes

@mpg mpg mpg approved these changes

Assignees

@gabor-mezei-arm gabor-mezei-arm

Labels
approved Design and code approved - may be waiting for CI or backports component-crypto Crypto primitives and low-level interfaces enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move base64 constant-time functions to the new module
3 participants
@gabor-mezei-arm @mpg @davidhorstmann-arm