Add support for standalone Service Params parsing, encoding and stringifying

[neilcook]

As discussed in #1489. The primary aim is to to provide an API for developers who just want to parse, encode and decode standalone service params, without reference to SVCB records.

• Adds a new parser for Service Params in scan.go, which returns a new type SvcParams, which implements encoding and stringify methods for all service params. This is probably most useful for server-side developers who would have service params in a config file.
• SVCBKeyValues type which implements SvcParams has additional Unpack method allowing construction and manipulation from wire-encoded params. This is useful for client-side developers who would read service params from the wire and then manipulate them.
• I've added basic tests in scan_test.go, as this seemed the most appropriate place. Also there is a test in svcb_test.go for the Unpack method.

[neilcook]

Any thoughts on this?

[miekg]

I think this looks ok. I do have some concerns on the API that we're adding here (and a whole bunch of code nits).
Also exporting pack/unpack feels strange for just this RR, and I think you can already do this (in a somewhat convoluted way)

[tmthrgd]

Yeah I'm not a huge fan of the API. It seems far too complex to parse a single record type.

[neilcook]

I think this looks ok. I do have some concerns on the API that we're adding here (and a whole bunch of code nits). Also exporting pack/unpack feels strange for just this RR, and I think you can already do this (in a somewhat convoluted way)

So, the rationale for the Pack/Unpack was explained in my original issue. This is unlike other RRs, in that part of the RR is now used standalone in other RFCs (specifically soon to be RFC 9463), and so requires Pack/Unpack routines. I couldn't find a way to do that with the existing API - even in a convoluted way, and certainly not without parsing a full SVCB RR, which is what this is specifically trying to avoid.

Please let me know on your API concerns. I could drop the SvcParams interface.

[neilcook]

Yeah I'm not a huge fan of the API. It seems far too complex to parse a single record type.

Any specific concerns? I'm not sure what you mean by too complex? I just added a parser so that folks can parse SvcParams in a standalone way (note that this is specifically not parsing an RR), and added a few methods to the SVCBKeyValues type to enable unpack and pack functionality just for the service params, which isn't currently available.

The SvcParams interface I added to simplify the API for folks using the parser, who just need to encode params, but it might just be confusing.

[miekg]

Hmm, let's unpack.

To access wiredata, you could make an SVCB record in a Msg, and just pack that and index at the correct byte. If you use a DNS question where the domain name is the root you know that packs to 00 . if the domain of the SVCB is also "." you can index with: header (12)+ question (1 + 2 +2) + RR header (1 + 2 + 2 + 4 + 2). The rest of the buf is your wiredata
Not super elegant, but no need to add to the API surface.

As for a text parser, I can see the need, but TBF that could live outside of this lib. To play devils advocate no SPF parser is included either, otoh SVCB is different again because those fields end up diff. in the wiredata (which wasn't the brightest idea in that rfc imo)

[neilcook]

Hmm, let's unpack.

To access wiredata, you could make an SVCB record in a Msg, and just pack that and index at the correct byte. If you use a DNS question where the domain name is the root you know that packs to 00 . if the domain of the SVCB is also "." you can index with: header (12)+ question (1 + 2 +2) + RR header (1 + 2 + 2 + 4 + 2). The rest of the buf is your wiredata Not super elegant, but no need to add to the API surface.

As I said above, I was trying to avoid having to create a SVCB record just to create ServiceParams. Also this doesn't handle the Unpack scenario.

As for a text parser, I can see the need, but TBF that could live outside of this lib. To play devils advocate no SPF parser is included either, otoh SVCB is different again because those fields end up diff. in the wiredata (which wasn't the brightest idea in that rfc imo)

Well sure, any of this could live outside the lib. I or anyone else could just write a whole standalone Service Params library and replicate all the great work you've done in this lib, but again this PR was trying to avoid that. Also I'm not sure what SPF has to do with this, as AFAIK they don't have a life outside of DNS records. Whereas Service Params can now be part of DHCP server configuration for example.

I think this is perhaps a philosophical difference of opinion. The changes in RFC 9463 mean that I would now consider Service Params to be a data structure that happens to be part of SVCB RR, but can also be used completely standalone, therefore IMO it requires straightforward APIs to handle that.

If you don't want to change your library's API then that is of course your prerogative. But it would perhaps have been helpful if you'd indicated so when I opened the issue rather than encouraging me to open a PR.

[miekg]

As I said above, I was trying to avoid having to create a SVCB record just to create ServiceParams. Also this doesn't handle the Unpack scenario.

But for Unpack you can just range over the SVCBKey slice, and then pack if want to re-use the DNS encoding? In the DHCP case the SVCB seems to come from the wire?

Exposing Pack/Unpack on an RR level would be new in this lib, it also historical never made sense, because of pointers in the RR and RDATA that would point into the dns message.

I've read the I-D yesterday, and I'm now trying to figure out what actually is something that would be missing?

• yes, a whole Msg for just some ServiceParams bits is a bit silly, but works now and does not need any new code
• parsing or getting the params from the DNS, would imply ranging over []SVCBKey

But what I'm I missing there, getting ServiceParams from a file for example?

[neilcook]

As I said above, I was trying to avoid having to create a SVCB record just to create ServiceParams. Also this doesn't handle the Unpack scenario.

But for Unpack you can just range over the SVCBKey slice, and then pack if want to re-use the DNS encoding? In the DHCP case the SVCB seems to come from the wire?

I'm confused by this. The wire encoding is just the service params, not the whole SVCB RR. So if as a client, I want to unpack the wire encoding of just the SvcParams, how would I do that using the current API?

Exposing Pack/Unpack on an RR level would be new in this lib, it also historical never made sense, because of pointers in the RR and RDATA that would point into the dns message.

I understand it didn't make sense historically. I think it makes sense in this specific use case because SvcParams can now be transmitted over the wire independently of SVCB RRs. It's not even exposing Pack/Unpack of the whole RR, just an element of it. And I do realise this makes the API less elegant, as it's this weird special case.

I've read the I-D yesterday, and I'm now trying to figure out what actually is something that would be missing?

• yes, a whole Msg for just some ServiceParams bits is a bit silly, but works now and does not need any new code

True. It's just that figuring out what to do for anyone using the API is I would say, extremely non trivial, vs a Pack method.

• parsing or getting the params from the DNS, would imply ranging over []SVCBKey

But what I'm I missing there, getting ServiceParams from a file for example?

Yes, that's a good example. A DHCP server serving DNR options, with ServiceParams in the config file. Another example for the Unpack case is a client receiving those options over the wire from the DHCP server.

[miekg]

As I said above, I was trying to avoid having to create a SVCB record just to create ServiceParams. Also this doesn't handle the Unpack scenario.

But for Unpack you can just range over the SVCBKey slice, and then pack if want to re-use the DNS encoding? In the DHCP case the SVCB seems to come from the wire?

I'm confused by this. The wire encoding is just the service params, not the whole SVCB RR. So if as a client, I want to unpack the wire encoding of just the SvcParams, how would I do that using the current API?

See #1490 (comment)
(and index manually)

Exposing Pack/Unpack on an RR level would be new in this lib, it also historical never made sense, because of pointers in the RR and RDATA that would point into the dns message.

I understand it didn't make sense historically. I think it makes sense in this specific use case because SvcParams can now be transmitted over the wire independently of SVCB RRs. It's not even exposing Pack/Unpack of the whole RR, just an element of it. And I do realise this makes the API less elegant, as it's this weird special case.

And PackRR then? (forgot we even had, this, just bumped into it while grepping)

I've read the I-D yesterday, and I'm now trying to figure out what actually is something that would be missing?

• yes, a whole Msg for just some ServiceParams bits is a bit silly, but works now and does not need any new code

True. It's just that figuring out what to do for anyone using the API is I would say, extremely non trivial, vs a Pack method.

• parsing or getting the params from the DNS, would imply ranging over []SVCBKey

But what I'm I missing there, getting ServiceParams from a file for example?

Yes, that's a good example. A DHCP server serving DNR options, with ServiceParams in the config file. Another example for the Unpack case is a client receiving those options over the wire from the DHCP server.

[miekg]

sorry misclicked

[neilcook]

I'm confused by this. The wire encoding is just the service params, not the whole SVCB RR. So if as a client, I want to unpack the wire encoding of just the SvcParams, how would I do that using the current API?

See #1490 (comment) (and index manually)

I'm sorry I still don't understand how that would work. The service params are wire encoded, so you just have a byte array, so I don't understand the comment about ranging over a SVCBKey slice.

I understand it didn't make sense historically. I think it makes sense in this specific use case because SvcParams can now be transmitted over the wire independently of SVCB RRs. It's not even exposing Pack/Unpack of the whole RR, just an element of it. And I do realise this makes the API less elegant, as it's this weird special case.

And PackRR then? (forgot we even had, this, just bumped into it while grepping)

That's great, but the issue of knowing all the steps to get to the packed data you actually want remain, so it doesn't actually address the intent of my PR, namely:

• Simple API to Pack and Unpack standalone SvcParams
• Parser for standalone SvcParams

I don't think we're going to get to agreement on this, so I'll just close this PR. Thanks for the feedback.