Message ID | 20241111134055.66981-1-pc@manguebit.com |
---|---|
State | New |
Headers | show |
Series | smb: client: fix use-after-free of signing key | expand |
tentatively merged into cifs-2.6.git for-next pending testing and additional review On Mon, Nov 11, 2024 at 7:41 AM Paulo Alcantara <pc@manguebit.com> wrote: > > Customers have reported use-after-free in @ses->auth_key.response with > SMB2.1 + sign mounts which occurs due to following race: > > task A task B > cifs_mount() > dfs_mount_share() > get_session() > cifs_mount_get_session() cifs_send_recv() > cifs_get_smb_ses() compound_send_recv() > cifs_setup_session() smb2_setup_request() > kfree_sensitive() smb2_calc_signature() > crypto_shash_setkey() *UAF* > > Fix this by ensuring that we have a valid @ses->auth_key.response by > checking whether @ses->ses_status is SES_GOOD or SES_EXITING with > @ses->ses_lock held. After commit 24a9799aa8ef ("smb: client: fix UAF > in smb2_reconnect_server()"), we made sure to call ->logoff() only > when @ses was known to be good (e.g. valid ->auth_key.response), so > it's safe to access signing key when @ses->ses_status == SES_EXITING. > > Reported-by: Jay Shin <jaeshin@redhat.com> > Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com> > --- > fs/smb/client/smb2proto.h | 2 -- > fs/smb/client/smb2transport.c | 56 +++++++++++++++++++++++++---------- > 2 files changed, 40 insertions(+), 18 deletions(-) > > diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h > index 6f9885e4f66c..71504b30909e 100644 > --- a/fs/smb/client/smb2proto.h > +++ b/fs/smb/client/smb2proto.h > @@ -37,8 +37,6 @@ extern struct mid_q_entry *smb2_setup_request(struct cifs_ses *ses, > struct smb_rqst *rqst); > extern struct mid_q_entry *smb2_setup_async_request( > struct TCP_Server_Info *server, struct smb_rqst *rqst); > -extern struct cifs_ses *smb2_find_smb_ses(struct TCP_Server_Info *server, > - __u64 ses_id); > extern struct cifs_tcon *smb2_find_smb_tcon(struct TCP_Server_Info *server, > __u64 ses_id, __u32 tid); > extern int smb2_calc_signature(struct smb_rqst *rqst, > diff --git a/fs/smb/client/smb2transport.c b/fs/smb/client/smb2transport.c > index b486b14bb330..475b36c27f65 100644 > --- a/fs/smb/client/smb2transport.c > +++ b/fs/smb/client/smb2transport.c > @@ -74,7 +74,7 @@ smb311_crypto_shash_allocate(struct TCP_Server_Info *server) > > > static > -int smb2_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key) > +int smb3_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key) > { > struct cifs_chan *chan; > struct TCP_Server_Info *pserver; > @@ -168,16 +168,41 @@ smb2_find_smb_ses_unlocked(struct TCP_Server_Info *server, __u64 ses_id) > return NULL; > } > > -struct cifs_ses * > -smb2_find_smb_ses(struct TCP_Server_Info *server, __u64 ses_id) > +static int smb2_get_sign_key(struct TCP_Server_Info *server, > + __u64 ses_id, u8 *key) > { > struct cifs_ses *ses; > + int rc = -ENOENT; > + > + if (SERVER_IS_CHAN(server)) > + server = server->primary_server; > > spin_lock(&cifs_tcp_ses_lock); > - ses = smb2_find_smb_ses_unlocked(server, ses_id); > + list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) { > + if (ses->Suid != ses_id) > + continue; > + > + rc = 0; > + spin_lock(&ses->ses_lock); > + switch (ses->ses_status) { > + case SES_EXITING: /* SMB2_LOGOFF */ > + case SES_GOOD: > + if (likely(ses->auth_key.response)) { > + memcpy(key, ses->auth_key.response, > + SMB2_NTLMV2_SESSKEY_SIZE); > + } else { > + rc = -EIO; > + } > + break; > + default: > + rc = -EAGAIN; > + break; > + } > + spin_unlock(&ses->ses_lock); > + break; > + } > spin_unlock(&cifs_tcp_ses_lock); > - > - return ses; > + return rc; > } > > static struct cifs_tcon * > @@ -236,14 +261,16 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, > unsigned char *sigptr = smb2_signature; > struct kvec *iov = rqst->rq_iov; > struct smb2_hdr *shdr = (struct smb2_hdr *)iov[0].iov_base; > - struct cifs_ses *ses; > struct shash_desc *shash = NULL; > struct smb_rqst drqst; > + __u64 sid = le64_to_cpu(shdr->SessionId); > + u8 key[SMB2_NTLMV2_SESSKEY_SIZE]; > > - ses = smb2_find_smb_ses(server, le64_to_cpu(shdr->SessionId)); > - if (unlikely(!ses)) { > - cifs_server_dbg(FYI, "%s: Could not find session\n", __func__); > - return -ENOENT; > + rc = smb2_get_sign_key(server, sid, key); > + if (unlikely(rc)) { > + cifs_server_dbg(FYI, "%s: [sesid=0x%llx] couldn't find signing key: %d\n", > + __func__, sid, rc); > + return rc; > } > > memset(smb2_signature, 0x0, SMB2_HMACSHA256_SIZE); > @@ -260,8 +287,7 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, > shash = server->secmech.hmacsha256; > } > > - rc = crypto_shash_setkey(shash->tfm, ses->auth_key.response, > - SMB2_NTLMV2_SESSKEY_SIZE); > + rc = crypto_shash_setkey(shash->tfm, key, sizeof(key)); > if (rc) { > cifs_server_dbg(VFS, > "%s: Could not update with response\n", > @@ -303,8 +329,6 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, > out: > if (allocate_crypto) > cifs_free_hash(&shash); > - if (ses) > - cifs_put_smb_ses(ses); > return rc; > } > > @@ -570,7 +594,7 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, > struct smb_rqst drqst; > u8 key[SMB3_SIGN_KEY_SIZE]; > > - rc = smb2_get_sign_key(le64_to_cpu(shdr->SessionId), server, key); > + rc = smb3_get_sign_key(le64_to_cpu(shdr->SessionId), server, key); > if (unlikely(rc)) { > cifs_server_dbg(FYI, "%s: Could not get signing key\n", __func__); > return rc; > -- > 2.47.0 >
diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h index 6f9885e4f66c..71504b30909e 100644 --- a/fs/smb/client/smb2proto.h +++ b/fs/smb/client/smb2proto.h @@ -37,8 +37,6 @@ extern struct mid_q_entry *smb2_setup_request(struct cifs_ses *ses, struct smb_rqst *rqst); extern struct mid_q_entry *smb2_setup_async_request( struct TCP_Server_Info *server, struct smb_rqst *rqst); -extern struct cifs_ses *smb2_find_smb_ses(struct TCP_Server_Info *server, - __u64 ses_id); extern struct cifs_tcon *smb2_find_smb_tcon(struct TCP_Server_Info *server, __u64 ses_id, __u32 tid); extern int smb2_calc_signature(struct smb_rqst *rqst, diff --git a/fs/smb/client/smb2transport.c b/fs/smb/client/smb2transport.c index b486b14bb330..475b36c27f65 100644 --- a/fs/smb/client/smb2transport.c +++ b/fs/smb/client/smb2transport.c @@ -74,7 +74,7 @@ smb311_crypto_shash_allocate(struct TCP_Server_Info *server) static -int smb2_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key) +int smb3_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key) { struct cifs_chan *chan; struct TCP_Server_Info *pserver; @@ -168,16 +168,41 @@ smb2_find_smb_ses_unlocked(struct TCP_Server_Info *server, __u64 ses_id) return NULL; } -struct cifs_ses * -smb2_find_smb_ses(struct TCP_Server_Info *server, __u64 ses_id) +static int smb2_get_sign_key(struct TCP_Server_Info *server, + __u64 ses_id, u8 *key) { struct cifs_ses *ses; + int rc = -ENOENT; + + if (SERVER_IS_CHAN(server)) + server = server->primary_server; spin_lock(&cifs_tcp_ses_lock); - ses = smb2_find_smb_ses_unlocked(server, ses_id); + list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) { + if (ses->Suid != ses_id) + continue; + + rc = 0; + spin_lock(&ses->ses_lock); + switch (ses->ses_status) { + case SES_EXITING: /* SMB2_LOGOFF */ + case SES_GOOD: + if (likely(ses->auth_key.response)) { + memcpy(key, ses->auth_key.response, + SMB2_NTLMV2_SESSKEY_SIZE); + } else { + rc = -EIO; + } + break; + default: + rc = -EAGAIN; + break; + } + spin_unlock(&ses->ses_lock); + break; + } spin_unlock(&cifs_tcp_ses_lock); - - return ses; + return rc; } static struct cifs_tcon * @@ -236,14 +261,16 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, unsigned char *sigptr = smb2_signature; struct kvec *iov = rqst->rq_iov; struct smb2_hdr *shdr = (struct smb2_hdr *)iov[0].iov_base; - struct cifs_ses *ses; struct shash_desc *shash = NULL; struct smb_rqst drqst; + __u64 sid = le64_to_cpu(shdr->SessionId); + u8 key[SMB2_NTLMV2_SESSKEY_SIZE]; - ses = smb2_find_smb_ses(server, le64_to_cpu(shdr->SessionId)); - if (unlikely(!ses)) { - cifs_server_dbg(FYI, "%s: Could not find session\n", __func__); - return -ENOENT; + rc = smb2_get_sign_key(server, sid, key); + if (unlikely(rc)) { + cifs_server_dbg(FYI, "%s: [sesid=0x%llx] couldn't find signing key: %d\n", + __func__, sid, rc); + return rc; } memset(smb2_signature, 0x0, SMB2_HMACSHA256_SIZE); @@ -260,8 +287,7 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, shash = server->secmech.hmacsha256; } - rc = crypto_shash_setkey(shash->tfm, ses->auth_key.response, - SMB2_NTLMV2_SESSKEY_SIZE); + rc = crypto_shash_setkey(shash->tfm, key, sizeof(key)); if (rc) { cifs_server_dbg(VFS, "%s: Could not update with response\n", @@ -303,8 +329,6 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, out: if (allocate_crypto) cifs_free_hash(&shash); - if (ses) - cifs_put_smb_ses(ses); return rc; } @@ -570,7 +594,7 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, struct smb_rqst drqst; u8 key[SMB3_SIGN_KEY_SIZE]; - rc = smb2_get_sign_key(le64_to_cpu(shdr->SessionId), server, key); + rc = smb3_get_sign_key(le64_to_cpu(shdr->SessionId), server, key); if (unlikely(rc)) { cifs_server_dbg(FYI, "%s: Could not get signing key\n", __func__); return rc;
Customers have reported use-after-free in @ses->auth_key.response with SMB2.1 + sign mounts which occurs due to following race: task A task B cifs_mount() dfs_mount_share() get_session() cifs_mount_get_session() cifs_send_recv() cifs_get_smb_ses() compound_send_recv() cifs_setup_session() smb2_setup_request() kfree_sensitive() smb2_calc_signature() crypto_shash_setkey() *UAF* Fix this by ensuring that we have a valid @ses->auth_key.response by checking whether @ses->ses_status is SES_GOOD or SES_EXITING with @ses->ses_lock held. After commit 24a9799aa8ef ("smb: client: fix UAF in smb2_reconnect_server()"), we made sure to call ->logoff() only when @ses was known to be good (e.g. valid ->auth_key.response), so it's safe to access signing key when @ses->ses_status == SES_EXITING. Reported-by: Jay Shin <jaeshin@redhat.com> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com> --- fs/smb/client/smb2proto.h | 2 -- fs/smb/client/smb2transport.c | 56 +++++++++++++++++++++++++---------- 2 files changed, 40 insertions(+), 18 deletions(-)