[ 
https://issues.apache.org/jira/browse/DIRKRB-692?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Fabiano Tarlao updated DIRKRB-692:
----------------------------------
    Description: 
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

  was:
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


> 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
>            Reporter: Fabiano Tarlao
>            Priority: Major
>              Labels: easyfix
>
> 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



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to