Remove AUTOMATIC_USE #480
Remove AUTOMATIC_USE #480
Conversation
MikeBishop
force-pushed
the
not_automatic
branch
from
a789d57
to
8dbce8e
Compare
| A server SHOULD provide certificates for an origin before pushing resources from | ||
| it or supplying content referencing the origin. If a client receives a | ||
| `PUSH_PROMISE` referencing an origin for which it has not yet received the | ||
| server's certificate, the client SHOULD verify the server's possession of an |
There was a problem hiding this comment.
You use a lot of "SHOULD" here, and I think that they are for the wrong reason.
In this case, you are using SHOULD to avoid having to say why the client might want to verify that the server has a valid certificate. Here, you can say that the verification is mandatory. In many other cases, you can describe what goal the endpoint might have and how it can achieve that goal.
| stream to inform the server that progress is blocked until the request is | ||
| satisfied. | ||
|
|
||
| If the certificate is already available, the server SHOULD immediately |
There was a problem hiding this comment.
lazy SHOULD: If a client requests a certificate that is already available, the server can send a USE_CERTIFICATE frame to signal which certificate it assumes to be effective.
| certificate has been received and validated. | ||
| On subsequent requests, the client can proactively indicate that an existing | ||
| certificate MAY be used by including a `USE_CERTIFICATE` frame as part of the | ||
| request. If the indicated certificate does not correspond to the request the |
There was a problem hiding this comment.
You need to explain what ordering you expect to have here. After the header block, but before the request body. Which implies an empty DATA frame rather than a HEADERS frame with FIN attached.
| @@ -470,6 +486,10 @@ authentication request identifier, `Request-ID`. A peer that receives a | |||
| error of type `PROTOCOL_ERROR`. Frames with identical request | |||
| identifiers refer to the same `CERTIFICATE_REQUEST`. | |||
|
|
|||
| The `CERTIFICATE_NEEDED` frame MUST NOT be sent by servers prior to seeing the | |||
| end of the client's request, and then MUST NOT be sent if the client included a | |||
There was a problem hiding this comment.
We need to be very clear about what it means to say that the request is ended, because the client won't be able to send USE_CERTIFICATE on a half-closed stream. (Do we have an issue for that?)
There was a problem hiding this comment.
Aside from the point raised by @martinthomson, I think we should allow a server to send a CERTIFICATE_NEEDED frame before seeing the end of the request.
As stated in MikeBishop/http2-certs#17 (comment), CGI (and other APIs that have been designed based on CGI) require the properties of a certificate (e.g. DN) to be sent before the request body.
Unless we allow the CERTIFICATE_NEEDED to be sent while the request body is being transmitted, a server will be required to buffer entire request body when it needs to enforce the use of a client certificate.
There was a problem hiding this comment.
That's a good point. The reason for that text is to ensure there's not a race, so the server doesn't send in ignorance of whether the client already included one. (And therefore, doesn't get either side confused about whether the USE_CERTIFICATE is in response to the server's CERTIFICATE_REQUIRED.)
Implements Kazuho's suggestion to remove AUTOMATIC_USE and replace it with the ability to send USE_CERTIFICATE as part of the request.
(Note that this runs headlong into the frames-on-closed-streams problem and might need to be delayed until we fix that.)
This is a straight port of MikeBishop/http2-certs#17; I'll address the feedback from that PR shortly.