[ https://issues.apache.org/jira/browse/DIRKRB-692?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16354135#comment-16354135 ]
Fabiano Tarlao commented on DIRKRB-692: --------------------------------------- Attached patches, UPDATED the main post (please read PS) > 1)sgtTicket clientPrincipal is not initialized + 2)KrbClient fails to store > SGT ticket in cache file > ---------------------------------------------------------------------------------------------------- > > Key: DIRKRB-692 > URL: https://issues.apache.org/jira/browse/DIRKRB-692 > Project: Directory Kerberos > Issue Type: Bug > Affects Versions: 1.1.0 > Environment: Linux Mint 17.1 + Netbeans 8.1 with a Maven Project + > kerb-core 1.1.0 in Windows AD (KDC is Windows 2016 Server) > Reporter: Fabiano Tarlao > Priority: Major > Labels: easyfix > Attachments: bug1.diff, bug2.diff > > > Two bugs that interact each other > h1. 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: > > {code:java} > @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; > }{code} > > h1. 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);{color:#ff0000} //Correctly > creates a new file but...{color}}} > {{ if (ccacheFile.exists() && ccacheFile.canWrite()) {}} > {{ CredentialCache cCache = new CredentialCache();}} > {{ try {}} > {{ cCache.load(ccacheFile);{color:#ff0000} //..this line > EXPLODES cause it tries to initialize from an empty file, the unexistent file > case is not managed correctly{color}}} > {{ 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 static void storeTicket(SgtTicket sgtTicket, File ccacheFile) throws > KrbException {}} > {{ LOG.info("Storing the sgt to the credential cache file.");}} > {{ boolean isFreshNew = !ccacheFile.exists();}} > {{ if (isFreshNew) {}} > {{ createCacheFile(ccacheFile);}} > {{ if (ccacheFile.exists() && ccacheFile.canWrite()) {}} > {{ }} > {{ try {}} > {{ CredentialCache cCache;}} > {{ if(!isFreshNew){}} > {{ cCache = new CredentialCache(sgtTicket); > {color:#ff0000}//This constructor sets also the cCache principal from > sgtTicket principal{color}}} > {{ cCache.load(ccacheFile);}} > {{ cCache.addCredential(new Credential(sgtTicket, > sgtTicket.getClientPrincipal()));}} > {{ > cCache.setPrimaryPrincipal(sgtTicket.getClientPrincipal());}} > {{ } else {}} > {{ 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. > Regards -- This message was sent by Atlassian JIRA (v7.6.3#76005)