| Message ID | CAH2r5muT0jBfh_K230dtNW5ZkVFx+evHiDA=+yoeG_PvDkCxmA@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2014-08-20 at 23:51 -0500, Steve French wrote: > This is an unusual sounding issue. Any comments on this from the auth experts? > > Seems better to investigate this more if we end up enforcing a "must > be within 5 minutes" threshold instead of this patch. Have we done a > dochelp on this before? I am certainly nervous about this patch, as I've not ever seen this before. The thing that makes me feel particularly odd about this is that: In general, NTLMSSP clients don't have the server's time, and certainly don't have the domain controller's time. (That CIFS provides this does not mean we should use it, NTLMSSP is a general protocol and adding CIFS-specific hacks indicates we are understanding it wrong, in my experience). BTW, the domain controller is the only element here that could check the embedded time, but I'll grant that typically servers are better in sync with each other than this embedded device might be. The 5 mins stuff probably refers to Kerberos, which does have such a time limit. I've never seen NTLMSSP fail against windows due to clock skew. I would like to see much more investigation here before this is done, because if you just trust the server's time and if you need to, to pass a security check, you override that check. We need to understand why it is in place. Thanks, Andrew Bartlett
On Wed, Aug 20, 2014 at 11:51:02PM -0500, Steve French wrote: > This is an unusual sounding issue. Any comments on this from the auth experts? > > Seems better to investigate this more if we end up enforcing a "must > be within 5 minutes" threshold instead of this patch. Have we done a > dochelp on this before? No we have not. I'd feel happier if you could add some test code to smbtorture that showed NTLM authentication succeeding with correct time, and it failing with a large time drift, that passes against a Windows server. Jeremy. -- 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 Fri, 2014-08-22 at 14:32 +1200, Andrew Bartlett wrote: > On Wed, 2014-08-20 at 23:51 -0500, Steve French wrote: > > This is an unusual sounding issue. Any comments on this from the auth experts? > > > > Seems better to investigate this more if we end up enforcing a "must > > be within 5 minutes" threshold instead of this patch. Have we done a > > dochelp on this before? > > I am certainly nervous about this patch, as I've not ever seen this > before. The thing that makes me feel particularly odd about this is > that: In general, NTLMSSP clients don't have the server's time, This is simply false. Modern servers send the server timestamp in the TargetInfo Av_Pair structure in the challenge message [see MS-NLMP 2.2.2.1]. In [MS-NLMP 3.1.5.1.2] it is explicitly mentioned that the client must use the provided (from the server) timestamp if present or current time if it is not. Not only a modern server *should* send a timestamp, but a modern client, on seeing that a timestamp is available should provide a MIC in the Authentication message [MS-NLMP 3.1.5.1.2]. Unfortunately samba at the moment has a very limited and incomplete implementation of the NTLM protocol and doesn't do much with targetinfo except mocking it up a little. An implementation that is very close to implementing all of the MS-NLMP specification is here: https://git.samba.org/?p=idra/gss-ntlmssp.git;a=summary or here: https://fedorahosted.org/gss-ntlmssp/ and I hope to make samba use it eventually, when we have a more decent set of interfaces to connect to DCs (the current winbind interfacing is suboptimal but fixable). > and certainly don't have the domain controller's time. This is true but not a problem today were servers are supposed to have the time in sync with the KDC/Domain Controller anyway. > (That CIFS provides > this does not mean we should use it, NTLMSSP is a general protocol and > adding CIFS-specific hacks indicates we are understanding it wrong, in > my experience). > > BTW, the domain controller is the only element here that could check the > embedded time, but I'll grant that typically servers are better in sync > with each other than this embedded device might be. > > The 5 mins stuff probably refers to Kerberos, which does have such a > time limit. I've never seen NTLMSSP fail against windows due to clock > skew. That's becasue a client is supposed to use the time provided by the server, so no clockskew is really possible, > I would like to see much more investigation here before this is done, > because if you just trust the server's time and if you need to, to pass > a security check, you override that check. We need to understand why it > is in place. Sorry but this makes no sense, if you can bypass a "security mechanism" in the client, then there is *no* security mechanism. The reason why a timestamp is provided by the server is (most probably) to mitigate replay attacks when the client uses a MIC (as it should) when the timestamp is provided. Simo. -- 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
Am 22.08.2014 um 06:17 schrieb Simo: > On Fri, 2014-08-22 at 14:32 +1200, Andrew Bartlett wrote: >> On Wed, 2014-08-20 at 23:51 -0500, Steve French wrote: >>> This is an unusual sounding issue. Any comments on this from the auth experts? >>> >>> Seems better to investigate this more if we end up enforcing a "must >>> be within 5 minutes" threshold instead of this patch. Have we done a >>> dochelp on this before? >> >> I am certainly nervous about this patch, as I've not ever seen this >> before. The thing that makes me feel particularly odd about this is >> that: In general, NTLMSSP clients don't have the server's time, > > This is simply false. > Modern servers send the server timestamp in the TargetInfo Av_Pair > structure in the challenge message [see MS-NLMP 2.2.2.1]. > > In [MS-NLMP 3.1.5.1.2] it is explicitly mentioned that the client must > use the provided (from the server) timestamp if present or current time > if it is not. I talks about the MsvAvTimestamp from CHALLENGE_MESSAGE.TargetInfo.Value not the timestamp from smb negprot. I think it would make sense to skip the timestamp if the client doesn't find the server time in CHALLENGE_MESSAGE.TargetInfo.Value and notices that the local time isn't correct. E.g. the date is before the year 2000. metze
On Fri, 2014-08-22 at 09:12 +0200, Stefan (metze) Metzmacher wrote: > Am 22.08.2014 um 06:17 schrieb Simo: > > On Fri, 2014-08-22 at 14:32 +1200, Andrew Bartlett wrote: > >> On Wed, 2014-08-20 at 23:51 -0500, Steve French wrote: > >>> This is an unusual sounding issue. Any comments on this from the auth experts? > >>> > >>> Seems better to investigate this more if we end up enforcing a "must > >>> be within 5 minutes" threshold instead of this patch. Have we done a > >>> dochelp on this before? > >> > >> I am certainly nervous about this patch, as I've not ever seen this > >> before. The thing that makes me feel particularly odd about this is > >> that: In general, NTLMSSP clients don't have the server's time, > > > > This is simply false. > > Modern servers send the server timestamp in the TargetInfo Av_Pair > > structure in the challenge message [see MS-NLMP 2.2.2.1]. > > > > In [MS-NLMP 3.1.5.1.2] it is explicitly mentioned that the client must > > use the provided (from the server) timestamp if present or current time > > if it is not. > > I talks about the MsvAvTimestamp from CHALLENGE_MESSAGE.TargetInfo.Value > not the timestamp from smb negprot. Sure, the point is that the server *does* send a timestamp that the client is supposed to use. > I think it would make sense to skip the timestamp if the client doesn't > find the server time in CHALLENGE_MESSAGE.TargetInfo.Value > and notices that the local time isn't correct. E.g. the date is > before the year 2000. IT probably shouldn't use the timestamp at all if the challenge message does not have it because it means the server doesn't know what to do about it anyway. However using the timestamp found in the smb packet is as good as anything else, it won't hurt anything, unless the server has its time completely off and relay the authentication to a DC while the client had the clock right. Simo. -- 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 4934347..d5cec81 100644 --- a/fs/cifs/cifsencrypt.c +++ b/fs/cifs/cifsencrypt.c @@ -671,8 +671,8 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp) (ses->auth_key.response + CIFS_SESS_KEY_SIZE); ntlmv2->blob_signature = cpu_to_le32(0x00000101); ntlmv2->reserved = 0; - /* Must be within 5 minutes of the server */ - ntlmv2->time = cpu_to_le64(cifs_UnixTimeToNT(CURRENT_TIME)); + /* Hack to get around windows extended security */ + ntlmv2->time = cpu_to_le64(ses->serverTime); get_random_bytes(&ntlmv2->client_chal, sizeof(ntlmv2->client_chal)); ntlmv2->reserved2 = 0; diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index ce24c1f..9344c94 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -796,6 +796,8 @@ struct cifs_ses { enum securityEnum sectype; /* what security flavor was specified? */ bool sign; /* is signing required? */ bool need_reconnect:1; /* connection reset, uid now invalid */ + __u64 serverTime; /* Keeps a track of server time sent by server + during negotiate response */ #ifdef CONFIG_CIFS_SMB2 __u16 session_flags; char smb3signingkey[SMB3_SIGN_KEY_SIZE]; /* for signing smb3 packets */ diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 86a2aa5..ead2da0 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -584,6 +584,8 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses) if (rc != 0) goto neg_err_exit; + ses->serverTime = le32_to_cpu(pSMBr->SystemTimeLow); + ses->serverTime |= ((__u64)le32_to_cpu(pSMBr->SystemTimeHigh) << 32); server->dialect = le16_to_cpu(pSMBr->DialectIndex); cifs_dbg(FYI, "Dialect: %d\n", server->dialect); /* Check wct = 1 error case */ diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index ed42234..a40f492 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -381,6 +381,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses) if (rc != 0) goto neg_exit; + ses->serverTime = le64_to_cpu(rsp->SystemTime); cifs_dbg(FYI, "mode 0x%x\n", rsp->SecurityMode); /* BB we may eventually want to match the negotiated vs. requested