Uploaded image for project: 'Directory Kerberos'
  1. Directory Kerberos
  2. DIRKRB-692

1)sgtTicket clientPrincipal is not initialized + 2)KrbClient fails to store SGT ticket in cache file

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 1.1.0
    • 1.1.1
    • None
    • Linux Mint 17.1 + Netbeans 8.1 with a Maven Project + kerb-core 1.1.0 in Windows AD (KDC is Windows 2016 Server)
    • Patch, Important

    Description

      Two bugs that interact each other

      1)

      SgtTicket, returned by KrbClient.requestSgt(..), has a null clientPrincipal field (unassigned). Perhaps this is not mandatory but your code assumes this property is populated (see later). I highly suggest to populate this field.

      I have wrote a workaround that overrides the requestSgt method and works for the USE_TGT case (the real complete fix is on GitHub,in a set of subsequent pull requests ):

       

      @Override
          public SgtTicket requestSgt(KOptions requestOptions) throws KrbException {
              SgtTicket requestSgt = super.requestSgt(requestOptions); 
              TgtTicket tgt = (TgtTicket) requestOptions.getOptionValue(KrbOption.USE_TGT);
              if(tgt != null){
                  requestSgt.setClientPrincipal(tgt.getClientPrincipal());
              }
              return requestSgt;
          }

       

      2)

      Creating a new credential cache file when storing only one SGT (service ticket) fails. (i.e., trying to create a new cache file containing only one SGT and no TGT)

      Invoking KrbClient.storeTicket(sgtTicket, File) fails for this reason (here is the original code in KrbClientBase class, my comments in RED ):

      public void storeTicket(SgtTicket sgtTicket, File ccacheFile) throws KrbException {
              LOG.info("Storing the sgt to the credential cache file.");
              if (!ccacheFile.exists()) {
                  createCacheFile(ccacheFile); //Correctly creates a new file but...
              if (ccacheFile.exists() && ccacheFile.canWrite()) {
                  CredentialCache cCache = new CredentialCache();
                  try {
                      cCache.load(ccacheFile); //..this line EXPLODES cause it tries to initialize from an empty file, the unexistent file case is not managed correctly
                      cCache.addCredential(new Credential(sgtTicket, sgtTicket.getClientPrincipal()));
                      cCache.setPrimaryPrincipal(sgtTicket.getClientPrincipal());
                      cCache.store(ccacheFile);
                  } catch (IOException e) {
                      throw new KrbException("Failed to store sgt", e);

              } else {
                  throw new IllegalArgumentException("Invalid ccache file, "
                          + "not exist or writable: " + ccacheFile.getAbsolutePath());
      }

      Here follows my proposal/fix, this code correctly manages the MIT ccache file creation for one SGT, please note that this fix assumes that bug 1 is already fixed:

       

      public void storeTicket(SgtTicket sgtTicket, File ccacheFile) throws KrbException {
              LOG.info("Storing the sgt to the credential cache file.");
              boolean isFreshNew = !ccacheFile.exists() || (ccacheFile.length() == 0);
              if (isFreshNew) {
                  createCacheFile(ccacheFile);
              }
              if (ccacheFile.exists() && ccacheFile.canWrite()) {
                  
                  try {
                      CredentialCache cCache;
                      if (!isFreshNew) {
                          cCache = new CredentialCache(); 
                          cCache.load(ccacheFile);
                          cCache.addCredential(new Credential(sgtTicket, sgtTicket.getClientPrincipal()));
                      } else {
                          //Remind: contructor sets the cCache client principal from the sgtTicket one
                          cCache = new CredentialCache(sgtTicket);
                      }
                      cCache.store(ccacheFile);
                  } catch (IOException e) {
                      throw new KrbException("Failed to store sgt", e);
                  }
              } else {
                  throw new IllegalArgumentException("Invalid ccache file, "
                          + "not exist or writable: " + ccacheFile.getAbsolutePath());
              }
          }
      

       

      Please note that YOUR CredentialCache contructor assumes the clientPrincipal is populated

      Hope useful,

      regards

      Fabiano

      P.s: Attached two paches for the project, bug2.diff for bug 2 is..I think.. production ready.

      bug1.diff for bug 1 is untested.. I ported the modification of my overriding method to the AbstractInternalKrbClient.requestSgt() method. But I have not performed run/test of modules, your project is big and needs time to manage well. BUT is this the right solution? or SHOULD the clientPrincipal be already populated from the doRequestSgt method? I suggest to look for a solution at a deeper level starting from DefaultInternalKrbClient. Consider bug1.diff a fair workaround.

      P.s.2:Added a pull request on GitHub that obsoletes the attached patches (now removed)

      Regards

      Attachments

        1. KrbClientTest.java
          2 kB
          Fabiano Tarlao

        Activity

          People

            Unassigned Unassigned
            ftarlao Fabiano Tarlao
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: