157 lines
6.2 KiB
Plaintext
157 lines
6.2 KiB
Plaintext
{
|
|
"comments": [
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "a32e8959_0d137ac8",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 27615
|
|
},
|
|
"writtenOn": "2024-04-11T14:11:50Z",
|
|
"side": 1,
|
|
"message": "some comments inline from a quick look",
|
|
"revId": "acbd671fcd3471637898c6b9f4a256cc2c9727b5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "c30d4ea6_4ed60c30",
|
|
"filename": "/PATCHSET_LEVEL",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 0,
|
|
"author": {
|
|
"id": 27665
|
|
},
|
|
"writtenOn": "2024-04-11T17:32:30Z",
|
|
"side": 1,
|
|
"message": "Good starting point! I added a few remarks on the spec that I think we still need to address.",
|
|
"revId": "acbd671fcd3471637898c6b9f4a256cc2c9727b5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "fa847f46_03ee4aba",
|
|
"filename": "specs/2024.2/byok-for-cinder.rst",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 35,
|
|
"author": {
|
|
"id": 27665
|
|
},
|
|
"writtenOn": "2024-04-11T17:32:30Z",
|
|
"side": 1,
|
|
"message": "Things that I think need to be addressed as well:\n\nWhen receiving the API request with BYOK, cinder-api should verify the Barbican secret:\n1. Does the secret referenced by ID exist and is it retrievable using the requesting user\u0027s auth token? Usually secrets are project-bound and cannot be retrieved by users of another project.\n2. Does the secret\u0027s metadata specify a secret type that can be processed by Cinder with the specified volume type? Currently it expects secret_type to be \"symmetric\". Also the other secret attributes (cipher, mode, bit length) may need to be put in relation to the target volume type\u0027s encryption specification.\n\nIn my opinion we should avoid this failing later in cinder-volume as the feedback loop to the user is lengthy and less obvious (volume will enter error state etc.). We should fail early in cinder-api wherever possible.",
|
|
"revId": "acbd671fcd3471637898c6b9f4a256cc2c9727b5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "3f91112a_9ddc1328",
|
|
"filename": "specs/2024.2/byok-for-cinder.rst",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 49,
|
|
"author": {
|
|
"id": 27615
|
|
},
|
|
"writtenOn": "2024-04-11T14:11:50Z",
|
|
"side": 1,
|
|
"message": "maybe prefix a POST ?",
|
|
"revId": "acbd671fcd3471637898c6b9f4a256cc2c9727b5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "8ac56267_b1a4fb86",
|
|
"filename": "specs/2024.2/byok-for-cinder.rst",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 50,
|
|
"author": {
|
|
"id": 27615
|
|
},
|
|
"writtenOn": "2024-04-11T14:11:50Z",
|
|
"side": 1,
|
|
"message": "can we also mention how the new parameter looks in the request body?",
|
|
"revId": "acbd671fcd3471637898c6b9f4a256cc2c9727b5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "159ede71_bfa4da2b",
|
|
"filename": "specs/2024.2/byok-for-cinder.rst",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 55,
|
|
"author": {
|
|
"id": 27665
|
|
},
|
|
"writtenOn": "2024-04-11T17:32:30Z",
|
|
"side": 1,
|
|
"message": "We should also take into account that the Key Manager API (Barbican) can either be unreachable, failing or does not even exist in the infrastructure at the time of the API request and return an appropriate error response code and message in such case.",
|
|
"revId": "acbd671fcd3471637898c6b9f4a256cc2c9727b5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "d1eb44fc_09423c37",
|
|
"filename": "specs/2024.2/byok-for-cinder.rst",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 60,
|
|
"author": {
|
|
"id": 27665
|
|
},
|
|
"writtenOn": "2024-04-11T17:32:30Z",
|
|
"side": 1,
|
|
"message": "I don\u0027t agree that there is no impact at all. Currently, Cinder instructs Barbican to create a key for encrypted volumes (via secret order API) with a specified bit length. Barbican might use an HSM or something similar as a backend. So there are some assumptions/guarantees around entropy and strength of the key generated there.\n\nIf users are able to specify *any* passphrase with BYOK, they are also free to use very weak ones when creating volumes. Images, snapshots or clones created from such volumes will then inherit the LUKS encryption and key, thus the weak passphrase would be passed on further.\n\nI think we could consider educating users (documentation) and/or checking the lengths of the BYOK secrets at some point.",
|
|
"revId": "acbd671fcd3471637898c6b9f4a256cc2c9727b5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "4328eef8_13fc0c4b",
|
|
"filename": "specs/2024.2/byok-for-cinder.rst",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 61,
|
|
"author": {
|
|
"id": 27615
|
|
},
|
|
"writtenOn": "2024-04-11T14:11:50Z",
|
|
"side": 1,
|
|
"message": "maybe mention here or somewhere that the passphrase used by user in barbican will not be able to decrypt the volume since cinder hexifies the barbican passphrase to encrypt the volume.",
|
|
"revId": "acbd671fcd3471637898c6b9f4a256cc2c9727b5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "7632bc51_f371890b",
|
|
"filename": "specs/2024.2/byok-for-cinder.rst",
|
|
"patchSetId": 1
|
|
},
|
|
"lineNbr": 76,
|
|
"author": {
|
|
"id": 27615
|
|
},
|
|
"writtenOn": "2024-04-11T14:11:50Z",
|
|
"side": 1,
|
|
"message": "need client support in OSC as well",
|
|
"revId": "acbd671fcd3471637898c6b9f4a256cc2c9727b5",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
}
|
|
]
|
|
} |