[GH-ISSUE #205] Create Proto crate: better SoC for `Signer`/`Verifier`, `KeyPair`/`PublicKey`, etc.

kerem commented

2026-03-15 22:15:27 +03:00

Owner

Originally created by @briansmith on GitHub (Sep 24, 2017).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/205

Originally assigned to: @bluejekyll on GitHub.

Pure client-side DNSSEC only requires public key operations, namely signature verification. It doesn't require signing. Signing components are really heavyweight, especially when things like HSM support are considered. Clients don't need those heavyweight things.

There are a lot of tests where a Signer (KeyPair) signs something and then a Verifier (PublicKey) verifies what was signed. Ideally all of these tests would be moved to the server crate too. However, we still need to test the verifier. In order to do that, we need to pregenerate (e.g. using the Server code, or using another DNSSEC implementation) the signed records that are being validated, and then use those pregenerated signed records in the test. This would allow the client to be tested 100% without requiring the server to be built.

Originally created by @briansmith on GitHub (Sep 24, 2017). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/205 Originally assigned to: @bluejekyll on GitHub. Pure client-side DNSSEC only requires public key operations, namely signature verification. It doesn't require signing. Signing components are really heavyweight, especially when things like HSM support are considered. Clients don't need those heavyweight things. There are a lot of tests where a Signer (KeyPair) signs something and then a Verifier (PublicKey) verifies what was signed. Ideally all of these tests would be moved to the server crate too. However, we still need to test the verifier. In order to do that, we need to pregenerate (e.g. using the Server code, or using another DNSSEC implementation) the signed records that are being validated, and then use those pregenerated signed records in the test. This would allow the client to be tested 100% without requiring the server to be built.

kerem

2026-03-15 22:15:27 +03:00

closed this issue
added the
crate:client

crate:proto

cleanup

crate:resolver
labels

kerem commented

2026-03-15 22:15:43 +03:00

Author

Owner

@bluejekyll commented on GitHub (Sep 24, 2017):

Is this a blocker for removing OpenSSL from the tests?

@bluejekyll commented on GitHub (Sep 24, 2017): Is this a blocker for removing OpenSSL from the tests?

kerem commented

2026-03-15 22:15:48 +03:00

Author

Owner

@briansmith commented on GitHub (Sep 24, 2017):

'> Is this a blocker for removing OpenSSL from the tests?

There are various ways of getting to that point. I filed this after reading the "Dynamic update" example in client/lib.rs, which uses OpenSSL APIs. I first thought about rewriting it to be not-OpenSSL-specific, but then I thought "why is this even in client/ in the first place?"

I think this is something that should be done anyway, so I guess if this can be done quickly, it will avoid some unnecessary work. OTOH, eventually Signer and KeyPair should have full functionality with ring; I just think that will be a while because I've no schedule for implementing RSA key generation in ring. (Another alternative would be to use ECDSA and/or Ed25519 in the tests and demos. Another alternative is to pregenerate the key pairs.)

@briansmith commented on GitHub (Sep 24, 2017): '> Is this a blocker for removing OpenSSL from the tests? There are various ways of getting to that point. I filed this after reading the "Dynamic update" example in client/lib.rs, which uses OpenSSL APIs. I first thought about rewriting it to be not-OpenSSL-specific, but then I thought "why is this even in client/ in the first place?" I think this is something that should be done anyway, so I guess if this can be done quickly, it will avoid some unnecessary work. OTOH, eventually Signer and KeyPair should have full functionality with *ring*; I just think that will be a while because I've no schedule for implementing RSA key generation in *ring*. (Another alternative would be to use ECDSA and/or Ed25519 in the tests and demos. Another alternative is to pregenerate the key pairs.)

kerem commented

2026-03-15 22:15:53 +03:00

Author

Owner

@bluejekyll commented on GitHub (Sep 24, 2017):

I just realized something. I don't think this can be easily moved. The Signer is used by the Client for SIG0 record creation. This is currently the only form of authentication support by the TRust-DNS for dynamic update, it's also the only option for compatibility with BIND9.

I do want to allow TRust-DNS servers to authenticate with TLS, but even in that case I expect people will want to use SIG0 for compatibility with other DNS servers/authorities.

@bluejekyll commented on GitHub (Sep 24, 2017): I just realized something. I don't think this can be easily moved. The Signer is used by the Client for `SIG0` record creation. This is currently the only form of authentication support by the `TRust-DNS` for dynamic update, it's also the only option for compatibility with `BIND9`. I do want to allow `TRust-DNS` servers to authenticate with TLS, but even in that case I expect people will want to use SIG0 for compatibility with other DNS servers/authorities.

kerem commented

2026-03-15 22:15:58 +03:00

Author

Owner

@briansmith commented on GitHub (Sep 24, 2017):

Dynamic update is a server feature, not a client feature. In particular, when one server delegates the serving of DNS to another DNS server, it will send dynamic update requests to the delegated server. Or, put another way, I think of things like "client," "server management," and "server," and dynamic update requests are a "server management" feature. A "normal" client is not going to need to send dynamic update requests.

@briansmith commented on GitHub (Sep 24, 2017): Dynamic update is a server feature, not a client feature. In particular, when one server delegates the serving of DNS to another DNS server, it will send dynamic update requests to the delegated server. Or, put another way, I think of things like "client," "server management," and "server," and dynamic update requests are a "server management" feature. A "normal" client is not going to need to send dynamic update requests.

kerem commented

2026-03-15 22:16:03 +03:00

Author

Owner

@bluejekyll commented on GitHub (Sep 24, 2017):

By "normal" client, in this context I would define that as a "resolver", and you're of course correct that the "resolver" doesn't need to sign requests. The TRust-DNS Resolver crate, for example, will never sign a query, and I would look at it as a bug to do so.

That being said, the logic for signing update requests currently resides in the Client crate. A potential solution to this is to start splitting the Client up in to multiple crates, for example a protocol crate which would only implement the DNS protocol, with basic sending and receiving of messages implemented, with the Resolver and "Server Manager" (for lack of a better name at the moment) being built atop of that. Doing this may clean up some issues in the code, but it means rethinking certain choices that were made early on. For example, I don't want the API to expose the query id association logic with the message, as that is a potential avenue for an attack of spoofing message responses (although if everything was just DNSSec I think this attack vector becomes mostly irrelevant). Right now, the id is generated, then the message is signed. We could refactor all of this of course to do things like add message handler facilities into the protocol crate that are run at different stages of the request being sent, but this seems like a lot of work... while the outcome might be cleaner separation of concerns (a valid goal), I'm not sure what the total value add would be.

I also worry that the crate overhead might become too high in a context like that, but that can be dealt with in the build scripts. There would need to be a lot more education of the community about which crate to use (and I'm already fairly bad at this aspect of managing the project ;).

@bluejekyll commented on GitHub (Sep 24, 2017): By "normal" client, in this context I would define that as a "resolver", and you're of course correct that the "resolver" doesn't need to sign requests. The TRust-DNS Resolver crate, for example, will never sign a query, and I would look at it as a bug to do so. That being said, the logic for signing update requests currently resides in the Client crate. A potential solution to this is to start splitting the Client up in to multiple crates, for example a protocol crate which would only implement the DNS protocol, with basic sending and receiving of messages implemented, with the Resolver and "Server Manager" (for lack of a better name at the moment) being built atop of that. Doing this may clean up some issues in the code, but it means rethinking certain choices that were made early on. For example, I don't want the API to expose the query id association logic with the message, as that is a potential avenue for an attack of spoofing message responses (although if everything was just DNSSec I think this attack vector becomes mostly irrelevant). Right now, the id is generated, then the message is signed. We could refactor all of this of course to do things like add message handler facilities into the protocol crate that are run at different stages of the request being sent, but this seems like a lot of work... while the outcome might be cleaner separation of concerns (a valid goal), I'm not sure what the total value add would be. I also worry that the crate overhead might become too high in a context like that, but that can be dealt with in the build scripts. There would need to be a lot more education of the community about which crate to use (and I'm already fairly bad at this aspect of managing the project ;).

kerem commented

2026-03-15 22:16:08 +03:00

Author

Owner

@briansmith commented on GitHub (Sep 25, 2017):

IMO, there are a lot of applications, e.g. web browsers and other desktop and web service clients, that will want only resolver functionality, and want to have a very small, well-audited, small-number-of-dependencies implementation of the resolver. So I definitely think that something should be done to make the dependencies of the resolver smaller, by moving functionality from client (etc.) to crates that the resolver doesn't depend on. In turn I think this would make the TRust-DNS resolver more interesting to more people, which would encourage more contributions (code, auditing, documentation, testing, etc.) for that code. For example, I'd like to work on making some aspects of the resolver more efficient in various ways and I'd like to help integrate the resolver with more projects.

By contrast, I probably can't afford to invest too much in server-side functionality or dynamic update, so my (selfish) goal is to move that code out of the TCB for the resolver. It's understandable if your priorities are different than mine, though. :)

Anyway, I started a PoC to do this, but #200 is a sticking point: I'd like to move TBS to the server along with Signer, but currently it is (mis-)used by Verifier, so I got stuck.

@briansmith commented on GitHub (Sep 25, 2017): IMO, there are a lot of applications, e.g. web browsers and other desktop and web service clients, that will want only resolver functionality, and want to have a very small, well-audited, small-number-of-dependencies implementation of the resolver. So I definitely think that something should be done to make the dependencies of the resolver smaller, by moving functionality from client (etc.) to crates that the resolver doesn't depend on. In turn I think this would make the TRust-DNS resolver more interesting to more people, which would encourage more contributions (code, auditing, documentation, testing, etc.) for that code. For example, I'd like to work on making some aspects of the resolver more efficient in various ways and I'd like to help integrate the resolver with more projects. By contrast, I probably can't afford to invest too much in server-side functionality or dynamic update, so my (selfish) goal is to move that code out of the TCB for the resolver. It's understandable if your priorities are different than mine, though. :) Anyway, I started a PoC to do this, but #200 is a sticking point: I'd like to move TBS to the server along with Signer, but currently it is (mis-)used by Verifier, so I got stuck.

kerem commented

2026-03-15 22:16:14 +03:00

Author

Owner

@bluejekyll commented on GitHub (Sep 25, 2017):

The issues with Verifier and Signer stem from the fact that they both started as just the Signer.

...want only resolver functionality, and want to have a very small, well-audited, small-number-of-dependencies implementation of the resolver.

You make very compelling arguments :)

Here's my proposal: move the majority of the current Client functionality to a new crate, Proto; this would reduce the disruption to current users. This makes the most sense to me: leave the existing Client in place, create the new Proto crate, to which we'd migrate most of the Client crate, except for the high level stuff. At that point the TRust-DNS libraries would look like this: (Resolver -> Proto) and (Server -> Client -> (Resolver?) -> Proto). There's some complexity here because I still want the Client to use the DNSSec validation which would now live in the Proto crate, but shouldn't be too hard. I think it would be transparent (mostly) to any current users of either the Resolver or the Client.

That might end up being the easiest. Then we'd strongly encourage users to use the Resolver instead of the Client for DNS' most common use cases. This sound good to you? All of the DNSSec stuff would migrate to the Proto crate, along with serialization, etc. Signer would stay with Client, and Verifier would migrate to proto. This should also have the side-effect of cleaning up any inter-dependence between them. The tests will be very annoying though.

It's understandable if your priorities are different than mine, though. :)

I think they only differ in so far as what I'm most interested in, new Server features (but I've been much more focused on the Resolver/Client lately). Give the people what they want, it's never a bad thing to have more users :)

There are people using it for dynamic update operations, so it's not that rare ;)

@bluejekyll commented on GitHub (Sep 25, 2017): The issues with Verifier and Signer stem from the fact that they both started as just the Signer. > ...want only resolver functionality, and want to have a very small, well-audited, small-number-of-dependencies implementation of the resolver. You make very compelling arguments :) Here's my proposal: move the majority of the current Client functionality to a new crate, Proto; this would reduce the disruption to current users. This makes the most sense to me: leave the existing Client in place, create the new Proto crate, to which we'd migrate most of the Client crate, except for the high level stuff. At that point the TRust-DNS libraries would look like this: (Resolver -> Proto) and (Server -> Client -> (Resolver?) -> Proto). There's some complexity here because I still want the Client to use the DNSSec validation which would now live in the Proto crate, but shouldn't be too hard. I think it would be transparent (mostly) to any current users of either the Resolver or the Client. That might end up being the easiest. Then we'd strongly encourage users to use the Resolver instead of the Client for DNS' most common use cases. This sound good to you? All of the DNSSec stuff would migrate to the Proto crate, along with serialization, etc. Signer would stay with Client, and Verifier would migrate to proto. This should also have the side-effect of cleaning up any inter-dependence between them. The tests will be very annoying though. > It's understandable if your priorities are different than mine, though. :) I think they only differ in so far as what I'm most interested in, new Server features (but I've been much more focused on the Resolver/Client lately). Give the people what they want, it's never a bad thing to have more users :) There are people using it for dynamic update operations, so it's not *that* rare ;)

kerem commented

2026-03-15 22:16:19 +03:00

Author

Owner

@briansmith commented on GitHub (Sep 25, 2017):

I think your idea sounds great.

There is this chunk of code that is currently in client/:

                    // update messages need to be signed.
                    if let OpCode::Update = message.op_code() {
                        if let Some(ref signer) = self.signer {
                            // TODO: it's too bad this happens here...
                            if let Err(e) = message.sign(signer, Utc::now().timestamp() as u32) {
                                warn!("could not sign message: {}", e);
                                complete.send(Err(e.into())).expect(
                                    "error notifying wait, possible future leak",
                                );
                                continue; // to the next message...
                            }
                        }
                    }

This is the main dependency on Signer. Signer depends on KeyPair and thus all the private-key crypto stuff.

Perhaps an easy way to resolve this would be to make trust_dns_proto::Signer a trait and then have the existing trust_dns::Signer implement trust_dns_proto::Signer.

Then we'd have signer.rs, keypair.rs, key_format.rs, and all the tests that use a Signer in client/, and most of the rest of the stuff in proto/?

@briansmith commented on GitHub (Sep 25, 2017): I think your idea sounds great. There is this chunk of code that is currently in `client/`: ```rust // update messages need to be signed. if let OpCode::Update = message.op_code() { if let Some(ref signer) = self.signer { // TODO: it's too bad this happens here... if let Err(e) = message.sign(signer, Utc::now().timestamp() as u32) { warn!("could not sign message: {}", e); complete.send(Err(e.into())).expect( "error notifying wait, possible future leak", ); continue; // to the next message... } } } ``` This is the main dependency on `Signer`. Signer depends on `KeyPair` and thus all the private-key crypto stuff. Perhaps an easy way to resolve this would be to make `trust_dns_proto::Signer` a trait and then have the existing `trust_dns::Signer` implement `trust_dns_proto::Signer`. Then we'd have signer.rs, keypair.rs, key_format.rs, and all the tests that use a Signer in client/, and most of the rest of the stuff in proto/?

kerem commented

2026-03-15 22:16:24 +03:00

Author

Owner

@briansmith commented on GitHub (Sep 25, 2017):

The tests will be very annoying though.

For the interim, all the tests would be in client/. Later, we can add separate tests just for Verifier that use fixed test vectors and so don't require a Signer. Basically, in the interim, you'd have to run the client/ tests to test the proto/ Verifier.

@briansmith commented on GitHub (Sep 25, 2017): > The tests will be very annoying though. For the interim, all the tests would be in client/. Later, we can add separate tests just for Verifier that use fixed test vectors and so don't require a Signer. Basically, in the interim, you'd have to run the client/ tests to test the proto/ Verifier.

kerem commented

2026-03-15 22:16:29 +03:00

Author

Owner

@briansmith commented on GitHub (Sep 25, 2017):

Here's a problem I see: Right now "trust-dns" has a "openssl" implicit default feature that enables crypto stuff. After the split, it would need to have an explicit "openssl" default feature that depends on the "trust-dns-proto/openssl" feature. However, keeping the OpenSSL-based tests in "trust-dns" (not moving them to "trust-dns-proto") requires keeping the dev-dependency on openssl in trust-dns. However, an explicit feature cannot have the same name as a dependency; only implicit features can! And, the "ring" feature has the same problem, potentially. So, AFAICT, with the split you suggest, it would be impossible to have a 100%-backward-compatible API, because the "openssl" and "ring" features need to be renamed. However, the default would still be backward compatible.

@briansmith commented on GitHub (Sep 25, 2017): Here's a problem I see: Right now "trust-dns" has a "openssl" implicit default feature that enables crypto stuff. After the split, it would need to have an explicit "openssl" default feature that depends on the "trust-dns-proto/openssl" feature. However, keeping the OpenSSL-based tests in "trust-dns" (not moving them to "trust-dns-proto") requires keeping the dev-dependency on openssl in trust-dns. However, an explicit feature cannot have the same name as a dependency; only implicit features can! And, the "ring" feature has the same problem, potentially. So, AFAICT, with the split you suggest, it would be impossible to have a 100%-backward-compatible API, because the "openssl" and "ring" features need to be renamed. However, the default would still be backward compatible.

kerem commented

2026-03-15 22:16:34 +03:00

Author

Owner

@bluejekyll commented on GitHub (Sep 25, 2017):

it would be impossible to have a 100%-backward-compatible API

My goal isn't to make it 100% backward compatible, but to make it at least easy and obvious to upgrade where it differs. I was able to do this when swapping out direct mio usage for tokio. If we can keep the default settings as the upgrade path, then we will be good.

@bluejekyll commented on GitHub (Sep 25, 2017): > it would be impossible to have a 100%-backward-compatible API My goal isn't to make it 100% backward compatible, but to make it at least easy and obvious to upgrade where it differs. I was able to do this when swapping out direct mio usage for tokio. If we can keep the default settings as the upgrade path, then we will be good.

kerem referenced this issue

2026-03-16 02:04:35 +03:00

[PR #389] [CLOSED] Bump futures from 0.1.21 to 0.2.0 #1401

Rows
Columns

[GH-ISSUE #205] Create Proto crate: better SoC for Signer/Verifier, KeyPair/PublicKey, etc. #389

[GH-ISSUE #205] Create Proto crate: better SoC for `Signer`/`Verifier`, `KeyPair`/`PublicKey`, etc. #389