jcmturner / gokrb5

Pure Go Kerberos library for clients and services
Apache License 2.0
727 stars 249 forks source link

question: cross-realm support #130

Open trung opened 6 years ago

trung commented 6 years ago

Please bare with me my limited knowledge about the domain language i'm using as I'm pretty new to this area.

In my use case, i need to access a resource which belongs to different realm as shown in the example below:

 foo.com   <----- trust -----> bar.com
    /                             \
trung@foo.com                    files.bar.com

I read this article and understand that ultimately, trung@foo.com needs to get a service ticket from bar.com KDC in order to access files.bar.com. However there's some dancing TGT exchanges in which foo.com offers a referral TGT and this would be used to exchange to bar.com TGT.

In Windows, with SSPI, it just works as I think Windows automatically performs some heavy lifting in terms of TGT exchanges. But in Linux, with your library, I'm unable to figure out the right sequence to achieve the above.

Appreciate any advice on the topic.

trung commented 6 years ago

When reading through the code i believe referral scenario has been taken care in the code. However, when running in my real environment, it returns an error:

KRB_AP_ERR_BADMATCH Ticket and authenticator don't match

And i found this

The name and realm of the client from the ticket are compared against the same fields in the authenticator. If they don't match, the KRB_AP_ERR_BADMATCH error is returned (they might not match, for example, if the wrong session key was used to encrypt the authenticator)

Not sure if it's related to the fact that i'm using Credentials Cache to initiate the Kerberos Client.

I'm unable to proceed further after this.

trung commented 6 years ago

I think I found the issue, will raise PR for review.

In a nutshell, the TGT ticket realm is not the same as the client realm due to multiple levels of client realm involved. My example above needs to be changed to illustrate the issue. Here we are

 foo.com   <----- trust -----> bar.com
    /                             \
sub.foo.com                       files.bar.com
  /
trung@sub.foo.com 

The library is doing the right thing by recursively obtaining the krbtgt for sub.foo.com followed by foo.com but when constructing the authenticator, it always uses the ticket realm which eventually is foo.com whereas client realm is sub.foo.com. This cause the KRB_AP_ERR_BADMATCH.

Not sure if this is due to some misconfiguration in the Microsoft Active Directory or it's really a bug in the library.

The change is simple, passing along client realm and use it to construct the authenticator.

jcmturner commented 6 years ago

Thanks for investigating. More complex set ups like this are difficult to configure to test against.

Happy to receive a PR. Please take a look at the contribution guide

samirkut commented 5 years ago

Was this ever merged? I am seeing an error with domain trust where I get a referral and this could perhaps explain it.

jcmturner commented 4 years ago

An issue with respect to referral was raised in #321 with an associated PR to fix it. I have merged this into master and will be looking to make a new release when some other bug fixes are merged in. I'd suggest testing with this version when it is released to see if this is related and the PR has fixed this.

dhrkumar commented 1 year ago

@jcmturner I’m facing the same issue and found @trung analysis helpful to find a workaround. Is it fine if I raise a PR for review?

momiji commented 1 year ago

I confirm I have the same issue, due to cross domain... In my case, we go from x.domain.com to domain.com then down to y.domain.com. the authenticator is created on domain.com instead if x.domain.com.

I have absolutely no knowledge on kerberos stuff, but based on the above comment, a temporary (and probably partial) fix that is working in MY situation on v8.4.4 is to propagate the original kdcRealm to the PAData used to generate the authenticator.

Hope this helps, let me known if 1. this is enough and 2. you want a PR that can/will be accepted and if more is required for the PR, as I don't have much time...

diff --git a/vendor/github.com/jcmturner/gokrb5/v8/client/TGSExchange.go b/vendor/github.com/jcmturner/gokrb5/v8/client/TGSExchange.go
index fd01342..5ac3831 100644
--- a/vendor/github.com/jcmturner/gokrb5/v8/client/TGSExchange.go
+++ b/vendor/github.com/jcmturner/gokrb5/v8/client/TGSExchange.go
@@ -10,17 +10,17 @@ import (

 // TGSREQGenerateAndExchange generates the TGS_REQ and performs a TGS exchange to retrieve a ticket to the specified SPN.
 func (cl *Client) TGSREQGenerateAndExchange(spn types.PrincipalName, kdcRealm string, tgt messages.Ticket, sessionKey types.EncryptionKey, renewal bool) (tgsReq messages.TGSReq, tgsRep messages.TGSRep, err error) {
-       tgsReq, err = messages.NewTGSReq(cl.Credentials.CName(), kdcRealm, cl.Config, tgt, sessionKey, spn, renewal)
+       tgsReq, err = messages.NewTGSReq(cl.Credentials.CName(), kdcRealm, kdcRealm, cl.Config, tgt, sessionKey, spn, renewal)
        if err != nil {
                return tgsReq, tgsRep, krberror.Errorf(err, krberror.KRBMsgError, "TGS Exchange Error: failed to generate a new TGS_REQ")
        }
-       return cl.TGSExchange(tgsReq, kdcRealm, tgsRep.Ticket, sessionKey, 0)
+       return cl.TGSExchange(tgsReq, kdcRealm, kdcRealm, tgsRep.Ticket, sessionKey, 0)
 }

 // TGSExchange exchanges the provided TGS_REQ with the KDC to retrieve a TGS_REP.
 // Referrals are automatically handled.
 // The client's cache is updated with the ticket received.
-func (cl *Client) TGSExchange(tgsReq messages.TGSReq, kdcRealm string, tgt messages.Ticket, sessionKey types.EncryptionKey, referral int) (messages.TGSReq, messages.TGSRep, error) {
+func (cl *Client) TGSExchange(tgsReq messages.TGSReq, paRealm string, kdcRealm string, tgt messages.Ticket, sessionKey types.EncryptionKey, referral int) (messages.TGSReq, messages.TGSRep, error) {
        var tgsRep messages.TGSRep
        b, err := tgsReq.Marshal()
        if err != nil {
@@ -60,11 +60,11 @@ func (cl *Client) TGSExchange(tgsReq messages.TGSReq, kdcRealm string, tgt messa
                                return tgsReq, tgsRep, err
                        }
                }
-               tgsReq, err = messages.NewTGSReq(cl.Credentials.CName(), realm, cl.Config, tgsRep.Ticket, tgsRep.DecryptedEncPart.Key, tgsReq.ReqBody.SName, tgsReq.Renewal)
+               tgsReq, err = messages.NewTGSReq(cl.Credentials.CName(), paRealm, realm, cl.Config, tgsRep.Ticket, tgsRep.DecryptedEncPart.Key, tgsReq.ReqBody.SName, tgsReq.Renewal)
                if err != nil {
                        return tgsReq, tgsRep, err
                }
-               return cl.TGSExchange(tgsReq, realm, tgsRep.Ticket, tgsRep.DecryptedEncPart.Key, referral)
+               return cl.TGSExchange(tgsReq, paRealm, realm, tgsRep.Ticket, tgsRep.DecryptedEncPart.Key, referral)
        }
        cl.cache.addEntry(
                tgsRep.Ticket,
diff --git a/vendor/github.com/jcmturner/gokrb5/v8/messages/KDCReq.go b/vendor/github.com/jcmturner/gokrb5/v8/messages/KDCReq.go
index 3745afe..46bc5d6 100644
--- a/vendor/github.com/jcmturner/gokrb5/v8/messages/KDCReq.go
+++ b/vendor/github.com/jcmturner/gokrb5/v8/messages/KDCReq.go
@@ -154,12 +154,12 @@ func NewASReq(realm string, c *config.Config, cname, sname types.PrincipalName)
 }

 // NewTGSReq generates a new KRB_TGS_REQ struct.
-func NewTGSReq(cname types.PrincipalName, kdcRealm string, c *config.Config, tgt Ticket, sessionKey types.EncryptionKey, sname types.PrincipalName, renewal bool) (TGSReq, error) {
+func NewTGSReq(cname types.PrincipalName, paRealm string, kdcRealm string, c *config.Config, tgt Ticket, sessionKey types.EncryptionKey, sname types.PrincipalName, renewal bool) (TGSReq, error) {
        a, err := tgsReq(cname, sname, kdcRealm, renewal, c)
        if err != nil {
                return a, err
        }
-       err = a.setPAData(tgt, sessionKey)
+       err = a.setPAData(paRealm, tgt, sessionKey)
        return a, err
 }

@@ -171,7 +171,7 @@ func NewUser2UserTGSReq(cname types.PrincipalName, kdcRealm string, c *config.Co
        }
        a.ReqBody.AdditionalTickets = []Ticket{verifyingTGT}
        types.SetFlag(&a.ReqBody.KDCOptions, flags.EncTktInSkey)
-       err = a.setPAData(clientTGT, sessionKey)
+       err = a.setPAData(clientTGT.Realm, clientTGT, sessionKey)
        return a, err
 }

@@ -226,7 +226,7 @@ func tgsReq(cname, sname types.PrincipalName, kdcRealm string, renewal bool, c *
        }, nil
 }

-func (k *TGSReq) setPAData(tgt Ticket, sessionKey types.EncryptionKey) error {
+func (k *TGSReq) setPAData(paRealm string, tgt Ticket, sessionKey types.EncryptionKey) error {
        // Marshal the request and calculate checksum
        b, err := k.ReqBody.Marshal()
        if err != nil {
@@ -243,7 +243,7 @@ func (k *TGSReq) setPAData(tgt Ticket, sessionKey types.EncryptionKey) error {

        // Form PAData for TGS_REQ
        // Create authenticator
-       auth, err := types.NewAuthenticator(tgt.Realm, k.ReqBody.CName)
+       auth, err := types.NewAuthenticator(paRealm, k.ReqBody.CName)
        if err != nil {
                return krberror.Errorf(err, krberror.KRBMsgError, "error generating new authenticator")
        }
snqk commented 1 year ago

I'd build on @momiji's work, as his patch unfortunately wasn't sufficient to make it work in my case.

My interpretation of the scenario:

       foo.com <--- trust ---> bar.com
         /                         \
     sub.foo.com           files.bar.com
       /                             \
 trung@sub.foo.com    HTTP/servicefqdn@files.bar.com

With @momiji's fixes, the exchange fails for me when I try to exchange krbtgt@files.bar.com (obtained from bar.com) for a session ticket ticket (from files.bar.com) against SPN HTTP/servicefqdn@files.bar.com.

This happens as in most places where the call to TGSREQGenerateAndExchange originates, the realm is the same as the client's: https://github.com/jcmturner/gokrb5/blob/47cd2e7744531465a983bf457bac38e6ad8f4684/v8/client/session.go#L225 https://github.com/jcmturner/gokrb5/blob/47cd2e7744531465a983bf457bac38e6ad8f4684/v8/client/client.go#L228

except: https://github.com/jcmturner/gokrb5/blob/47cd2e7744531465a983bf457bac38e6ad8f4684/v8/client/TGSExchange.go#L103

..which handles the last leg of the journey. Here it simply uses the realm for the target SPN, which even with @momiji's changes evaluates to files.bar.com. However since the original client was from sub.foo.com, we get a KRB_AP_ERR_BADMATCH.

We can resolve this fairly elegantly without too many changes, as we know that paRealm is always cl.Credentials.Domain().

I'm happy to raise this into a PR if this works for people.

diff --git a/v8/messages/KDCReq.go b/v8/messages/KDCReq.go
--- a/v8/messages/KDCReq.go (revision 47cd2e7744531465a983bf457bac38e6ad8f4684)
+++ b/v8/messages/KDCReq.go (date 1683256890660)
@@ -154,12 +154,12 @@
 }

 // NewTGSReq generates a new KRB_TGS_REQ struct.
-func NewTGSReq(cname types.PrincipalName, kdcRealm string, c *config.Config, tgt Ticket, sessionKey types.EncryptionKey, sname types.PrincipalName, renewal bool) (TGSReq, error) {
+func NewTGSReq(cname types.PrincipalName, paRealm, kdcRealm string, c *config.Config, tgt Ticket, sessionKey types.EncryptionKey, sname types.PrincipalName, renewal bool) (TGSReq, error) {
    a, err := tgsReq(cname, sname, kdcRealm, renewal, c)
    if err != nil {
        return a, err
    }
-   err = a.setPAData(tgt, sessionKey)
+   err = a.setPAData(paRealm, tgt, sessionKey)
    return a, err
 }

@@ -171,7 +171,7 @@
    }
    a.ReqBody.AdditionalTickets = []Ticket{verifyingTGT}
    types.SetFlag(&a.ReqBody.KDCOptions, flags.EncTktInSkey)
-   err = a.setPAData(clientTGT, sessionKey)
+   err = a.setPAData(clientTGT.Realm, clientTGT, sessionKey)
    return a, err
 }

@@ -226,7 +226,7 @@
    }, nil
 }

-func (k *TGSReq) setPAData(tgt Ticket, sessionKey types.EncryptionKey) error {
+func (k *TGSReq) setPAData(paRealm string, tgt Ticket, sessionKey types.EncryptionKey) error {
    // Marshal the request and calculate checksum
    b, err := k.ReqBody.Marshal()
    if err != nil {
@@ -243,7 +243,7 @@

    // Form PAData for TGS_REQ
    // Create authenticator
-   auth, err := types.NewAuthenticator(tgt.Realm, k.ReqBody.CName)
+   auth, err := types.NewAuthenticator(paRealm, k.ReqBody.CName)
    if err != nil {
        return krberror.Errorf(err, krberror.KRBMsgError, "error generating new authenticator")
    }
diff --git a/v8/client/TGSExchange.go b/v8/client/TGSExchange.go
--- a/v8/client/TGSExchange.go  (revision 47cd2e7744531465a983bf457bac38e6ad8f4684)
+++ b/v8/client/TGSExchange.go  (date 1683260235556)
@@ -10,7 +10,7 @@

 // TGSREQGenerateAndExchange generates the TGS_REQ and performs a TGS exchange to retrieve a ticket to the specified SPN.
 func (cl *Client) TGSREQGenerateAndExchange(spn types.PrincipalName, kdcRealm string, tgt messages.Ticket, sessionKey types.EncryptionKey, renewal bool) (tgsReq messages.TGSReq, tgsRep messages.TGSRep, err error) {
-   tgsReq, err = messages.NewTGSReq(cl.Credentials.CName(), kdcRealm, cl.Config, tgt, sessionKey, spn, renewal)
+   tgsReq, err = messages.NewTGSReq(cl.Credentials.CName(), cl.Credentials.Domain(), kdcRealm, cl.Config, tgt, sessionKey, spn, renewal)
    if err != nil {
        return tgsReq, tgsRep, krberror.Errorf(err, krberror.KRBMsgError, "TGS Exchange Error: failed to generate a new TGS_REQ")
    }
@@ -60,7 +60,7 @@
                return tgsReq, tgsRep, err
            }
        }
-       tgsReq, err = messages.NewTGSReq(cl.Credentials.CName(), realm, cl.Config, tgsRep.Ticket, tgsRep.DecryptedEncPart.Key, tgsReq.ReqBody.SName, tgsReq.Renewal)
+       tgsReq, err = messages.NewTGSReq(cl.Credentials.CName(), cl.Credentials.Domain(), realm, cl.Config, tgsRep.Ticket, tgsRep.DecryptedEncPart.Key, tgsReq.ReqBody.SName, tgsReq.Renewal)
        if err != nil {
            return tgsReq, tgsRep, err
        }
snqk commented 1 year ago

I just came across this snippet of RFC4120#1.2.

It is important for the end-service to know which realms were transited when deciding how much faith to place in the authentication process. To facilitate this decision, a field in each ticket contains the names of the realms that were involved in authenticating the client.

Both the ticket and KDC-REP publish the crealm field:

   Ticket          ::= [APPLICATION 1] SEQUENCE {
           tkt-vno         [0] INTEGER (5),
           realm           [1] Realm,
           sname           [2] PrincipalName,
           enc-part        [3] EncryptedData -- EncTicketPart
   }

   -- Encrypted part of ticket
   EncTicketPart   ::= [APPLICATION 3] SEQUENCE {
           flags                   [0] TicketFlags,
           key                     [1] EncryptionKey,
           crealm                  [2] Realm,
           cname                   [3] PrincipalName,
           transited               [4] TransitedEncoding,
           authtime                [5] KerberosTime,
           starttime               [6] KerberosTime OPTIONAL,
           endtime                 [7] KerberosTime,
           renew-till              [8] KerberosTime OPTIONAL,
           caddr                   [9] HostAddresses OPTIONAL,
           authorization-data      [10] AuthorizationData OPTIONAL
   }
   KDC-REP         ::= SEQUENCE {
           pvno            [0] INTEGER (5),
           msg-type        [1] INTEGER (11 -- AS -- | 13 -- TGS --),
           padata          [2] SEQUENCE OF PA-DATA OPTIONAL
                                   -- NOTE: not empty --,
           crealm          [3] Realm,
           cname           [4] PrincipalName,
           ticket          [5] Ticket,
           enc-part        [6] EncryptedData
                                   -- EncASRepPart or EncTGSRepPart,
                                   -- as appropriate
   }

It seems to be intended for verification instead, so don't think it's wise to make use of it for the purposes of crafting the authenticator.

If the reply message type is KRB_AS_REP, then the client verifies that the cname and crealm fields in the cleartext portion of the reply match what it requested.

dhrkumar commented 1 year ago

@snqk My domain forest scenario was almost like yours. I had a similar solution(not an expert in Go) but instead of introducing a new parameter I changed cname name and type to pass credentials reference. I’m happy to raise PR or review PR.

joanlopez commented 10 months ago

Hey @momiji, @snqk and anyone else involved in this issue,

I've opened a new pull request (https://github.com/jcmturner/gokrb5/pull/534) with your suggestion solution (thus I added you as co-authors), which also includes a new Go integration test that helped me reproduce the issue in the testing environment, as suggested by @jcmturner.

I hope this can receive some love, and get merged.

Thanks!

snqk commented 6 months ago

Thanks @joanlopez!

I'm afraid it does look like there is little we can do to avoid a v9 fork, though it's definitely worth it.

@jcmturner, friendly ping for a review, keen to hear your thoughts on this. I'm happy to lend a hand where needed to help expedite.