Implemented inplace base64 encoding aligning it to decode function #368

2020-02-15T05:06:46-06:00

mirkoflchtt commented

2020-02-15 05:06:46 -06:00

(Migrated from github.com)

I've noticed that base64 encoding routine was not working properly when src and dst buffers overlaps.
Safe inplace encoding could be a nice to have feature on some very limited memory device, allowing as a sideback src buffer to be overwritten (safely) during the encoding.
Thus I have modify the routine to always encode from back to front considering that output buffer is always larger than src buffer. I've also updated self_test to test for both encoding and decoding that
inplace operations are correctly implemented.

I've noticed that base64 encoding routine was not working properly when src and dst buffers overlaps. Safe inplace encoding could be a nice to have feature on some very limited memory device, allowing as a sideback src buffer to be overwritten (safely) during the encoding. Thus I have modify the routine to always encode from back to front considering that output buffer is always larger than src buffer. I've also updated self_test to test for both encoding and decoding that inplace operations are correctly implemented.

gilles-peskine-arm commented

2020-02-17 08:32:26 -06:00

(Migrated from github.com)

Thank you for your contribution! Before we review it:

Please make sure that the CI passes. At the moment, tests/scripts/check-files.py fails.
Please make separate commits for the self-test changes, the code changes, and the interface/documentation changes.

Thank you for your contribution! Before we review it: * Please make sure that the CI passes. At the moment, `tests/scripts/check-files.py` fails. * Please make separate commits for the self-test changes, the code changes, and the interface/documentation changes.

gilles-peskine-arm (Migrated from github.com) reviewed

2020-02-17 08:35:06 -06:00

include/mbedtls/base64.h

					
				@ -42,7 +42,9 @@ extern "C" {

				/**

Please use full sentences, correct punctuation, and applicable formatting. Older documentation sometimes takes liberties, but we fix those when we touch them.

Destination buffer. This can be \c NULL to obtain the required buffer size.
The \p src and \p dst buffers may overlap.

Please use full sentences, correct punctuation, and applicable formatting. Older documentation sometimes takes liberties, but we fix those when we touch them. > Destination buffer. This can be `\c NULL` to obtain the required buffer size. > The \p src and \p dst buffers may overlap.

gilles-peskine-arm (Migrated from github.com) reviewed

2020-02-17 08:35:57 -06:00

include/mbedtls/base64.h

					
				@ -42,7 +42,9 @@ extern "C" {

				/**

Note that in case of partial overlap, this function is only allowed to write between dst and dst + dlen. It is not allowed to overwrite any part of the src buffer that does not overlap, since src is a const * parameter.

Note that in case of partial overlap, this function is only allowed to write between `dst` and `dst + dlen`. It is not allowed to overwrite any part of the `src` buffer that does not overlap, since `src` is a `const *` parameter.

mirkoflchtt (Migrated from github.com) reviewed

2020-03-06 14:11:45 -06:00

include/mbedtls/base64.h

					
				@ -42,7 +42,9 @@ extern "C" {

				/**

Hopefully it should be ok now.

mirkoflchtt (Migrated from github.com) reviewed

2020-03-06 14:25:09 -06:00

include/mbedtls/base64.h

					
				@ -42,7 +42,9 @@ extern "C" {

				/**

Not sure to understand. The proposed patch is intended to support full buffer overlapping to potentially save memory on some scenario. Former implementation was not allowing this. I don't think partial overlapping is that relevant.
But maybe I missed something there..

Not sure to understand. The proposed patch is intended to support full buffer overlapping to potentially save memory on some scenario. Former implementation was not allowing this. I don't think partial overlapping is that relevant. But maybe I missed something there..

mirkoflchtt commented

2020-03-06 22:44:17 -06:00

(Migrated from github.com)

Not sure what's going on there. I follow the instructions to pass dco checks rebasing the repositoru.
That added others 25 commits to pull request.

Not sure what's going on there. I follow the instructions to pass dco checks rebasing the repositoru. That added others 25 commits to pull request.

gilles-peskine-arm commented

2020-03-24 15:28:02 -05:00

(Migrated from github.com)

You appear to have made a mistake during a rebase. Please rebase the pull request to have just your own commits. It might be easier to start from the current state of development and git cherry-pick your commits one by one.

You appear to have made a mistake during a rebase. Please rebase the pull request to have just your own commits. It might be easier to start from the current state of `development` and `git cherry-pick` your commits one by one.

gilles-peskine-arm commented

2023-05-24 13:44:06 -05:00

(Migrated from github.com)

Having base64 work in place would be nice. However, this pull request appears to have been abandoned, and the Mbed TLS maintenance team does not intend to work on it.

If you're still interested in this feature, please raise a new pull request against Mbed TLS which has reabsorbed Mbed Crypto.

Having base64 work in place would be nice. However, this pull request appears to have been abandoned, and the Mbed TLS maintenance team does not intend to work on it. If you're still interested in this feature, please raise a new pull request against Mbed TLS which has reabsorbed Mbed Crypto.