[
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 (the real complete fix is on GitHub,in a set of subsequent
pull requests :) ):
{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:
{code:java}
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());
}
}
{code}
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
Regards
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
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
Regards
> 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
> Fix For: 1.0.2, 1.1.1
>
> Attachments: KrbClientTest.java, 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 (the real complete fix is on GitHub,in a set of
> subsequent pull requests :) ):
>
> {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:
>
> {code:java}
> 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());
> }
> }
> {code}
>
> 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
> Regards
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
