8000 Changing mbedtls encryption API by ccfelius · Pull Request #16196 · duckdb/duckdb · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Changing mbedtls encryption API #16196

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

Merged
merged 31 commits into from
Apr 14, 2025
Merged

Conversation

ccfelius
Copy link
Contributor

This PR replaces the mbedtls gcm encryption API (mbedtls_gcm_xxx) to the general mbedtls encryption API (mbedtls_cipher_xxxx). It also patches the httpfs extension to be compatible with the new function signature of the encryption state.
 
The main reason for replacing the old API with the new one is to be able to support AES_GCM_CTR_V1 for Parquet encryption.  I thought it might be better to split this up in two PR's.
 
The change of the encryption API is beneficial such that (i) the mbedtls [en/de]cryption flow is now exactly similar to OpenSSL leading to a single decryption flow in Parquet and (ii) the new API can handle any cipher supported by mbedtls, which is e.g. necessary for supporting AES_GCM_CTR_V1 for Parquet encryption.
 
Note that Windows Extensions is failing in the CI due to the httpfs patch. I have opened an Issue for this (#16177).

@duckdb-draftbot duckdb-draftbot marked this pull request as draft March 31, 2025 10:04
@ccfelius ccfelius marked this pull request as ready for review April 3, 2025 18:20
@carlopi
Copy link
Contributor
carlopi commented Apr 4, 2025

Failure on Windows is connected to removal of httpfs, I guess problem is that dependency on httpfs is not explicit. In any case, failing later (during testing) is better than during build, and this will be solved when applying changes and changing hash on httpfs side.

@duckdb-draftbot duckdb-draftbot marked this pull request as draft April 7, 2025 05:27
@ccfelius ccfelius marked this pull request as ready for review April 7, 2025 05:27
Mytherin added a commit that referenced this pull request Apr 11, 2025
…error` word (#17075)

Avoid `git log` while applying patches, that will print a bunch of
content to stdout.
The stdout then might (or not) contain the word error, interpreted by
MSBuild's cmake as an... error.

Thanks to the internet, at
https://stackoverflow.com/questions/78622876/visual-studio-msbuild-error-msb8066-custom-build
and
https://developercommunity.visualstudio.com/t/MSBuild:-error:-output-of-custom-build/10554390?sort=newest,
and @ccfelius that first raised the problem.

Fixes #16177, also reverts
#16722.

This should unlock #16196 and
#16463.
@duckdb-draftbot duckdb-draftbot marked this pull request as draft April 12, 2025 16:16
@ccfelius ccfelius marked this pull request as ready for review April 12, 2025 16:25
@duckdb-draftbot duckdb-draftbot marked this pull request as draft April 14, 2025 09:55
@ccfelius ccfelius marked this pull request as ready for review April 14, 2025 10:23
@Mytherin Mytherin merged commit 93a9d9f into duckdb:main Apr 14, 2025
54 checks passed
@Mytherin
Copy link
Collaborator

Thanks!

krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 18, 2025
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 18, 2025
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0