Re: [Pki-devel] [PATCH] 159, 160 - allow for automated generation of shared secrets for TKS/TPS connectors
On 9/26/2013 11:12 AM, Ade Lee wrote:
Patch 159:
Add service to generate and retrieve a shared secret
A new REST service has been added to the TKS to manage shared secrets.
The shared secret is tied to the TKS-TPS connector, and is created at the
end of the TPS configuration. At this point, the TPS contacts the TKS and
requests that the shared secret be generated. The secret is returned to the
TPS, wrapped using the subsystem certificate of the TPS.
The TPS should then decrypt the shared secret and store it in its certificate
database. This operations requires JSS changes, though, and so will be deferred
to a later patch. For now, though, if the TPS and TKS share the same certdb, then
it is sufficient to generate the shared secret.
Clients and CLI are also provided. The CLI in particular is used to remove the
TPSConnector entries and the shared secret when the TPS is pkidestroyed.
In addition to the things that were discussed over IRC earlier I have
some comments:
1. The createConnector() probably should return HTTP 201 since it's
creating a new entry. This would be consistent with other resources.
2. The capitalization of "connector" in the help message is not consistent:
tks-tpsconnector TPS Connector management commands
tks-tpsconnector-add Add TPS Connector to TKS
tks-tpsconnector-find Find TPS connector details on TKS
tks-tpsconnector-del Remove TPS connector from TKS
3. The tks-tpsconnector-find output should have a header and footer like
other *-find commands:
----------------------
1 connector(s) matched
----------------------
Connector ID: 0
Host: server.example.com
Port: 8443
User ID: TPS-server.example.com-8443
Nickname: TPS-server.example.com-8443 sharedSecret
----------------------------
Number of entries returned 1
----------------------------
4. Currently the access to the connector is limited to the subsystem
user, which doesn't correspond to a real person. It should allow the TKS
administrator to add/delete the connectors. This can be done by checking
the principal.getRoles() in validateUser().
5. The tks-tpsconnector-add/del parameters right now are passed as
arguments. It probably should be passed using options because it may
change in the future:
tks-tpsconnector-add --host <host> --port <port>
tks-tpsconnector-del --connector-id <connector ID>
6. We discussed about changing the error code for non-existent entries
to 204 (https://fedorahosted.org/pki/ticket/746). However, now I think
404 actually makes more sense. See
http://stackoverflow.com/questions/11746894/what-is-the-proper-rest-respo....
Both of the following URL's should return the same error code because
the client shouldn't need to know which path is mapped to REST method:
* /rest/users/wronguser
* /rest/wrongpath
So methods like deleteConnector(), deleteSharedSecret(), etc. should
throw ResourceNotFoundException instead of returning silently if the
entry doesn't exist.
I'll continue the review tomorrow.
--
Endi S. Dewata