Skip to content
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

Feature Request: add sanity checks to hsw-cli when adding/modifying records #32

Open
mrose17 opened this issue Jan 9, 2021 · 3 comments

Comments

@mrose17
Copy link

mrose17 commented Jan 9, 2021

Some of the data in the HNS root zone are "wrong", e.g., IP addresses being used as the name of NS RRs (instead of as the value of the RR).

i suggest that hsw-cli be extended to include a module that does a syntax/sanity check when records are created/modified, e.g., as in:

    hsw-cli rpc sendupdate easyhandshake '{"records": [ ... ]}'

If you go to any DNS service that let's you edit RRs, they all perform such checks (or at least they should). I suggest taht hsw-cli do the same!

@tynes
Copy link
Contributor

tynes commented Jan 10, 2021

There is an RPC on the node that can be used for this, validateresource
See handshake-org/hsd#429

@pinheadmz
Copy link
Member

pinheadmz commented Jan 10, 2021

@mrose17 thanks for bringing this up. hs-client is just an empty wrapper client to make calls to an hsd node, so features like this would likely go in hsd itself. Like @tynes said we have a validateresource rpc already that checks for basic syntax errors in the resource JSON object before the user calls sendupdate -- but perhaps it could be expanded.

For example, reminiscent of Bitcoin Core's testmempoolaccept, validateresource could return a complex JSON object listing arrays of "errors" and "warnings", where errors are the current syntax tests and warnings are violations of traditional DNS configuration.

Possible example result

{
  errors: [
    "Invalid DS record. KeyTag must be a uint16.",
    "Invalid GLUE6 record. ns must be a valid name."
  ],
  warnings: [
    "Unknown DS algorithm identifier",
    "DS Digest length does not match digestType",
    "Glue record is not a subdomain of the zone"
  ]
}

Marshall I'd appreciate your suggestions for things to test

@mrose17
Copy link
Author

mrose17 commented Jan 10, 2021

@pinheadmz - when in doubt, see what the isc did with bind. in this case, we have https://kb.isc.org/docs/aa-01127 ... so my short answer is to get the source for that and integrate that into validateresource. hopefully, you can just copy-paste without having to do any transcoding by hand.

the other thing I would check is to see how the invalid RRs got into the HNS zone. if sendupdate wasn't used, i suppose there isn't much that can be done; otherwise, it is almost certainly the case that when updating records that a syntax check is mandated. (although I suppose you could add a "i know what i'm doing and please do not sanity-check this parameter" option on sendupdate).

does that make sense?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants