-
Notifications
You must be signed in to change notification settings - Fork 1k
Fix encrypted fresh DB page-size retargeting #7387
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2761,6 +2761,25 @@ impl Pager { | |
| // Clear dirty pages since this is pre-initialization setup, not a real write transaction. | ||
| // Rebuilding init_page_1 must not leak any stale 4 KiB page-1 image into the first write. | ||
| self.dirty_pages.write().clear(); | ||
|
|
||
| // Invariant: this is the sole pre-initialization page-size retargeting | ||
| // hook for the fresh DB / fresh ATTACH / in-place VACUUM temp DB paths. | ||
| // Any encryption context installed before this point (PRAGMA | ||
| // cipher/hexkey, URI-supplied cipher/hexkey, fresh-attach `_init`) was | ||
| // built against the prior page size and would panic on the first | ||
| // encrypted write with a stale buffer-length assertion. Retarget it in | ||
| // place and propagate to the WAL IOContext when one is installed. | ||
| let retargeted = { | ||
| let mut io_ctx = self.io_ctx.write(); | ||
| io_ctx.retarget_encryption_page_size(size) | ||
| }; | ||
| if !retargeted { | ||
| return Ok(()); | ||
| } | ||
| let Some(wal) = self.wal.as_ref() else { | ||
| return Ok(()); | ||
| }; | ||
| wal.set_io_context(self.io_ctx.read().clone()); | ||
|
Comment on lines
+2772
to
+2782
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a method on within it, you can modify the ctx if the page size is non default. Then you can set the WAL ctx too |
||
| Ok(()) | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1351,3 +1351,184 @@ fn test_non_4k_page_size_encryption_enable_mvcc_after_encryption( | |
| .try_for_each(|query| run_query(&tmp_db, &conn, query))?; | ||
| do_flush(&conn, &tmp_db) | ||
| } | ||
|
|
||
| // Regression coverage for https://github.com/tursodatabase/turso/issues/7375. | ||
| // PRAGMA cipher/hexkey before PRAGMA page_size used to leave EncryptionContext | ||
| // pinned to the default 4096-byte page size; the first write then panicked. | ||
|
|
||
| const ISSUE_7375_KEY_256: &str = "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f"; | ||
| const ISSUE_7375_KEY_128: &str = "000102030405060708090a0b0c0d0e0f"; | ||
|
Comment on lines
+1355
to
+1360
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is extra noise and the comment is not relevant to the keys you can just use the existing CIPHER_A / KEY_A etc. |
||
|
|
||
| fn assert_encrypted_page_size_after_key_and_cipher( | ||
| page_size: i64, | ||
| cipher: &str, | ||
| hexkey: &str, | ||
| ) -> anyhow::Result<()> { | ||
| let tmp_db = TempDatabaseBuilder::new() | ||
| .with_opts(DatabaseOpts::new().with_encryption(true)) | ||
| .build(); | ||
|
|
||
| let conn = tmp_db.connect_limbo(); | ||
| conn.execute(format!("PRAGMA cipher = '{cipher}'"))?; | ||
| conn.execute(format!("PRAGMA hexkey = '{hexkey}'"))?; | ||
| conn.execute(format!("PRAGMA page_size = {page_size}"))?; | ||
| conn.execute("CREATE TABLE t(a INTEGER PRIMARY KEY, b BLOB)")?; | ||
| conn.execute("INSERT INTO t VALUES(1, randomblob(300))")?; | ||
|
|
||
| let rows: Vec<(i64,)> = conn.exec_rows("SELECT count(*) FROM t"); | ||
| assert_eq!(rows[0].0, 1, "row count mismatch for page_size={page_size}"); | ||
| let ps: Vec<(i64,)> = conn.exec_rows("PRAGMA page_size"); | ||
| assert_eq!(ps[0].0, page_size, "page_size readback mismatch"); | ||
|
|
||
| do_flush(&conn, &tmp_db)?; | ||
|
|
||
| let uri = format!( | ||
| "file:{}?cipher={cipher}&hexkey={hexkey}", | ||
| tmp_db.path.to_str().unwrap() | ||
| ); | ||
| let (_io, reopened) = | ||
| turso_core::Connection::from_uri(&uri, DatabaseOpts::new().with_encryption(true))?; | ||
| let rows: Vec<(i64,)> = reopened.exec_rows("SELECT count(*) FROM t"); | ||
| assert_eq!( | ||
| rows[0].0, 1, | ||
| "row count after reopen mismatch for page_size={page_size}" | ||
| ); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| #[turso_macros::test] | ||
| fn test_encrypted_page_size_after_key_and_cipher(_tmp_db: TempDatabase) -> anyhow::Result<()> { | ||
| let _ = env_logger::try_init(); | ||
| for page_size in [512, 4096, 65536] { | ||
| assert_encrypted_page_size_after_key_and_cipher( | ||
| page_size, | ||
| "aegis256x4", | ||
| ISSUE_7375_KEY_256, | ||
| )?; | ||
| } | ||
| // Different key size / metadata path. | ||
| assert_encrypted_page_size_after_key_and_cipher(512, "aes128gcm", ISSUE_7375_KEY_128)?; | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[turso_macros::test] | ||
| fn test_uri_encryption_then_page_size(_tmp_db: TempDatabase) -> anyhow::Result<()> { | ||
| let _ = env_logger::try_init(); | ||
| let tmp_db = TempDatabaseBuilder::new() | ||
| .with_opts(DatabaseOpts::new().with_encryption(true)) | ||
| .build(); | ||
|
|
||
| let uri = format!( | ||
| "file:{}?cipher=aegis256x4&hexkey={ISSUE_7375_KEY_256}", | ||
| tmp_db.path.to_str().unwrap() | ||
| ); | ||
|
|
||
| { | ||
| let (io, conn) = | ||
| turso_core::Connection::from_uri(&uri, DatabaseOpts::new().with_encryption(true))?; | ||
| conn.execute("PRAGMA page_size = 512")?; | ||
| conn.execute("CREATE TABLE t(a INTEGER PRIMARY KEY, b BLOB)")?; | ||
| conn.execute("INSERT INTO t VALUES(1, randomblob(300))")?; | ||
|
|
||
| let rows: Vec<(i64,)> = conn.exec_rows("SELECT count(*) FROM t"); | ||
| assert_eq!(rows[0].0, 1); | ||
| let ps: Vec<(i64,)> = conn.exec_rows("PRAGMA page_size"); | ||
| assert_eq!(ps[0].0, 512); | ||
|
|
||
| for c in conn.cacheflush()? { | ||
| io.wait_for_completion(c)?; | ||
| } | ||
| } | ||
|
|
||
| let (_io, reopened) = | ||
| turso_core::Connection::from_uri(&uri, DatabaseOpts::new().with_encryption(true))?; | ||
| let rows: Vec<(i64,)> = reopened.exec_rows("SELECT count(*) FROM t"); | ||
| assert_eq!(rows[0].0, 1); | ||
| let ps: Vec<(i64,)> = reopened.exec_rows("PRAGMA page_size"); | ||
| assert_eq!(ps[0].0, 512); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| #[turso_macros::test] | ||
| fn test_fresh_attach_encrypted_non_4k_page_size(_tmp_db: TempDatabase) -> anyhow::Result<()> { | ||
| let _ = env_logger::try_init(); | ||
| let aux_dir = tempfile::tempdir()?; | ||
| let aux_path = aux_dir.path().join("aux.db"); | ||
| let aux_uri = format!( | ||
| "file:{}?cipher=aegis256x4&hexkey={ISSUE_7375_KEY_256}", | ||
| aux_path.to_str().unwrap() | ||
| ); | ||
|
|
||
| { | ||
| let main_db = TempDatabaseBuilder::new() | ||
| .with_opts(DatabaseOpts::new().with_encryption(true).with_attach(true)) | ||
| .build(); | ||
| let conn = main_db.connect_limbo(); | ||
| conn.execute("PRAGMA page_size = 512")?; | ||
| conn.execute(format!("ATTACH '{aux_uri}' AS aux"))?; | ||
| conn.execute("CREATE TABLE aux.t(a INTEGER PRIMARY KEY, b BLOB)")?; | ||
| conn.execute("INSERT INTO aux.t VALUES(1, randomblob(300))")?; | ||
|
|
||
| let rows: Vec<(i64,)> = conn.exec_rows("SELECT count(*) FROM aux.t"); | ||
| assert_eq!(rows[0].0, 1); | ||
| // Layout inheritance: the attached pager must adopt the main | ||
| // connection's page size, not the fresh-DB 4096 default. | ||
| let aux_ps: Vec<(i64,)> = conn.exec_rows("PRAGMA aux.page_size"); | ||
| assert_eq!(aux_ps[0].0, 512); | ||
|
|
||
| // Force aux WAL onto its main file so the standalone reopen sees the row. | ||
| conn.execute("PRAGMA aux.wal_checkpoint(TRUNCATE)")?; | ||
| do_flush(&conn, &main_db)?; | ||
| } | ||
|
|
||
| let (_io, aux_conn) = | ||
| turso_core::Connection::from_uri(&aux_uri, DatabaseOpts::new().with_encryption(true))?; | ||
| let rows: Vec<(i64,)> = aux_conn.exec_rows("SELECT count(*) FROM t"); | ||
| assert_eq!(rows[0].0, 1); | ||
| let ps: Vec<(i64,)> = aux_conn.exec_rows("PRAGMA page_size"); | ||
| assert_eq!(ps[0].0, 512); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| #[turso_macros::test] | ||
| fn test_inplace_vacuum_non_4k_encryption(_tmp_db: TempDatabase) -> anyhow::Result<()> { | ||
| let _ = env_logger::try_init(); | ||
| let tmp_db = TempDatabaseBuilder::new() | ||
| .with_opts(DatabaseOpts::new().with_encryption(true)) | ||
| .build(); | ||
|
|
||
| let conn = tmp_db.connect_limbo(); | ||
| // Safe creation order so setup itself does not trigger the issue path. | ||
| conn.execute("PRAGMA page_size = 512")?; | ||
| conn.execute("PRAGMA cipher = 'aegis256x4'")?; | ||
| conn.execute(format!("PRAGMA hexkey = '{ISSUE_7375_KEY_256}'"))?; | ||
|
|
||
| conn.execute("CREATE TABLE t(id INTEGER PRIMARY KEY, payload BLOB)")?; | ||
| for i in 0..100i64 { | ||
| conn.execute(format!("INSERT INTO t VALUES({i}, randomblob(300))"))?; | ||
| } | ||
| conn.execute("DELETE FROM t WHERE id % 3 = 0")?; | ||
|
|
||
| conn.execute("VACUUM")?; | ||
|
|
||
| let rows: Vec<(i64,)> = conn.exec_rows("SELECT count(*) FROM t"); | ||
| assert_eq!(rows[0].0, 66); | ||
| let ps: Vec<(i64,)> = conn.exec_rows("PRAGMA page_size"); | ||
| assert_eq!(ps[0].0, 512); | ||
|
|
||
| do_flush(&conn, &tmp_db)?; | ||
|
|
||
| let uri = format!( | ||
| "file:{}?cipher=aegis256x4&hexkey={ISSUE_7375_KEY_256}", | ||
| tmp_db.path.to_str().unwrap() | ||
| ); | ||
| let (_io, reopened) = | ||
| turso_core::Connection::from_uri(&uri, DatabaseOpts::new().with_encryption(true))?; | ||
| let rows: Vec<(i64,)> = reopened.exec_rows("SELECT count(*) FROM t"); | ||
| assert_eq!(rows[0].0, 66); | ||
|
|
||
| Ok(()) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer calling it as
reset_page_size_in_encryption_ctx