Kerberos.NET icon indicating copy to clipboard operation
Kerberos.NET copied to clipboard

Question: Is RequestServiceTicket.CanCacheTickets condition too restrictive ?

Open sqladmin-zz opened this issue 1 year ago • 1 comments

in this PR https://github.com/dotnet/Kerberos.NET/pull/249 CanCacheTickets property was introduced in order to fix issue https://github.com/dotnet/Kerberos.NET/issues/248

        public bool CanCacheTicket => this.CacheTicket ?? true &&
                                      string.IsNullOrWhiteSpace(this.S4uTarget) && // is this line needed ?
                                      this.S4uTicket == null &&
                                      this.S4uTargetCertificate == null;

But when we pass string s4u to GetServiceTicket - it looks like tickets can be cached - because s4u goes to Container string parameter in TicketCacheEntry and allows to correctly identify record in cache.

It will not break the test case described in https://github.com/dotnet/Kerberos.NET/issues/248

In our case we are calling http service from backend service on behalf of the user in many parallel threads - every call to GetServiceTicket(..., s4u: username) goes to KDC (because ticket for user is not cached) and we running out of SocketPool in case of many threads (even if TcpKerberosTransport.MaxPoolSize increased).

We have workarounds on this: locking call to GetServiceTicket or using SemaphoreSlim(TcpKerberosTransport.MaxPoolSize,TcpKerberosTransport.MaxPoolSize) before it ... but it looks too rude

sqladmin-zz avatar Nov 08 '24 04:11 sqladmin-zz

Ish. It's more designed to prevent polluting caches that don't understand S4U which would be very bad security-wise. Making that work requires reworking the cache system a bit.


From: sqladmin-zz @.> Sent: Thursday, November 7, 2024 8:13:42 PM To: dotnet/Kerberos.NET @.> Cc: Subscribed @.***> Subject: [dotnet/Kerberos.NET] Question: Is RequestServiceTicket.CanCacheTickets condition too restrictive ? (Issue #383)

in this PR #249https://github.com/dotnet/Kerberos.NET/pull/249 CanCacheTickets property was introduced in order to fix issue #248https://github.com/dotnet/Kerberos.NET/issues/248

    public bool CanCacheTicket => this.CacheTicket ?? true &&
                                  string.IsNullOrWhiteSpace(this.S4uTarget) && // is this line needed ?
                                  this.S4uTicket == null &&
                                  this.S4uTargetCertificate == null;

But when we pass string s4u to GetServiceTicket - it looks like tickets can be cached - because s4u goes to Container string parameter in TicketCacheEntry and allows to correctly identify record in cache.

It will not break the test case described in #248https://github.com/dotnet/Kerberos.NET/issues/248

In our case we are calling http service from backend service on behalf of the user in many parallel threads - every call to GetServiceTicket(..., s4u: username) goes to KDC (because ticket for user is not cached) and we running out of SocketPool in case of many threads (even if TcpKerberosTransport.MaxPoolSize increased).

We have workarounds on this: locking call to GetServiceTicket or using SemaphoreSlim(TcpKerberosTransport.MaxPoolSize,TcpKerberosTransport.MaxPoolSize) before it ... but it looks too rude

— Reply to this email directly, view it on GitHubhttps://github.com/dotnet/Kerberos.NET/issues/383 or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAJHTYJNRTYTXWLJGU6J4KTZ7Q27PBFKMF2HI4TJMJ2XIZLTSOBKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJLJONZXKZNENZQW2ZNLORUHEZLBMRPXI6LQMWBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTLDTOVRGUZLDORPXI6LQMWSUS43TOVS2M5DPOBUWG44SQKSHI6LQMWVHEZLQN5ZWS5DPOJ42K5TBNR2WLKBYGU2DQOJRGM4IFJDUPFYGLJLJONZXKZNFOZQWY5LFVIZDMNBSG44TANBSG6TXI4TJM5TWK4VGMNZGKYLUMU. You are receiving this email because you are subscribed to this thread.

Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

SteveSyfuhs avatar Nov 08 '24 04:11 SteveSyfuhs