-
Notifications
You must be signed in to change notification settings - Fork 10
Fix EOM description in certificate validator helpers docs #47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
09b9ff1
064b8e0
92bef84
17d0cce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1119,32 +1119,34 @@ Result line sent back to Squid: | |||||||||||||||||||
|
|
||||||||||||||||||||
| This interface is similar to the SSL certificate generation interface. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Input *line* received from Squid: | ||||||||||||||||||||
| Request messages sent by Squid have the following syntax: | ||||||||||||||||||||
|
|
||||||||||||||||||||
| request size [kv-pairs] | ||||||||||||||||||||
| [request-ID SP] action SP size SP body LF | ||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Multiple issues:
Please match the definition from PR #30:
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Why do you think so?
I disagree that we should use what you call "fieldset semantic definition" for all helpers. I disagree that we should not list delimiters when describing certificate helper messages syntax, especially when other helpers are using the same delimiters differently. We should address certificate helper needs using the best tools matching those needs instead. IMO, this PR is a step forward in that direction. |
||||||||||||||||||||
|
|
||||||||||||||||||||
|  | ||||||||||||||||||||
| *line* refers to a logical input. **body** may contain \\n characters so | ||||||||||||||||||||
| each line in this format is delimited by a 0x01 byte instead of the | ||||||||||||||||||||
| standard \\n byte. | ||||||||||||||||||||
| ... where `SP` is an ASCII space character, `LF` is an ASCII new line | ||||||||||||||||||||
| character (\\n), and the other fields are documented below. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - request | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - The type of action being requested. Presently the code | ||||||||||||||||||||
| **cert\_validate** is the only request made. | ||||||||||||||||||||
| :warning: The `body` field usually contains new line characters. Use the | ||||||||||||||||||||
| `size` field to find the end of the body. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - size | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - Total size of the following request bytes taken by the | ||||||||||||||||||||
| **key=pair** parameters. | ||||||||||||||||||||
| - request-ID: An unsigned positive 64-bit decimal number uniquely identifying | ||||||||||||||||||||
| this request among all requests submitted to this helper process (i.e. on | ||||||||||||||||||||
| this helper stdin connection). Squid sends this field when (and only when) | ||||||||||||||||||||
| the `concurrency=N` helper configuration option is used with `N` values | ||||||||||||||||||||
| grater than 1. When this field is present, the corresponding helper response | ||||||||||||||||||||
| must start with a matching `request-ID` field. | ||||||||||||||||||||
|
Comment on lines
+1134
to
+1137
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIRC these certificate helpers were where we chose to make concurrency the default-on and a mandatory field. Which would mean all this text about special values is wrong/irrelevant, at least for this specific helper API. Please use the text already agreed in PR #30:
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Where? |
||||||||||||||||||||
|
|
||||||||||||||||||||
| - kv-pairs | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - An optional list of key=value parameters separated by new lines. | ||||||||||||||||||||
| Supported parameters are: | ||||||||||||||||||||
|
|
||||||||||||||||||||
| | | | | ||||||||||||||||||||
| | --------------------- | ------------------------------------------------------------------------------------------------------------------------------- | | ||||||||||||||||||||
| - action: A string with the name of the helper operation being requested. | ||||||||||||||||||||
| Squid currently only sends `cert_validate` requests. | ||||||||||||||||||||
|
Comment on lines
+1139
to
+1140
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you are going to change the field semantic from "request" to "action". Then please do so consistently.
Suggested change
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| - size: A decimal number representing the size of the `body` field in bytes. | ||||||||||||||||||||
| The size of an empty `body` is zero. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - body: A possibly empty list of the following key=value pairs separated by | ||||||||||||||||||||
| ASCII new line characters (\\n). | ||||||||||||||||||||
|
|
||||||||||||||||||||
| | key | value description | | ||||||||||||||||||||
| | --------------------- | --------------------------- | | ||||||||||||||||||||
| | host | FQDN host name or the domain | | ||||||||||||||||||||
| | proto\_version | The SSL/TLS version | | ||||||||||||||||||||
| | cipher | The SSL/TLS cipher being used | | ||||||||||||||||||||
|
|
@@ -1155,7 +1157,7 @@ standard \\n byte. | |||||||||||||||||||
|
|
||||||||||||||||||||
| Example request: | ||||||||||||||||||||
|
|
||||||||||||||||||||
| 0 cert_validate 1519 host=dmz.example-domain.com | ||||||||||||||||||||
| 17179869184 cert_validate 1519 host=dmz.example-domain.com | ||||||||||||||||||||
| cert_0=-----BEGIN CERTIFICATE----- | ||||||||||||||||||||
| MIID+DCCA2GgAwIBAgIJAIDcHRUxB2O4MA0GCSqGSIb3DQEBBAUAMIGvMQswCQYD | ||||||||||||||||||||
| ... | ||||||||||||||||||||
|
|
@@ -1164,49 +1166,56 @@ Example request: | |||||||||||||||||||
| error_name_0=X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT | ||||||||||||||||||||
| error_cert_0=cert0 | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Result line sent back to Squid: | ||||||||||||||||||||
|
|
||||||||||||||||||||
| result size kv-pairs | ||||||||||||||||||||
| Squid expects response messages with the following syntax: | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - result | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - One of the result codes: | ||||||||||||||||||||
| [request-ID SP] result SP size SP body SOH | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ... where `SP` is an ASCII space character, `SOH` is an ASCII "Start of | ||||||||||||||||||||
| Heading" character (with the value of 1), and the other fields are documented | ||||||||||||||||||||
| below. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| :warning: If the response `body` field is empty or contains a single key=value | ||||||||||||||||||||
| pair that has no embedded new lines, then the entire response message should | ||||||||||||||||||||
| have no new line characters. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - request-ID: An unsigned positive 64-bit decimal number equal to the | ||||||||||||||||||||
| `request-ID` field of the corresponding helper request. These fields must be | ||||||||||||||||||||
| sent when (and only when) the `concurrency=N` helper configuration option is | ||||||||||||||||||||
| used with `N` values grater than 1. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - result: A string matching one of the following codes: | ||||||||||||||||||||
|
|
||||||||||||||||||||
| | | | | ||||||||||||||||||||
| | code| code meaning | | ||||||||||||||||||||
| | --- | ------------------------------------------ | | ||||||||||||||||||||
| | OK | Success. Certificate validated. | | ||||||||||||||||||||
| | ERR | Success. Certificate not validated. | | ||||||||||||||||||||
| | BH | Failure. The helper encountered a problem. | | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - size: A decimal number representing the size of the `body` field in bytes. | ||||||||||||||||||||
| The size of an empty `body` is zero. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - size | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - Total size of the following response bytes taken by the | ||||||||||||||||||||
| **key=pair** parameters. | ||||||||||||||||||||
| - body: A possibly empty list of the following key=value pairs separated by | ||||||||||||||||||||
| ASCII new line characters (\\n). | ||||||||||||||||||||
|
Comment on lines
+1198
to
+1199
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please do not add the new term "body". It is far too easily conflated with the entire message being a "body". This area of the message is an optional sequence of |
||||||||||||||||||||
|
|
||||||||||||||||||||
| - kv-pairs | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - A list of key=value parameters separated by new lines. The | ||||||||||||||||||||
| supported parameters are: | ||||||||||||||||||||
|
|
||||||||||||||||||||
| | | | | ||||||||||||||||||||
| | ----------------------- | ------------------------------------------------------------------------------------------------------------------------- | | ||||||||||||||||||||
| | key | value description | | ||||||||||||||||||||
| | ----------------------- | --------------------------- | | ||||||||||||||||||||
| | cert\_***ID*** | A certificate send from helper to squid. The **ID** is an index number for this certificate | | ||||||||||||||||||||
| | error\_name\_***ID*** | The openSSL error name for the error **ID** | | ||||||||||||||||||||
| | error\_reason\_***ID*** | A reason for the error **ID** | | ||||||||||||||||||||
| | error\_cert\_***ID*** | The broken certificate. It can be one of the certificates sent by helper to squid or one of those sent by squid to helper | | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| Example response message: | ||||||||||||||||||||
| Example response message (with the terminating SOH character shown as `^A`): | ||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Abbreviations for ASCII character are standardized. SOH and EOL are specific characters with specific well-known semantic meanings.
The correct "term of art" to be using here is
Suggested change
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| ERR 1444 cert_10=-----BEGIN CERTIFICATE----- | ||||||||||||||||||||
| 17179869184 ERR 1444 cert_10=-----BEGIN CERTIFICATE----- | ||||||||||||||||||||
| MIIDojCCAoqgAwIBAgIQE4Y1TR0/BvLB+WUF1ZAcYjANBgkqhkiG9w0BAQUFADBr | ||||||||||||||||||||
| ... | ||||||||||||||||||||
| 398znM/jra6O1I7mT1GvFpLgXPYHDw== | ||||||||||||||||||||
| -----END CERTIFICATE----- | ||||||||||||||||||||
| error_name_0=X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT | ||||||||||||||||||||
| error_reason_0=Checked by Cert Validator | ||||||||||||||||||||
| error_cert_0=cert_10 | ||||||||||||||||||||
| error_cert_0=cert_10^A | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ### Cache file eraser | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -56,82 +56,9 @@ triggering the existing SSL error processing code. | |||||||||||||||
| Helper responses will be cached to reduce validation performance burden | ||||||||||||||||
| (indexed by validation query parameters). | ||||||||||||||||
|
|
||||||||||||||||
| ## Helper communication protocol | ||||||||||||||||
|
|
||||||||||||||||
| This interface is similar to the SSL certificate generation interface. | ||||||||||||||||
|
|
||||||||||||||||
| Input *line* received from Squid: | ||||||||||||||||
|
|
||||||||||||||||
| request size [kv-pairs] | ||||||||||||||||
|
|
||||||||||||||||
| > :warning: | ||||||||||||||||
| *line* refers to a logical input. **body** may contain \\n characters so | ||||||||||||||||
| each line in this format is delimited by a 0x01 byte instead of the | ||||||||||||||||
| standard \\n byte. | ||||||||||||||||
|
|
||||||||||||||||
| - request | ||||||||||||||||
| : The type of action being requested. Presently the code | ||||||||||||||||
| **cert_validate** is the only request made. | ||||||||||||||||
| - size | ||||||||||||||||
| : Total size of the following request bytes taken by the | ||||||||||||||||
| **key=pair** parameters. | ||||||||||||||||
| - kv-pairs | ||||||||||||||||
| : An optional list of key=value parameters separated by new lines. | ||||||||||||||||
| Supported parameters are: | ||||||||||||||||
| | --- | --- | | ||||||||||||||||
| | host | FQDN host name or the domain | | ||||||||||||||||
| | proto_version | The SSL/TLS version | | ||||||||||||||||
| | cipher | The SSL/TLS cipher being used | | ||||||||||||||||
| | cert_***ID*** | Server certificate. The ID is an index number for this certificate. This parameter exist as many as the server certificates are | | ||||||||||||||||
| | error_name_***ID*** | The openSSL certificate validation error. The ID is an index number for this error | | ||||||||||||||||
| | error_cert_***ID*** | The ID of the certificate which caused error_name_ID | | ||||||||||||||||
|
|
||||||||||||||||
| Example request: | ||||||||||||||||
|
|
||||||||||||||||
| 0 cert_validate 1519 host=dmz.example-domain.com | ||||||||||||||||
| cert_0=-----BEGIN CERTIFICATE----- | ||||||||||||||||
| MIID+DCCA2GgAwIBAgIJAIDcHRUxB2O4MA0GCSqGSIb3DQEBBAUAMIGvMQswCQYD | ||||||||||||||||
| ... | ||||||||||||||||
| YpVJGt5CJuNfCcB/ | ||||||||||||||||
| -----END CERTIFICATE----- | ||||||||||||||||
| error_name_0=X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT | ||||||||||||||||
| error_cert_0=cert0 | ||||||||||||||||
|
|
||||||||||||||||
| Result line sent back to Squid: | ||||||||||||||||
|
|
||||||||||||||||
| result size kv-pairs | ||||||||||||||||
|
|
||||||||||||||||
| - result | ||||||||||||||||
| : One of the result codes: | ||||||||||||||||
|
|
||||||||||||||||
| | --- | ------------------------------------------ | | ||||||||||||||||
| | OK | Success. Certificate validated. | | ||||||||||||||||
| | ERR | Success. Certificate not validated. | | ||||||||||||||||
| | BH | Failure. The helper encountered a problem. | | ||||||||||||||||
|
|
||||||||||||||||
| - size | ||||||||||||||||
| : Total size of the following response bytes taken by the | ||||||||||||||||
| **key=pair** parameters. | ||||||||||||||||
| - kv-pairs | ||||||||||||||||
| : A list of key=value parameters separated by new lines. The | ||||||||||||||||
| supported parameters are: | ||||||||||||||||
|
|
||||||||||||||||
| | --- | --- | | ||||||||||||||||
| | cert_***ID*** | A certificate send from helper to squid. The **ID** is an index number for this certificate | | ||||||||||||||||
| | error_name_***ID*** | The openSSL error name for the error **ID** | | ||||||||||||||||
| | error_reason_***ID*** | A reason for the error **ID** | | ||||||||||||||||
| | error_cert_***ID*** | The broken certificate. It can be one of the certificates sent by helper to squid or one of those sent by squid to helper | | ||||||||||||||||
|
|
||||||||||||||||
| Example response message: | ||||||||||||||||
|
|
||||||||||||||||
| ERR 1444 cert_10=-----BEGIN CERTIFICATE----- | ||||||||||||||||
| MIIDojCCAoqgAwIBAgIQE4Y1TR0/BvLB+WUF1ZAcYjANBgkqhkiG9w0BAQUFADBr | ||||||||||||||||
| ... | ||||||||||||||||
| 398znM/jra6O1I7mT1GvFpLgXPYHDw== | ||||||||||||||||
| -----END CERTIFICATE----- | ||||||||||||||||
| error_name_0=X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT | ||||||||||||||||
| error_reason_0=Checked by Cert Validator | ||||||||||||||||
| error_cert_0=cert_10 | ||||||||||||||||
| Helper communication protocol will be similar to the one used by the x509 | ||||||||||||||||
| certificate generator helper, as documented | ||||||||||||||||
| [elsewhere](Features/AddonHelpers). | ||||||||||||||||
|
Comment on lines
+59
to
+61
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better just to link to the actual definition(s).
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
| ## Design decision points | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Input line received from Squid"is text shared by all helper API documentation. Please revert.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverting this change would make things worse overall. I will not do it. We should describe certificate helper messages without using line-based terminology that is not applicable to (and dangerously misleading for) these helpers. If we want to use the same terms for all helpers, then line-based helpers should be updated as well (and I can volunteer to do that), but that update is out of this PR scope.