Hi Eric,

thank you for the review.  Please see the detailed responses inline.

The diff of all changes that should address your comments can be found here:
https://author-tools.ietf.org/api/iddiff?doc_1=draft-ietf-intarea-proxy-config&url_2=https://tfpauly.github.io/privacy-proxy/draft-ietf-intarea-proxy-config.txt


Best,
Dragana


# Éric Vyncke, INT AD, AD review for draft-ietf-intarea-proxy-config-10
CC @evyncke

Thank you for the work put into this document. Please find below my AD review.

As the responsible AD, I expect all the points below to be addressed, either by 
a revised I-D, or an email reply. Of course, authors and WG can reject my 
points, but this needs to be justified. Once all the points are addressed, I 
will proceed with the publication process, i.e., IETF Last Call.

Special thanks to Marcus Ihlar for the shepherd's detailed write-up including 
the WG consensus *and* the justification of the intended status.

I hope that this review helps to improve the document,

Regards,

-éric

Note: this AD reviews follows the Markdown syntax of 
https://github.com/mnot/ietf-comments/tree/main, i.e., they can be processed by 
a tool to create github issues.

## Critical issues

### Section 1

I find it very weird to read such a statement in a proposed standard draft: 
`this document partly describes`...

### Section 1.1

I was unaware of `This solution squats on DHCP option 252`... but please 
rewrite the sentence by reference DHCPv4 (as opposed to DHCPv6 -- my guess) and 
rather write "uses the private use option 252".




The previous 2 comments are addressed in PR 
https://github.com/tfpauly/privacy-proxy/pull/328

### Section 2

Suggest clarifying that the request is for the PvD in `will not make a request 
to port 8080`.

`Proxy deployments that need separate PvD configuration properties SHOULD use 
different hosts` why not a MUST ? If SHOULD is kept, when can this be bypassed 
or what are the consequence of not following the SHOULD ? See also:
https://datatracker.ietf.org/doc/statement-iesg-statement-on-clarifying-the-use-of-bcp-14-key-words/

Same comment about `The "prefixes" array SHOULD be empty by default.`

The previous 3 comments are addressi in PR:
https://github.com/tfpauly/privacy-proxy/pull/329


### Section 3

s/array of dictionaries/list of JSON dictionaries/ ? But this is outside of my 
expertise... but I do not think that JSON has the concept of "array" but has 
"list".

In the last updated we converged on using 'array' instead of list. 'Arrays' are 
defined in JSON, please take a look at Section 5 of RFC8259.




`single FQDN hostname` all the text before uses simply "hostname", is there any 
reason why in this sentence it is qualified by FQDN ?


### Section 3.1

See IESG statement above about "SHOULD" and update the text `SHOULD define the 
protocol value`.

### Section 3.2

I find this section under-specified... the example has two "_" which one 
separate the vendor from the name ? In the example, it seems obvious but is it 
normative to use the last "_" ? Is there a registry for "vendor" ?

The last 3 comments are addressed in PR: 
https://github.com/tfpauly/privacy-proxy/pull/330

The PR updates the text about the proprietary keys and the use of "_". The text 
is not normative and there is no registry for "vendors".





### Section 4.1

`an entry "*.example.com" in the domains array of a proxy-match rule would 
match the FQDN "example.com"` so there is no way to have different proxies for 
foo.example.com<http://foo.example.com/> and example.com<http://example.com/> ?


This is possible by defining something like this:

{
  "identifier": "proxy.example.org.",
  "expires": "2026-06-23T06:00:00Z",
  "prefixes": [],
  "proxies": [
    {
      "protocol": "http-connect",
      "proxy": "proxy1.example.org:80",
      "identifier": "proxy1"
    },
    {
      "protocol": "http-connect",
      "proxy": "proxy2.example.org:80",
      "identifier": "proxy2"
    }
  ],
  "proxy-match": [
    {
      "domains": [ "example.org" ],
      "proxies": [ "proxy1" ]
    },
    {
      "domains": [ "*.example.org" ],
      "proxies": [ "proxy2" ]
    }
  ]
}

We added this example to the document. Please take a look at PR: 
https://github.com/tfpauly/privacy-proxy/pull/331


### Section 4.2

Another "orphaned" SHOULD in `the client SHOULD treat the array as an ordered 
list`

### Section 7.1

Please add a reference URI 
https://www.iana.org/assignments/pvds/pvds.xhtml#additional-information-pvd-keys
 to the registry name to avoid any ambiguity.

### Section 7.5

Same with 
https://www.iana.org/assignments/dns-svcb/dns-svcb.xhtml#dns-svcparamkeys

### Sectin 8.1

I wonder whether the reference to CONNECT-* should rather be informative. Same 
for SOCKS and URITEMPLATE.

Previous 4 comments area ddressed in PR 
https://github.com/tfpauly/privacy-proxy/pull/330


## Non-critical / cosmetic issues

Note: these points must also be addressed.

### Section 1

Should plural form be used in `Client can also benefit` ?

### typos

s/assocated/assoc*i*ated/

s/negotation/negot*i*ation/

s/as optional;or proprietary/as optional*,* or proprietary/ ?

s/comparis*i*ons/comparisons/

s/2001:DB8/2001:db8/

s/For example/For example*,*/

s/if destination rules include*s*/if destination rules include/

s/more then one/more th*a*n one/

s/with a separate identifier*s*/with a separate identifier/ ?

### Dates

Dates in examples are back in 2023, any chance to update them ?


These "non-critical" comments are addressed in PR 
https://github.com/tfpauly/privacy-proxy/pull/327




  *   [Int-area] AD review of 
draft-ietf-intarea-proxy-…<https://mailarchive.ietf.org/arch/msg/int-area/SGG4fFuOTN_yMd03q02JnHmxTE4/>
  Eric Vyncke (evyncke)

  *


_______________________________________________
Int-area mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to