| Message ID | 1310393813-12443-1-git-send-email-jlayton@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 11, 2011 at 9:16 AM, Jeff Layton <jlayton@redhat.com> wrote: > Sniffing traffic on the wire shows that windows clients send a zeroed > out signature field in a NEGOTIATE request, and send "BSRSPYL" in the > signature field during SESSION_SETUP. Make the cifs client behave the > same way. > > It doesn't seem to make much difference in any server that I've tested > against, but it's probably best to follow windows behavior as closely as > possible here. > > Signed-off-by: Jeff Layton <jlayton@redhat.com> > --- > fs/cifs/cifsencrypt.c | 16 ++++++++++++++-- > 1 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c > index 5a0ee7f..45ae18e 100644 > --- a/fs/cifs/cifsencrypt.c > +++ b/fs/cifs/cifsencrypt.c > @@ -77,9 +77,15 @@ int cifs_sign_smb(struct smb_hdr *cifs_pdu, struct TCP_Server_Info *server, > if ((cifs_pdu == NULL) || (server == NULL)) > return -EINVAL; > > - if ((cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) == 0) > + if (!(cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) || > + server->tcpStatus == CifsNeedNegotiate) Jeff, why do we need to set the tcpStatus here? Not sure what would be a value of tcpStatus at this point. > return rc; > > + if (!server->session_estab) { > + strncpy(cifs_pdu->Signature.SecuritySignature, "BSRSPYL", 8); > + return rc; > + } > + > cifs_pdu->Signature.Sequence.SequenceNumber = > cpu_to_le32(server->sequence_number); > cifs_pdu->Signature.Sequence.Reserved = 0; > @@ -154,9 +160,15 @@ int cifs_sign_smb2(struct kvec *iov, int n_vec, struct TCP_Server_Info *server, > if ((cifs_pdu == NULL) || (server == NULL)) > return -EINVAL; > > - if ((cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) == 0) > + if (!(cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) || > + server->tcpStatus == CifsNeedNegotiate) > return rc; > > + if (!server->session_estab) { > + strncpy(cifs_pdu->Signature.SecuritySignature, "BSRSPYL", 8); > + return rc; > + } > + > cifs_pdu->Signature.Sequence.SequenceNumber = > cpu_to_le32(server->sequence_number); > cifs_pdu->Signature.Sequence.Reserved = 0; > -- > 1.7.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 11 Jul 2011 09:28:24 -0500 Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote: > On Mon, Jul 11, 2011 at 9:16 AM, Jeff Layton <jlayton@redhat.com> wrote: > > Sniffing traffic on the wire shows that windows clients send a zeroed > > out signature field in a NEGOTIATE request, and send "BSRSPYL" in the > > signature field during SESSION_SETUP. Make the cifs client behave the > > same way. > > > > It doesn't seem to make much difference in any server that I've tested > > against, but it's probably best to follow windows behavior as closely as > > possible here. > > > > Signed-off-by: Jeff Layton <jlayton@redhat.com> > > --- > > fs/cifs/cifsencrypt.c | 16 ++++++++++++++-- > > 1 files changed, 14 insertions(+), 2 deletions(-) > > > > diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c > > index 5a0ee7f..45ae18e 100644 > > --- a/fs/cifs/cifsencrypt.c > > +++ b/fs/cifs/cifsencrypt.c > > @@ -77,9 +77,15 @@ int cifs_sign_smb(struct smb_hdr *cifs_pdu, struct TCP_Server_Info *server, > > if ((cifs_pdu == NULL) || (server == NULL)) > > return -EINVAL; > > > > - if ((cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) == 0) > > + if (!(cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) || > > + server->tcpStatus == CifsNeedNegotiate) > > Jeff, why do we need to set the tcpStatus here? Not sure what would be > a value of tcpStatus at this point. > Uhh...that's a check to see if it's in CifsNeedNegotiate. If it is, then we don't want to do anything -- just leave it zeroed out. > > return rc; > > > > + if (!server->session_estab) { > > + strncpy(cifs_pdu->Signature.SecuritySignature, "BSRSPYL", 8); > > + return rc; > > + } > > + > > cifs_pdu->Signature.Sequence.SequenceNumber = > > cpu_to_le32(server->sequence_number); > > cifs_pdu->Signature.Sequence.Reserved = 0; > > @@ -154,9 +160,15 @@ int cifs_sign_smb2(struct kvec *iov, int n_vec, struct TCP_Server_Info *server, > > if ((cifs_pdu == NULL) || (server == NULL)) > > return -EINVAL; > > > > - if ((cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) == 0) > > + if (!(cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) || > > + server->tcpStatus == CifsNeedNegotiate) > > return rc; > > > > + if (!server->session_estab) { > > + strncpy(cifs_pdu->Signature.SecuritySignature, "BSRSPYL", 8); > > + return rc; > > + } > > + > > cifs_pdu->Signature.Sequence.SequenceNumber = > > cpu_to_le32(server->sequence_number); > > cifs_pdu->Signature.Sequence.Reserved = 0; > > -- > > 1.7.6 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > >
On Mon, Jul 11, 2011 at 9:16 AM, Jeff Layton <jlayton@redhat.com> wrote: > Sniffing traffic on the wire shows that windows clients send a zeroed > out signature field in a NEGOTIATE request, and send "BSRSPYL" in the > signature field during SESSION_SETUP. Make the cifs client behave the > same way. > > It doesn't seem to make much difference in any server that I've tested > against, but it's probably best to follow windows behavior as closely as > possible here. > > Signed-off-by: Jeff Layton <jlayton@redhat.com> > --- > fs/cifs/cifsencrypt.c | 16 ++++++++++++++-- > 1 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c > index 5a0ee7f..45ae18e 100644 > --- a/fs/cifs/cifsencrypt.c > +++ b/fs/cifs/cifsencrypt.c > @@ -77,9 +77,15 @@ int cifs_sign_smb(struct smb_hdr *cifs_pdu, struct TCP_Server_Info *server, > if ((cifs_pdu == NULL) || (server == NULL)) > return -EINVAL; > > - if ((cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) == 0) > + if (!(cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) || > + server->tcpStatus == CifsNeedNegotiate) > return rc; > > + if (!server->session_estab) { > + strncpy(cifs_pdu->Signature.SecuritySignature, "BSRSPYL", 8); > + return rc; > + } > + > cifs_pdu->Signature.Sequence.SequenceNumber = > cpu_to_le32(server->sequence_number); > cifs_pdu->Signature.Sequence.Reserved = 0; > @@ -154,9 +160,15 @@ int cifs_sign_smb2(struct kvec *iov, int n_vec, struct TCP_Server_Info *server, > if ((cifs_pdu == NULL) || (server == NULL)) > return -EINVAL; > > - if ((cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) == 0) > + if (!(cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) || > + server->tcpStatus == CifsNeedNegotiate) > return rc; > > + if (!server->session_estab) { > + strncpy(cifs_pdu->Signature.SecuritySignature, "BSRSPYL", 8); > + return rc; > + } > + > cifs_pdu->Signature.Sequence.SequenceNumber = > cpu_to_le32(server->sequence_number); > cifs_pdu->Signature.Sequence.Reserved = 0; > -- > 1.7.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Looks correct. Reviewed-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c index 5a0ee7f..45ae18e 100644 --- a/fs/cifs/cifsencrypt.c +++ b/fs/cifs/cifsencrypt.c @@ -77,9 +77,15 @@ int cifs_sign_smb(struct smb_hdr *cifs_pdu, struct TCP_Server_Info *server, if ((cifs_pdu == NULL) || (server == NULL)) return -EINVAL; - if ((cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) == 0) + if (!(cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) || + server->tcpStatus == CifsNeedNegotiate) return rc; + if (!server->session_estab) { + strncpy(cifs_pdu->Signature.SecuritySignature, "BSRSPYL", 8); + return rc; + } + cifs_pdu->Signature.Sequence.SequenceNumber = cpu_to_le32(server->sequence_number); cifs_pdu->Signature.Sequence.Reserved = 0; @@ -154,9 +160,15 @@ int cifs_sign_smb2(struct kvec *iov, int n_vec, struct TCP_Server_Info *server, if ((cifs_pdu == NULL) || (server == NULL)) return -EINVAL; - if ((cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) == 0) + if (!(cifs_pdu->Flags2 & SMBFLG2_SECURITY_SIGNATURE) || + server->tcpStatus == CifsNeedNegotiate) return rc; + if (!server->session_estab) { + strncpy(cifs_pdu->Signature.SecuritySignature, "BSRSPYL", 8); + return rc; + } + cifs_pdu->Signature.Sequence.SequenceNumber = cpu_to_le32(server->sequence_number); cifs_pdu->Signature.Sequence.Reserved = 0;
Sniffing traffic on the wire shows that windows clients send a zeroed out signature field in a NEGOTIATE request, and send "BSRSPYL" in the signature field during SESSION_SETUP. Make the cifs client behave the same way. It doesn't seem to make much difference in any server that I've tested against, but it's probably best to follow windows behavior as closely as possible here. Signed-off-by: Jeff Layton <jlayton@redhat.com> --- fs/cifs/cifsencrypt.c | 16 ++++++++++++++-- 1 files changed, 14 insertions(+), 2 deletions(-)