4:30 p.m.
Patch accomplishes the following:
1. Read applet into memory to prepare to write to token.
2. With tpsclient create secure channel by implementing Initialize Update and
ExternalAuthenticate messages.
3. Support for MAC and encryption for messages going on after secure channel has been
created.
4. Implemented method to remove an aid file or instance from the token.
5. Added some symkey methods to allow TPS to manipulate session keys.
Have not tried this with real token as of yet. The tpsclient does verify of the MAC coming
from the server and decrypts encrypted messages. Decrypted messages have to be correct for
the MAC verification to work.
Next step will be to add the phone home servlet to the TPS and give it a try with a real
token and esc.
4:33 p.m.
New subject: Fwd: [pki-devel][PATCH] 0008-Further-progress-Format-operation.patch
Actually attach the patch this time...
----- Forwarded Message -----
From: "John Magne" <jmagne(a)redhat.com>
To: "pki-devel" <pki-devel(a)redhat.com>
Sent: Tuesday, April 8, 2014 2:30:05 PM
Subject: [pki-devel][PATCH] 0008-Further-progress-Format-operation.patch
Patch accomplishes the following:
1. Read applet into memory to prepare to write to token.
2. With tpsclient create secure channel by implementing Initialize Update and
ExternalAuthenticate messages.
3. Support for MAC and encryption for messages going on after secure channel has been
created.
4. Implemented method to remove an aid file or instance from the token.
5. Added some symkey methods to allow TPS to manipulate session keys.
Have not tried this with real token as of yet. The tpsclient does verify of the MAC coming
from the server and decrypts encrypted messages. Decrypted messages have to be correct for
the MAC verification to work.
Next step will be to add the phone home servlet to the TPS and give it a try with a real
token and esc.
1:22 p.m.
New subject: Fwd: [pki-devel][PATCH] 0008-Further-progress-Format-operation.patch
Please see my review comments below.
Christina
================
1. In APDU.java I'd prefer you kept the old name "secureMessage" instead
of "encryptData" for the function that calls Util.encryptData() for two
reasons:
a. It would be easier for people (like myself) to match the tps
tomcat code with the tps c++ code
b. Now you have two "encryptData" functions in two separate files --
one in util.java, and one in APDU.java. Could get confusing.
2. The specialURLEncode() function you added in Util.java looks almost
identical to SpecialEncode() in TKSKnownSessionKey.java.
How about filing a separate ticket for making TKSKnownSessionKey.java
call your version instead and remove the one in TKSKnownSessionKey.java?
3. for JNI function UnwrapSessionKeyWithSharedSecret, just wondering, in
JSS, there is a native function called nativeWrapSymWithSym, which is
mapped from pkcs11/PK11KeyWrapper.java:wrap(). have you looked into if
that would have been utilized? And if not, would your method be
something to be made generic and placed into jss? I am not sure if we
already have a ticket for this, but perhaps file one to put all the
functions from symkey that fit to be generic JSS functions to be moved
into JSS instead.
4. for JNI function GetSymKeyByName().
a. I think it's customary to initialize slot to be
PK11_GetInternalKeySlot(). So in the case of tokenName not supplied by
the caller, the function will at least try looking into internal token
by default. And if you must require tokenName, you should have checked
it at the very beginning along with other required param such as keyName.
b. you don't need to do if (tokenName) again after you already
checked it right before:
if(tokenName == NULL || sharedSecretKey == NULL ||
sessionKeyBA == NULL) {
goto loser;
}
c. Also, same as above. if you file a ticket from #3 (moving
appropriate things into JSS), this should be considered.
5. In ReturnSymKey(), you added some printf for debugging I presume.
since this is JNI C code, one should be careful on not making attempt to
print something that could turn out to be null. I suggest you add check
to null before trying to print.
Same comment applies to wherever else that's appropriate.
6. CreateDesKey24Bit is moved from RA::CreateDesKey24Byte()? I believe
the DES key is 24 "bytes" not "bits". You probably want to name it
back
to CreateDesKey24Byte?
7. In SecureChannel.java, I'm not sure I understand the value of the
constructor that does not take any parameters if you bomb them out if
any of the class variables are not set. If one calls new
SecureChannel() without any parameters how would those variables be set
if it just bombs you out right away?
8. For those APDU classes under common/src/org/dogtagpki/tps/apdu/,
I'd prefer you kept the APDU in the names of those APDU classes/files..
It would have made it much easier for the readers.
e.g. DeleteFileAPDU
9. in format(), the tksConnId is just temporarily hard coded to tks1
right? You might wan to add a TODO comment there to retrieve from
profile in config.
10. in getChannelBlockSize, would it possible to throw EBaseException
when you already have a default value? Same comment with other similar
getXXX() functions.
On 04/08/2014 02:33 PM, John Magne wrote:
Actually attach the patch this time...
----- Forwarded Message -----
From: "John Magne"<jmagne(a)redhat.com>
To: "pki-devel"<pki-devel(a)redhat.com>
Sent: Tuesday, April 8, 2014 2:30:05 PM
Subject: [pki-devel][PATCH] 0008-Further-progress-Format-operation.patch
Patch accomplishes the following:
1. Read applet into memory to prepare to write to token.
2. With tpsclient create secure channel by implementing Initialize Update and
ExternalAuthenticate messages.
3. Support for MAC and encryption for messages going on after secure channel has been
created.
4. Implemented method to remove an aid file or instance from the token.
5. Added some symkey methods to allow TPS to manipulate session keys.
Have not tried this with real token as of yet. The tpsclient does verify of the MAC
coming from the server and decrypts encrypted messages. Decrypted messages have to be
correct for the MAC verification to work.
Next step will be to add the phone home servlet to the TPS and give it a try with a real
token and esc.
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
12:49 p.m.
New subject: Fwd: [pki-devel][PATCH] 0008-Further-progress-Format-operation.patch
Cfu gave verbal ack after addressing comments, which has been done.
Verbal ack from edewata.
Pushed to master.
----- Original Message -----
From: "Christina Fu" <cfu(a)redhat.com>
To: pki-devel(a)redhat.com
Sent: Friday, April 11, 2014 11:22:57 AM
Subject: Re: [Pki-devel] Fwd:
[pki-devel][PATCH] 0008-Further-progress-Format-operation.patch
Please see my review comments below.
Christina
================
1. In APDU.java I'd prefer you kept the old name "secureMessage" instead
of
"encryptData" for the function that calls Util.encryptData() for two
reasons:
a. It would be easier for people (like myself) to match the tps tomcat code
with the tps c++ code
b. Now you have two "encryptData" functions in two separate files -- one in
util.java, and one in APDU.java. Could get confusing.
2. The specialURLEncode() function you added in Util.java looks almost
identical to SpecialEncode() in TKSKnownSessionKey.java.
How about filing a separate ticket for making TKSKnownSessionKey.java call
your version instead and remove the one in TKSKnownSessionKey.java?
3. for JNI function UnwrapSessionKeyWithSharedSecret, just wondering, in JSS,
there is a native function called nativeWrapSymWithSym, which is mapped from
pkcs11/PK11KeyWrapper.java:wrap(). have you looked into if that would have
been utilized? And if not, would your method be something to be made generic
and placed into jss? I am not sure if we already have a ticket for this, but
perhaps file one to put all the functions from symkey that fit to be generic
JSS functions to be moved into JSS instead.
4. for JNI function GetSymKeyByName().
a. I think it's customary to initialize slot to be PK11_GetInternalKeySlot().
So in the case of tokenName not supplied by the caller, the function will at
least try looking into internal token by default. And if you must require
tokenName, you should have checked it at the very beginning along with other
required param such as keyName.
b. you don't need to do if (tokenName) again after you already checked it
right before:
if(tokenName == NULL || sharedSecretKey == NULL || sessionKeyBA == NULL) {
goto loser;
}
c. Also, same as above. if you file a ticket from #3 (moving appropriate
things into JSS), this should be considered.
5. In ReturnSymKey(), you added some printf for debugging I presume. since
this is JNI C code, one should be careful on not making attempt to print
something that could turn out to be null. I suggest you add check to null
before trying to print.
Same comment applies to wherever else that's appropriate.
6. CreateDesKey24Bit is moved from RA::CreateDesKey24Byte()? I believe the
DES key is 24 "bytes" not "bits". You probably want to name it back
to
CreateDesKey24Byte?
7. In SecureChannel.java, I'm not sure I understand the value of the
constructor that does not take any parameters if you bomb them out if any of
the class variables are not set. If one calls new SecureChannel() without
any parameters how would those variables be set if it just bombs you out
right away?
8. For those APDU classes under common/src/org/dogtagpki/tps/apdu/, I'd
prefer you kept the APDU in the names of those APDU classes/files.. It would
have made it much easier for the readers.
e.g. DeleteFileAPDU
9. in format(), the tksConnId is just temporarily hard coded to tks1 right?
You might wan to add a TODO comment there to retrieve from profile in
config.
10. in getChannelBlockSize, would it possible to throw EBaseException when
you already have a default value? Same comment with other similar getXXX()
functions.
On 04/08/2014 02:33 PM, John Magne wrote:
Actually attach the patch this time...
----- Forwarded Message -----
From: "John Magne" <jmagne(a)redhat.com> To: "pki-devel"
<pki-devel(a)redhat.com>
Sent: Tuesday, April 8, 2014 2:30:05 PM
Subject: [pki-devel][PATCH] 0008-Further-progress-Format-operation.patch
Patch accomplishes the following:
1. Read applet into memory to prepare to write to token.
2. With tpsclient create secure channel by implementing Initialize Update and
ExternalAuthenticate messages.
3. Support for MAC and encryption for messages going on after secure channel
has been created.
4. Implemented method to remove an aid file or instance from the token.
5. Added some symkey methods to allow TPS to manipulate session keys.
Have not tried this with real token as of yet. The tpsclient does verify of
the MAC coming from the server and decrypts encrypted messages. Decrypted
messages have to be correct for the MAC verification to work.
Next step will be to add the phone home servlet to the TPS and give it a try
with a real token and esc.
_______________________________________________
Pki-devel mailing list Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
3668
days inactive
3674
days old
3 comments
2 participants
participants (2)
-
Christina Fu -
John Magne