1:31 p.m.
The CRMFPopClient has been refactored such that it is easier
to understand and reuse. The code has been fixed such that it
can read a normal PEM transport certificate. It also has been
fixed to parse the request submission result properly.
The client-cert-request CLI command was modified to support CRMF
requests.
The MainCLI and ClientConfig were modified to accept a security
token name.
The pki_java_command_wrapper.in was modified to include the Apache
Commons IO library.
https://fedorahosted.org/pki/ticket/1074
--
Endi S. Dewata
8:45 p.m.
Looks nice. Thinks are much simpler.
Quick Comments/Questions:
1. Looks like we've added a command line param for the transport cert.
I notice there is still a check to see if the number of params is 4 or more.
Is this still valid, or is the transport cert param optional?
2. Down in the code where we are generating keys either ec or rsa, we still
check to see if the algorithm is valid. Was this check not already done at the top
of the method?
3. Inside "submitRequest" you have some System.out.println's lingering. Was
the idea to
get rid of all of those?
4. Does the usage string give an idea what params are optional and such? If not would this
be
helpful? Is it the "default" designation used for this purpose?
Anyway, ACK on the functionality if its been of course tested to work.
----- Original Message -----
From: "Endi Sukma Dewata" <edewata(a)redhat.com>
To: "pki-devel" <pki-devel(a)redhat.com>
Sent: Tuesday, January 27, 2015 11:31:45 AM
Subject: [Pki-devel] [PATCH] 547 Refactored CRMFPopClient.
The CRMFPopClient has been refactored such that it is easier
to understand and reuse. The code has been fixed such that it
can read a normal PEM transport certificate. It also has been
fixed to parse the request submission result properly.
The client-cert-request CLI command was modified to support CRMF
requests.
The MainCLI and ClientConfig were modified to accept a security
token name.
The pki_java_command_wrapper.in was modified to include the Apache
Commons IO library.
https://fedorahosted.org/pki/ticket/1074
--
Endi S. Dewata
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
12:21 p.m.
Thanks for the review.
On 1/27/2015 8:45 PM, John Magne wrote:
Looks nice. Thinks are much simpler.
Quick Comments/Questions:
1. Looks like we've added a command line param for the transport cert.
I notice there is still a check to see if the number of params is 4 or more.
Is this still valid, or is the transport cert param optional?
I think it's no longer valid since the parameters are now specified
using options instead of arguments. I'll remove it before push. Ideally
the parameters should be handled using a getopt library.
2. Down in the code where we are generating keys either ec or rsa, we
still
check to see if the algorithm is valid. Was this check not already done at the top
of the method?
Yes, in the CRMFPopClient's main program it does validate the algorithm
already. However, since the CRMFPopClient methods now can be called by
other code directly, and potentially be given an invalid algorithm, we
should check the validity there too.
3. Inside "submitRequest" you have some
System.out.println's lingering. Was the idea to
get rid of all of those?
Some of the println()'s are displaying the server response (HTML page)
and some others are displaying the request ID and status. I'll change it
such that the server response is only displayed in verbose mode.
4. Does the usage string give an idea what params are optional and
such? If not would this be
helpful? Is it the "default" designation used for this purpose?
Currently it lists the ECC params under "Optionally...", but actually
there are other params that should be optional too (e.g. the hostPort
param). I think we should revise the usage string and also implement
getopt. It can be done as a separate patch later.
The 'default' indicates the value of an optional parameter if it's not
specified by the user. An optional parameter does not always need to
have a 'default' value, and that means the value will be empty/null if
not specified.
Anyway, ACK on the functionality if its been of course tested to
work.
Yes, I was able to submit CRMF request using CRMFPopClient and pki
client-cert-request (see
http://pki.fedoraproject.org/wiki/User_Certificate#Dogtag_10.2.2), and
when it's approved it archives the key in KRA.
--
Endi S. Dewata
1:48 p.m.
On 1/28/2015 12:21 PM, Endi Sukma Dewata wrote:
> Anyway, ACK on the functionality if its been of course tested to
work.
Yes, I was able to submit CRMF request using CRMFPopClient and pki
client-cert-request (see
http://pki.fedoraproject.org/wiki/User_Certificate#Dogtag_10.2.2), and
when it's approved it archives the key in KRA.
Pushed the revised patch to master.
--
Endi S. Dewata
3376
days inactive
3377
days old
3 comments
2 participants
participants (2)
-
Endi Sukma Dewata -
John Magne