[Pki-devel] TPS REST interface design

Ade Lee alee at redhat.com
Sat Jun 29 19:16:41 UTC 2013


I'll respond to the comments below in a separate email.

One thing I just noticed though is the operation to add a token.
You have POST /tokens passing in the tokenId.

POST /tokens is only used when creating a resource when the URL of the
resource is unknown -- ie. it will be specified by the server.  An
example of this is POST /certrequests which returns the URL of the
request /certrequests/0xf00 where 0xf00 is the request ID as assigned by
the server.

In this case, the token ID is what is being passed in.  So we know the
resultant  URL when the resource is created.  There are two options
instead:

a) PUT  /tokens/tokenID
b) POST /tokens/tokenID

My preference is a) but of course the advantage of (b) is that there is
a distinction between an ADD and a modify operation.

The same comment applies to all the ADD operations in the design.
 
On Fri, 2013-06-28 at 23:57 -0500, Endi Sukma Dewata wrote:
> On 6/28/2013 9:36 AM, Ade Lee wrote:
> >>> Or is your intention to send in something with an "action" parameter -
> >>> in which case, the correct operation is a POST?
> >>
> >> I use 'action' in the modify token operation because I couldn't find a
> >> better replacement for the 'question' parameter in the original
> >> op=do_token. Ideally it should be called 'status' but there's already
> >> another attribute with that name. Any suggestion? This 'action' should
> >> actually be returned by the GET operation too. I'll add it after we
> >> finalize the name.
> >
> > do_token?question=X is essentially an operation where you are attempting
> > to set the state of the token resource - for example, from "active" to
> > "lost".  There is no need for an additional parameter.
> 
> I added a Change Token State operation:
> http://pki.fedoraproject.org/wiki/TPS_REST_Interface#Change_Token_State
> 
> As discussed, for now we'll call this parameter 'next state', which will 
> update the status & reason depending on the current state of the token.
> 
> >>> 3.  Should we be worried about a XSS attack here for modifying the state
> >>> of the token?  My guess is that we need to take advantage of the nonce
> >>> mechanism, in which case, token state modification will require two
> >>> steps.
> >>
> >> Yes, but similar to our discussion in the past, nonces or ETags can be
> >> added in a later phase without changing the interface. Since the
> >> modification is a PUT operation, and the request attributes are obtained
> >> using a GET operation done earlier, this is a two-step operation.
> >> Without the nonce or ETag the GET operation will be optional, and the
> >> client can construct the request from scratch. But later the PUT
> >> operation can require a nonce or ETag from a previous GET operation.
> >
> > OK, please put the nonce in the design.  We dont want to forget it.
> 
> I added two sections related to this:
> http://pki.fedoraproject.org/wiki/TPS_REST_Interface#Concurrency_Control
> http://pki.fedoraproject.org/wiki/TPS_REST_Interface#Vulnerabilities
> 
> >>> 4. You should also note that we will be sending back BAD_REQUEST (401?)
> >>> when the token state transition is not permitted.
> >>
> >> That should fall under "Normal application errors will return HTTP 4xx
> >> return code." We can add more details as we implement it.
> >
> > Right, I'm just pointing this out because in this case, there is a state
> > machine which determines which operations are successful.  From the
> > point of view of the API, doing a token transition is just an attempt to
> > set the state of the token differently, but unlike something simple like
> > just changing an attribute value, the operation is subject to the
> > results of a state machine.
> 
> I noted that in the response of Change Token State:
> http://pki.fedoraproject.org/wiki/TPS_REST_Interface#Change_Token_State
> 
> >>> 5. In the response to PUT /tokens/X, it is not necessary to return the
> >>> state of the token.  Rather, you should return a URL pointer to the
> >>> newly modified resource.  The same comment applies to the other PUT
> >>> operations as well.
> >>
> >> Since PUT operation in general doesn't create a new resource, and also
> >> the request is sent to the resource URL being modified, there is no need
> >> to return a new location because it will be identical. In this design
> >> the new location will only be returned on POST operations when adding a
> >> new resource, which in this case the new resource location will be
> >> different from the POST target (the resource collection).
> >>
> > Agreed.  But in PUT /tokens/X, you return state information.  This is
> > not necessary.  You should probably return nothing - other than status.
> 
> See explanation in #10.
> 
> >>> 6. Same question about XSS for add/remove token/ add/remove user.
> >>
> >> Similarly, nonces or ETags can be added later. As described earlier,
> >> modifying users is a two-step process too with GET and PUT. Add and
> >> remove operations do not require the client to do any other operation,
> >> but it can be made so without changing the interface. The DELETE can
> >> require a GET, but I'm not sure what operation should be required before
> >> add.
> >>
> > Yeah - we need to think about ADD.  Being able to add a privileged user
> > for example by XSS would be troubling.
> 
> I think we're mixing up CSRF and XSS. Nonces will be useful to prevent 
> CSRF. XSS can be prevented by encoding/escaping the values.
> 
> If we implement a single-page Web UI (like IPA Web UI) the client can 
> obtain a nonce during login, then the UI will keep the nonce in memory 
> throughout the session and use it for all update requests including add 
> (the nonce should really be called CSRF token). CSRF attack usually will 
> open a new page, so it will not be able to know the nonce.
> 
> If we implement a multi-page Web UI (like the current Dogtag UI), the 
> add operation can be done in 2 steps. The first page will generate the 
> nonce, then the second page will execute the operation.
> 
> >>> 9. Is there a mapping between Profile and Profile Mapping operations and
> >>> the old operations?  Please put that in so we can see whether we have
> >>> complete coverage.  In particular, I do not see an operation to
> >>> approve/enable a profile.  This is important because we have Common
> >>> Criteria requirements that two users are involved in the approval of a
> >>> profile.  In our case, IIRC we implemented it such that an admin can
> >>> configure a profile but an agent must approve it.
> >>
> >> There should be, but they are not in the docs and I have not had a
> >> chance to test all possible operations in the UI. Once I get the TPS
> >> instance back running I'll continue this. But in general we can
> >> implement something similar to the cert requests:
> >>
> >>     /tps/rest/profiles/{id}/approve (all approvers will use the same URL)
> >>     /tps/rest/profiles/{id}/enable
> >>
> > Thats fine - lets make sure we get those operations in.
> 
> I added the missing mappings. I also added a Change Profile State operation:
> http://pki.fedoraproject.org/wiki/TPS_REST_Interface#Change_Profile_State
> 
> >>> 10.  In POST /tps/rest/profilemappings, you return the location of the
> >>> profile mappings as well as the contents of the profile mapping
> >>> resource.  You should just return the location.  In fact, its probably
> >>> better to just return locations in general.  The client would then use
> >>> GET to fetch the details if needed.  This comment applies to many of the
> >>> resources.
> >>
> >> In general I'd rather return the resource attributes in the POST
> >> response than requiring the client to do a separate GET later. This
> >> response acts as a receipt/acknowledgement for the add request, and it
> >> will return the actual attributes stored on the server before any other
> >> operation is done on it. This is rather important because sometimes a
> >> lazy UI won't do an extra GET because it assumes the attributes don't
> >> change, where in fact they could be normalized, ignored, or generated by
> >> the server. It also prevents a possible (although unlikely) attack that
> >> inserts an operation between the POST and GET.
> >
> > I guess you are suggesting the same for PUT requests as above.
> > I'm not sure whether this is vaid reasoning or not.  Just because you
> > see what has been posted (or PUT) in the server does not mean that this
> > data has not been changed the next time the data is accessed.
> 
> There's never any guarantee that the data we get (either from PUT, POST, 
> or even GET) has not been changed when we submit it for another update 
> operation. The advantage of returning the data in PUT/POST response is 
> we're pushing the updated data to the client's feet.
> 
> > If anything, returning this data encourages lazy clients.
> 
> On the contrary, by returning the data in the PUT/POST response the 
> client will have little reason not to use it. If the client has no 
> intention to get the new data anyway (either from PUT/POST response or 
> by doing another GET) there's nothing we can do about it. But if the 
> client wants to get the new data, using PUT/POST response is a much 
> cheaper option.
> 
> Compare the following code:
> 
>      user = users.update(user);
> 
> with this:
> 
>      users.update(user);
>      user = users.get(user.getId());
> 
> > The fact is
> > though that we will have to implement nonces and possibly etags, and so
> > GETs will become mandatory for any changes.
> 
> Not necessarily. ETag is only an optimistic lock; it's not a guarantee 
> that the data won't change. The PUT/POST response will also provide 
> ETag. Also see comment #6, we may be able to reuse the nonce.
> 
> If there's nobody else updating the same data, we could reuse the 
> PUT/POST response and the ETag that came with it to make subsequent 
> update. So we're still using a two-step update process, but the 2nd step 
> of the first update may become the 1st step of the second update. See 
> also the Concurrency Control section in the wiki page.
> 
> Imagine this, you updated a user, then you went to do some other 
> operations. Then you came back to this user and edited it again. If 
> there's nobody else changing the user, then the new data and ETag that 
> you got from the first update would still be valid. But if first update 
> happened a long time ago, the client may decide according to the caching 
> policy that the data has expired and then issue a GET to get a newer 
> data and ETag.
> 
> >>> 11. In the PUT /tps/rest/config, we have requirements for having than
> >>> one user to approve changes.  Admins make changes and agents approve
> >>> them if I recall correctly.  You should look at the differences between
> >>> the agent and admin pages.
> >>
> >> I'll take a look again after I have the TPS instance back. Is there any
> >> documents or diagrams showing the workflow of all approval processes in
> >> TPS UI? Thanks.
> >>
> > The CC docs are probably the best bet for that (other than looking at
> > the code).
> 
> The only approval process I saw in the UI is for profiles, not for 
> general config. Could you point me to a specific page in the docs?
> 
> Thanks.
> 





More information about the Pki-devel mailing list