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

Should Resize() after EncryptionLoad() account for the encryption header space? #972

Closed
willgorman opened this issue Mar 14, 2024 · 7 comments · Fixed by #973
Closed

Should Resize() after EncryptionLoad() account for the encryption header space? #972

willgorman opened this issue Mar 14, 2024 · 7 comments · Fixed by #973
Labels

Comments

@willgorman
Copy link

willgorman commented Mar 14, 2024

I have a question about the expected behavior for calling Resize() on an encrypted image after applying EncryptionLoad(). As I expect, calling GetSize() after EncryptionLoad() returns the effective size available in the image minus the space consumed by the LUKS header. However, calling Resize() with a size that is less than the total size of the image (including the header) appears to have no effect. I had expected that resizing in this case would be similar to the example of the rbd resize CLI command:

rbd create --size 50G mypool/myimage
rbd encryption format mypool/myimage luks2 passphrase.bin
rbd resize --size 50G --encryption-passphrase-file passphrase.bin mypool/myimage

The following test demonstrates the scenario:

func TestResizeEncrypted(t *testing.T) {
	conn, err := rados.NewConn()
	assert.Nil(t, err)
	assert.Nil(t, conn.Connect())
	ioctx, err := conn.OpenIOContext("pool")
	assert.Nil(t, err)

	// create 50MiB image
	err = rbd.CreateImage(ioctx, "example", 50*1024*1024, rbd.NewRbdImageOptions())
	assert.Nil(t, err)

	image, err := rbd.OpenImage(ioctx, "example", rbd.NoSnapshot)
	assert.Nil(t, err)
	t.Log("before encryption format")
	t.Log(image.GetSize())

	err = image.EncryptionFormat(rbd.EncryptionOptionsLUKS2{
		Alg:        rbd.EncryptionAlgorithmAES256,
		Passphrase: []byte("passphrase"),
	})
	assert.Nil(t, err)

	t.Log("after encryption format")
	t.Log(image.GetSize())

	image.Close()

	encrypted, err := rbd.OpenImage(ioctx, "example", rbd.NoSnapshot)
	assert.Nil(t, err)
	t.Log("before encryption load")
	t.Log(encrypted.GetSize())

	encrypted.EncryptionLoad(rbd.EncryptionOptionsLUKS2{
		Alg:        rbd.EncryptionAlgorithmAES256,
		Passphrase: []byte("passphrase"),
	})
	t.Log("after encryption load")
	t.Log(encrypted.GetSize())

        // attempt to increase size back to 50MiB
	err = encrypted.Resize(50 * 1024 * 1024)
	assert.Nil(t, err)

	t.Log("after resize")
	t.Log(encrypted.GetSize())
}
before encryption format
52428800 <nil>
after encryption format
35651584 <nil>
before encryption load
52428800 <nil>
after encryption load
35651584 <nil>
after resize
35651584 <nil>

Since GetSize() in "after encryption load" returns 34MiB I expected that Resize() to 50MiB would increase the available space inside of the encrypted image back to 50MiB but it appears to be unchanged at 34MiB. Is this expected? Is there a different way that Resize() should be used in order to get it to account for the encryption header?

@phlogistonjohn
Copy link
Collaborator

phlogistonjohn commented Mar 14, 2024

Here's a potentially silly question: did you verify that the resize procedure in the docs that you quoted works? If yes, we can probably look into what apis the command line tool might be using that is different from go-ceph. Otherwise, it could possibly be a docs issue.

Thanks!

@willgorman
Copy link
Author

@phlogistonjohn Yes, it does look to me like that resize procedure for the rbd command line tool works as I expect it to from the documentation. Resizing an encrypted image that way will update the effective usable space in the image, not the total size of the image including the encryption header.

@phlogistonjohn
Copy link
Collaborator

OK, I took a look at the source for the rbd command line tool's resize action:
https://github.com/ceph/ceph/blob/bed336a3e044e8da9d8137c03ed8f4d9406ad339/src/tools/rbd/action/Resize.cc#L55

I looked over the code and saw the only major differences were uses of image.encryption_load2 and image.resize2 but when I looked closely into those functions they appeared to use the same inner apis and should not exhibit externally different behavior.

After looking at a few other options I decided to try to replicate your test case by adapting the code you shared into the go-ceph tests. Here's a copy-n-paste of the code:

// added to rbd/encryption_test.go
func TestEncryptedResize(t *testing.T) {
	conn := radosConnect(t)
	defer conn.Shutdown()

	poolname := GetUUID()
	err := conn.MakePool(poolname)
	assert.NoError(t, err)
	defer conn.DeletePool(poolname)

	imageName := "resizeme"
	imageSize := uint64(50) * 1024 * 1024
	encOpts := EncryptionOptionsLUKS2{
		Alg:        EncryptionAlgorithmAES256,
		Passphrase: []byte("test-password"),
	}

	t.Run("create", func(t *testing.T) {
		ioctx, err := conn.OpenIOContext(poolname)
		require.NoError(t, err)
		defer ioctx.Destroy()
		err = CreateImage(ioctx, imageName, imageSize, NewRbdImageOptions())
		require.NoError(t, err)

		image, err := OpenImage(ioctx, imageName, NoSnapshot)
		require.NoError(t, err)
		defer image.Close()

		s, err := image.GetSize()
		require.NoError(t, err)
		t.Logf("image size before encryption: %d", s)

		err = image.EncryptionFormat(encOpts)
		require.NoError(t, err)

		s, err = image.GetSize()
		require.NoError(t, err)
		t.Logf("image size after encryption: %d", s)
	})

	t.Run("resize", func(t *testing.T) {
		ioctx, err := conn.OpenIOContext(poolname)
		require.NoError(t, err)
		defer ioctx.Destroy()

		image, err := OpenImage(ioctx, imageName, NoSnapshot)
		require.NoError(t, err)
		defer image.Close()

		err = image.EncryptionLoad(encOpts)
		assert.NoError(t, err)

		s, err := image.GetSize()
		require.NoError(t, err)
		t.Logf("image size before resize: %d", s)
		assert.NotEqual(t, s, imageSize)

		err = image.Resize(imageSize)
		assert.NoError(t, err)

		s, err = image.GetSize()
		require.NoError(t, err)
		t.Logf("image size after encryption: %d", s)
	})
}

Fortunately, or unfortunately, the test passed for me:

=== RUN   TestEncryptedResize
=== RUN   TestEncryptedResize/create
    encryption_test.go:161: image size before encryption: 52428800
    encryption_test.go:168: image size after encryption: 35651584
=== RUN   TestEncryptedResize/resize
    encryption_test.go:185: image size before resize: 35651584
    encryption_test.go:193: image size after encryption: 52428800
--- PASS: TestEncryptedResize (12.67s)
    --- PASS: TestEncryptedResize/create (9.34s)
    --- PASS: TestEncryptedResize/resize (1.99s)
PASS

I did try using one ioctx for both blocks of the test without seeing any difference. But using multiple ioctx is a tad bit closer to the working CLI version because ioctx's aren't shared across cli invocation (neither are connections, but I digress).

Anyway, I'm not sure where we differ. I'll note that I did my run with reef libraries against a reef cluster (based on our CI container image). What versions of the ceph libraries and cluster are you using?

@phlogistonjohn
Copy link
Collaborator

phlogistonjohn commented Mar 18, 2024

I reran the test on quincy and pacific. Both passed.

FWIW, now that we have this test I plan on making a PR out of it to at least serve as a regression test for this use case. But I'll hold off on that for a little while in order not to side-track our conversation too much.

@willgorman
Copy link
Author

Thanks for checking that out, that's a surprising result but it definitely looks like it's an issue on my end. I'm running the go-ceph tests from a machine with quincy client libraries and using a microceph single VM running reef. However when I tested the rbd resize command I did run that from the microceph VM so that would have been reef end-to-end. I tried to run the rbd resize again from the machine with quincy and see if that also produces the behavior I see from go-ceph but that results in the error rbd: unrecognised option '--encryption-passphrase-file' so I guess resizing that way from the command line is probably a recent feature added in reef.

@willgorman
Copy link
Author

I upgraded the machine running go-ceph from quincy to reef and now I see the same result you do from TestEncryptedResize so it seems to me that the quincy client -> reef server was the problem.

@phlogistonjohn
Copy link
Collaborator

OK, thanks for letting me know. If you want to you could report it to ceph's tracker (https://tracker.ceph.com). Otherwise I'm going to consider it solved. I will now use this issue tracker to add the test I shared earlier as a regression test. Thanks!

phlogistonjohn added a commit to phlogistonjohn/go-ceph that referenced this issue Mar 19, 2024
Add a test to verify that an encrypted volume can be resized to
the desired unencrypted size similar to how the rbd command line
tool is documented as able to do.

Fixes: ceph#972

Original-Version-By: Will Gorman <will.gorman@gmail.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
phlogistonjohn added a commit to phlogistonjohn/go-ceph that referenced this issue Mar 19, 2024
Add a test to verify that an encrypted volume can be resized to
the desired unencrypted size similar to how the rbd command line
tool is documented as able to do.

Fixes: ceph#972

Original-Version-By: Will Gorman <will.gorman@gmail.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
phlogistonjohn added a commit to phlogistonjohn/go-ceph that referenced this issue Mar 21, 2024
Add a test to verify that an encrypted volume can be resized to
the desired unencrypted size similar to how the rbd command line
tool is documented as able to do.

Fixes: ceph#972

Original-Version-By: Will Gorman <will.gorman@gmail.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
@mergify mergify bot closed this as completed in #973 Mar 21, 2024
mergify bot pushed a commit that referenced this issue Mar 21, 2024
Add a test to verify that an encrypted volume can be resized to
the desired unencrypted size similar to how the rbd command line
tool is documented as able to do.

Fixes: #972

Original-Version-By: Will Gorman <will.gorman@gmail.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants