nvme-auth: Don't log shared secret in nvme_auth_dhchap_exponential()
Subsystems (4)
PATCH 1/3 nvme-auth: Don't log shared secret in nvme_auth_dhchap_exponential()
When debug logging is enabled, nvme_auth_dhchap_exponential() logs the DHCHAP shared secret. Remove the log to avoid exposing key material.
--- drivers/nvme/host/auth.c | 2 -- 1 file changed, 2 deletions(-)
drivers/nvme/host/auth.c +0 -2
@@ -655,8 +655,6 @@ static int nvme_auth_dhchap_exponential(struct nvme_ctrl *ctrl, chap->status =NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD ;return ret; } -dev_dbg (ctrl->device,"shared secret %*ph\n" , - (int )chap->sess_key_len, chap->sess_key);return 0 ; }
Signature · 2 lines
Thorsten Blum <[email protected]> GPG: 1D60 735E 8AEF 3BE4 73B6 9D84 7336 78FD 8DFE EAD4
PATCH 2/3 nvmet-auth: Don't log DHCHAP keys in nvmet_setup_auth()
When debug logging is enabled, nvmet_setup_auth() logs the host and controller DHCHAP key bytes. Remove the keys from debug logs to avoid exposing key material.
--- drivers/nvme/target/auth.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/nvme/target/auth.c +4 -6
@@ -199,10 +199,9 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq) ctrl->host_key =NULL ; goto out_free_hash; } -pr_debug ("%s: using hash %s key %*ph\n" , __func__, +pr_debug ("%s: using hash %s\n" , __func__, ctrl->host_key->hash >0 ? -nvme_auth_hmac_name (ctrl->host_key->hash) :"none" , - (int )ctrl->host_key->len, ctrl->host_key->key); +nvme_auth_hmac_name (ctrl->host_key->hash) :"none" );nvme_auth_free_key (ctrl->ctrl_key);if (!host->dhchap_ctrl_secret) {
@@ -217,10 +216,9 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq) ctrl->ctrl_key =NULL ; goto out_free_hash; } -pr_debug ("%s: using ctrl hash %s key %*ph\n" , __func__, +pr_debug ("%s: using ctrl hash %s\n" , __func__, ctrl->ctrl_key->hash >0 ? -nvme_auth_hmac_name (ctrl->ctrl_key->hash) :"none" , - (int )ctrl->ctrl_key->len, ctrl->ctrl_key->key); +nvme_auth_hmac_name (ctrl->ctrl_key->hash) :"none" ); out_free_hash:if (ret) {
Signature · 2 lines
Thorsten Blum <[email protected]> GPG: 1D60 735E 8AEF 3BE4 73B6 9D84 7336 78FD 8DFE EAD4
Hannes Reinecke
maintainerRe: [PATCH 2/3] nvmet-auth: Don't log DHCHAP keys in nvmet_setup_auth()
Quoted 31 lines · Thorsten Blum
When debug logging is enabled,
nvmet_setup_auth()logs the host and controller DHCHAP key bytes. Remove the keys from debug logs to avoid exposing key material.Fixes db1312dd9548 ("nvmet: implement basic In-Band Authentication")Signed-off-by Thorsten Blum <[email protected]>--- drivers/nvme/target/auth.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)drivers/nvme/target/auth.c +2
-3@@ -199,10 +199,9 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq) ctrl->host_key =NULL ; goto out_free_hash; } -pr_debug ("%s: using hash %s key %*ph\n" , __func__, +pr_debug ("%s: using hash %s\n" , __func__, ctrl->host_key->hash >0 ? -nvme_auth_hmac_name (ctrl->host_key->hash) :"none" , - (int )ctrl->host_key->len, ctrl->host_key->key); +nvme_auth_hmac_name (ctrl->host_key->hash) :"none" );nvme_auth_free_key (ctrl->ctrl_key);if (!host->dhchap_ctrl_secret) {@@ -217,10 +216,9 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq) ctrl->ctrl_key =NULL ; goto out_free_hash;
drivers/nvme/target/auth.c u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq) · line 224
Quoted 10 lines
drivers/nvme/target/auth.c +2
-3} -pr_debug ("%s: using ctrl hash %s key %*ph\n" , __func__, +pr_debug ("%s: using ctrl hash %s\n" , __func__, ctrl->ctrl_key->hash >0 ? -nvme_auth_hmac_name (ctrl->ctrl_key->hash) :"none" , - (int )ctrl->ctrl_key->len, ctrl->ctrl_key->key); +nvme_auth_hmac_name (ctrl->ctrl_key->hash) :"none" ); out_free_hash:if (ret) {
Without the key the pr_debug calls are pretty much pointless anyway, so you might want to remove them, too.
However, these debug prints really help when trying to figure out authentication failures.
I think it would be better to add a compile-time option to disable these outputs entirely.
I'll send a patch.
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Re: [PATCH 2/3] nvmet-auth: Don't log DHCHAP keys in nvmet_setup_auth()
Quoted 44 lines · Hannes Reinecke
Quoted 29 lines · Thorsten Blum
When debug logging is enabled,
nvmet_setup_auth()logs the host and controller DHCHAP key bytes. Remove the keys from debug logs to avoid exposing key material.Fixes db1312dd9548 ("nvmet: implement basic In-Band Authentication")Signed-off-by Thorsten Blum <[email protected]>--- drivers/nvme/target/auth.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)drivers/nvme/target/auth.c +2
-3@@ -199,10 +199,9 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq) ctrl->host_key =NULL ; goto out_free_hash; } -pr_debug ("%s: using hash %s key %*ph\n" , __func__, +pr_debug ("%s: using hash %s\n" , __func__, ctrl->host_key->hash >0 ? -nvme_auth_hmac_name (ctrl->host_key->hash) :"none" , - (int )ctrl->host_key->len, ctrl->host_key->key); +nvme_auth_hmac_name (ctrl->host_key->hash) :"none" );nvme_auth_free_key (ctrl->ctrl_key);if (!host->dhchap_ctrl_secret) {@@ -217,10 +216,9 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq) ctrl->ctrl_key =NULL ;drivers/nvme/target/auth.c u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq) · line 223
Quoted 10 lines
drivers/nvme/target/auth.c +2
-3goto out_free_hash; } -pr_debug ("%s: using ctrl hash %s key %*ph\n" , __func__, +pr_debug ("%s: using ctrl hash %s\n" , __func__, ctrl->ctrl_key->hash >0 ? -nvme_auth_hmac_name (ctrl->ctrl_key->hash) :"none" , - (int )ctrl->ctrl_key->len, ctrl->ctrl_key->key); +nvme_auth_hmac_name (ctrl->ctrl_key->hash) :"none" ); out_free_hash:if (ret) {Without the key the
pr_debugcalls are pretty much pointless anyway, so you might want to remove them, too.
Quoted 6 lines
However, these debug prints really help when trying to figure out authentication failures.
I think it would be better to add a compile-time option to disable these outputs entirely.I'll send a patch.
Did you ever send a patch? I couldn't find anything. The code hasn't changed either and the keys are still logged (same for patch 3/3).
Thanks,
Thorsten
PATCH 3/3 nvmet-auth: Don't log DHCHAP shared secret in nvmet_auth_ctrl_sesskey()
In reply to Thorsten BlumWhen debug logging is enabled, nvmet_auth_ctrl_sesskey() logs the DHCHAP shared secret. Remove the log to avoid exposing key material.
--- drivers/nvme/target/auth.c | 4 ---- 1 file changed, 4 deletions(-)
drivers/nvme/target/auth.c +0 -4
@@ -544,10 +544,6 @@ int nvmet_auth_ctrl_sesskey(struct nvmet_req *req, req->sq->dhchap_skey_len);if (ret)pr_debug ("failed to compute shared secret, err %d\n" , ret); -else -pr_debug ("%s: shared secret %*ph\n" , __func__, - (int )req->sq->dhchap_skey_len, - req->sq->dhchap_skey);return ret; }
Signature · 2 lines
Thorsten Blum <[email protected]> GPG: 1D60 735E 8AEF 3BE4 73B6 9D84 7336 78FD 8DFE EAD4
Hannes Reinecke
maintainerRe: [PATCH 3/3] nvmet-auth: Don't log DHCHAP shared secret in nvmet_auth_ctrl_sesskey()
Quoted 15 lines · Thorsten Blum
When debug logging is enabled,
nvmet_auth_ctrl_sesskey()logs the DHCHAP shared secret. Remove the log to avoid exposing key material.Fixes 7a277c37d352 ("nvmet-auth: Diffie-Hellman key exchange support")Signed-off-by Thorsten Blum <[email protected]>--- drivers/nvme/target/auth.c | 4 ---- 1 file changed, 4 deletions(-)drivers/nvme/target/auth.c +0
-0
drivers/nvme/target/auth.c int nvmet_auth_ctrl_sesskey(struct nvmet_req *req, · line 549
Quoted 10 lines
drivers/nvme/target/auth.c +0
-4@@ -544,10 +544,6 @@ int nvmet_auth_ctrl_sesskey(struct nvmet_req *req, req->sq->dhchap_skey_len);if (ret)pr_debug ("failed to compute shared secret, err %d\n" , ret); -else -pr_debug ("%s: shared secret %*ph\n" , __func__, - (int )req->sq->dhchap_skey_len, - req->sq->dhchap_skey);return ret; }
As indicated in the previous patch, we should use a compile time option to disable the messages.
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Quoted 15 lines · Thorsten Blum
When debug logging is enabled,
nvme_auth_dhchap_exponential()logs the DHCHAP shared secret. Remove the log to avoid exposing key material.Fixes b61775d185a3 ("nvme-auth: Diffie-Hellman key exchange support")Signed-off-by Thorsten Blum <[email protected]>--- drivers/nvme/host/auth.c | 2 -- 1 file changed, 2 deletions(-)drivers/nvme/host/auth.c +0
-0
drivers/nvme/host/auth.c static int nvme_auth_dhchap_exponential(struct nvme_ctrl *ctrl, · line 660
Quoted 8 lines
drivers/nvme/host/auth.c +0
-2@@ -655,8 +655,6 @@ static int nvme_auth_dhchap_exponential(struct nvme_ctrl *ctrl, chap->status =NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD ;return ret; } -dev_dbg (ctrl->device,"shared secret %*ph\n" , - (int )chap->sess_key_len, chap->sess_key);return 0 ; }
Yeah, that was primarily for debugging.
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Quoted 26 lines · Hannes Reinecke
Quoted 15 lines · Thorsten Blum
When debug logging is enabled,
nvme_auth_dhchap_exponential()logs the DHCHAP shared secret. Remove the log to avoid exposing key material.Fixes b61775d185a3 ("nvme-auth: Diffie-Hellman key exchange support")Signed-off-by Thorsten Blum <[email protected]>--- drivers/nvme/host/auth.c | 2 -- 1 file changed, 2 deletions(-)drivers/nvme/host/auth.c +0
-0drivers/nvme/host/auth.c static int nvme_auth_dhchap_exponential(struct nvme_ctrl *ctrl, · line 659
Quoted 7 lines
drivers/nvme/host/auth.c +0
-2@@ -655,8 +655,6 @@ static int nvme_auth_dhchap_exponential(struct nvme_ctrl *ctrl, chap->status =NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD ;return ret; } -dev_dbg (ctrl->device,"shared secret %*ph\n" , - (int )chap->sess_key_len, chap->sess_key);return 0 ; }Yeah, that was primarily for debugging.
Quoted 1 line
Reviewed-by Hannes Reinecke <[email protected]>
This was never applied, was it?
Thanks,
Thorsten
Quoted 6 lines · Thorsten Blum
When debug logging is enabled,
nvme_auth_dhchap_exponential()logs the DHCHAP shared secret. Remove the log to avoid exposing key material.Fixes b61775d185a3 ("nvme-auth: Diffie-Hellman key exchange support")Signed-off-by Thorsten Blum <[email protected]>