Repository navigation
squid: Fix nvmeof gateway startup with aes256k daemon - #39
Merged
Merged
Conversation
Since 19.2.6 cephadm creates daemon keys as aes256k (ROTATION_CIPHER). The nvmeof gateway images for squid (quay.io/ceph/nvmeof 1.2.x, the default 1.2.1, and the squid line 1.4.x up to 1.4.28) carry their own librados, built before aes256k (19.0.0 dev, 19.2.3), and cannot connect with such a key: the gateway crash-loops with "RADOS I/O error (error connecting to the cluster)" and "Unable to create OMAP, exiting!", and the service never comes up (orch:cephadm:smoke-roleless 2-services/nvmeof times out on "nvmeof.foo has 0/1"). Create and rotate nvmeof keys with the previous key type (aes): a per-daemon-type key type in utils (key_type_for), used by the nvmeof service's keyring and by key_rotate (ceph orch daemon rotate-key). A gateway already deployed with an aes256k key gets its key rotated to aes on its next deploy, so `ceph orch redeploy nvmeof.<svc>` heals a failed service. Reproduced on a cephadm cluster: with an aes256k key, python3-rados in quay.io/ceph/nvmeof:1.2.1, 1.2.5 and 1.4.28 fails to connect ([errno 5]); in 1.7 (librados 20.2.4) and 2.0.0 it connects. Fixes: https://tracker.ceph.com/issues/81778 Assisted-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Dan van der Ster <dan.vanderster@clyso.com>
kshtsk
reviewed
Oct 21, 2026
|
|
||
|
|
||
| # CryptoKey type numbers (CEPH_CRYPTO_AES, CEPH_CRYPTO_AES256KRB5) | ||
| CRYPTO_KEY_TYPES = {1: 'aes', 2: 'aes256k'} |
There was a problem hiding this comment.
Instead strings aes and aes256k there would be better use constants: LEGACY_CIPHER and ROTATION_CIPHER.
Author
There was a problem hiding this comment.
Updated the PR by an additional commit.
There was a problem hiding this comment.
Also, instead of magic numbers, it is better either import constant or even point where the constants are taken from:
$ grep CEPH_CRYPTO_ src/include/ceph_fs.h
#define CEPH_CRYPTO_NONE 0x0
#define CEPH_CRYPTO_AES 0x1
#define CEPH_CRYPTO_AES256KRB5 0x2 /* AES256-CTS-HMAC384-192 */
So if we don't have import them, python specific declaration would look like:
# C. f. src/include/ceph_fs.h
CEPH_CRYPTO_NONE = 0x0
CEPH_CRYPTO_AES = 0x1
CEPH_CRYPTO_AES256KRB5 = 0x2
CRYPTO_KEY_TYPES = {
CEPH_CRYPTO_AES: LEGACY_CIPHER,
CEPH_CRYPTO_AES256KRB5: ROTATION_CIPHER
}
Author
There was a problem hiding this comment.
What's the reason behind CEPH_CRYPTO_NONE?
kshtsk
reviewed
Oct 21, 2026
| t = struct.unpack('<H', raw[:2])[0] | ||
| except Exception: | ||
| return None | ||
| return CRYPTO_KEY_TYPES.get(t) |
There was a problem hiding this comment.
What happens when t != 1 and t != 2 KeyError exception happens?
Author
There was a problem hiding this comment.
The .get() will return None if the key is not found.
CRYPTO_KEY_TYPES spelled both its cipher names and its CryptoKey type numbers as literals, although LEGACY_CIPHER and ROTATION_CIPHER are declared right above it. Re-use them, and name the type numbers after the CEPH_CRYPTO_* macros they copy, pointing at the header that defines them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Tatjana Dehler <tatjana.dehler@clyso.com>
The unit test has to be updated from 23 to 24 mentiones of the quay.io images, because as part of 0486502 ("mgr/cephadm: give nvmeof gateways a key they can use") a new reference has been added in a comment. Signed-off-by: Tatjana Dehler <tatjana.dehler@clyso.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Tatjana Dehler <tatjana.dehler@clyso.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes cobaltcore-dev/cloud-storage#623
Backport file
release-management/backports/<uuid>.mdadded (or updated) by this PRidin frontmatter matches the filename stemprovenance.upstream_prsset (orprovenance.type: otherwith another:block)Stage-B risk (filled per
release-management/backports/RISK-RUBRIC.md)blast— cosmetic / availability / data-lossconflict— clean / trivial / substantivecoverage— strong / partial / weakhighband)Merge
Land this via
just merge-backport PR=<N>— do not click the green merge button.The just recipe constructs the merge commit's
Backport-Idtrailer.