Bug 1733 - Enhance support for QoS (ToS) by supporting DSCP/CS and adding option
Summary: Enhance support for QoS (ToS) by supporting DSCP/CS and adding option
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 5.4p1
Hardware: All Linux
: P2 enhancement
Assignee: Assigned to nobody
URL:
Keywords: openbsd, patch
Depends on:
Blocks: V_5_7
  Show dependency treegraph
 
Reported: 2010-03-10 14:39 AEDT by Philip Prindeville
Modified: 2011-09-02 09:53 AEST (History)
3 users (show)

See Also:


Attachments
QoS fix for DSCP/CS markings (14.33 KB, text/plain)
2010-03-10 14:39 AEDT, Philip Prindeville
no flags Details
QoS fix for DSCP/CS markings with sytem-wide config restriction (26.44 KB, patch)
2010-03-14 05:01 AEDT, Philip Prindeville
no flags Details | Diff
QoS fix for DSCP/CS markings with sytem-wide config restriction (26.27 KB, patch)
2010-03-14 05:05 AEDT, Philip Prindeville
no flags Details | Diff
Upstream patch for QoS markings (28.83 KB, patch)
2010-06-21 17:42 AEST, Philip Prindeville
no flags Details | Diff
/home/djm/ssh-qos.diff (15.78 KB, patch)
2010-11-05 14:35 AEDT, Damien Miller
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Prindeville 2010-03-10 14:39:30 AEDT
Created attachment 1808 [details]
QoS fix for DSCP/CS markings

This adds the option "UseQoS" which takes two marking arguments, the first for non-interactive traffic and the second for interactive traffic.

For instance, in a DSCP environment, one could use:

UseQoS af12 cs2

to retain compatibility, the default is:

UseQoS throughput lowdelay
Comment 1 Philip Prindeville 2010-03-14 05:01:17 AEDT
Created attachment 1809 [details]
QoS fix for DSCP/CS markings with sytem-wide config restriction

Add flag to both keywords[] and read_config_file()/process_config_line() to keep track of whether we're parsing a user profile or a system profile.

If it's a user profile and the keyword[].restricted flag is set, don't allow this directive.
Comment 2 Philip Prindeville 2010-03-14 05:05:47 AEDT
Created attachment 1810 [details]
QoS fix for DSCP/CS markings with sytem-wide config restriction
Comment 3 Philip Prindeville 2010-06-21 17:42:39 AEST
Created attachment 1880 [details]
Upstream patch for QoS markings
Comment 4 Damien Miller 2010-07-02 13:53:19 AEST
Why is the restriction that UseQoS may only appear in /etc/ssh/ssh_config necessary? It isn't an effective control, since a user who wanted to circumvent it could just build a ssh without the check. Does the IP_TOS ioctl perform any privilege checks when setting low-precedence traffic classes?
Comment 5 Philip Prindeville 2010-07-02 15:11:46 AEST
(In reply to comment #4)
> Why is the restriction that UseQoS may only appear in
> /etc/ssh/ssh_config necessary? It isn't an effective control, since a
> user who wanted to circumvent it could just build a ssh without the
> check. Does the IP_TOS ioctl perform any privilege checks when setting
> low-precedence traffic classes?

It's a somewhat effective check, since not everyone may have access to recompiled binaries or even sources and a compiler.

It removes for the large majority the need to fiddle with settings they might not (and most likely don't) entirely understand.

And no, the setsockopt(IP_TOS) is entirely unprivileged.
Comment 6 Damien Miller 2010-07-25 09:55:25 AEST
If the setsockopt doesn't perform privilege checks, then why should ssh?
Comment 7 Philip Prindeville 2010-07-25 09:58:28 AEST
(In reply to comment #6)
> If the setsockopt doesn't perform privilege checks, then why should
> ssh?

It's not a privilege check so much as it's an enforcement of system-wide policy.
Comment 8 Damien Miller 2010-07-25 13:38:03 AEST
You have lost me - if the IP_TOS is allowed by any user by the kernel, then whose policy is ssh enforcing?
Comment 9 Philip Prindeville 2010-07-25 14:03:59 AEST
(In reply to comment #8)
> You have lost me - if the IP_TOS is allowed by any user by the kernel,
> then whose policy is ssh enforcing?

Don't think of it as policy enforcement.  Think of it as centralized configuration management for certain critical controls.
Comment 10 Damien Miller 2010-08-03 15:41:00 AEST
We are freezing for the OpenSSH 5.6 release. Retargetting these bugs to the next release.
Comment 11 Damien Miller 2010-08-03 15:42:43 AEST
Targetting OpenSSH 5.7
Comment 12 Philip Prindeville 2010-08-04 03:09:24 AEST
(In reply to comment #9)
> (In reply to comment #8)
> > You have lost me - if the IP_TOS is allowed by any user by the kernel,
> > then whose policy is ssh enforcing?
> 
> Don't think of it as policy enforcement.  Think of it as centralized
> configuration management for certain critical controls.

Didn't see a response to this, so I'm not sure if this question is adequately resolved or not.

In any case, how does the review process usually go?

Regardless of whether this is going into 5.6 or 5.7, how do I get a sign-off on this?

Thanks.
Comment 13 Gary T. Giesen 2010-08-26 16:31:46 AEST
I'd like to see DSCP implemented as well instead of the obsolete (and unconfigurable) IP Precedence model...
Comment 14 Gary T. Giesen 2010-08-26 17:11:39 AEST
Just a further note that having the "system-wide config restriction" is pretty useless if anyone can copy their own binary onto the system and bypass it. I suggest having a default in /etc/ssh/ssh_config, and can can be overridden by the user either in their local config or as an argument on the command line.
Comment 15 Philip Prindeville 2010-08-27 05:04:53 AEST
(In reply to comment #14)
> Just a further note that having the "system-wide config restriction" is
> pretty useless if anyone can copy their own binary onto the system and
> bypass it. I suggest having a default in /etc/ssh/ssh_config, and can
> can be overridden by the user either in their local config or as an
> argument on the command line.

And... this is why I think that would be a bad idea.

You're fairly typical, I'd say, of most users.  You understand what QoS settings are for, and how they can be changed, and what ultimately they're expected to do.

But the finer details would seem to have been missed.

There are no "standard" profiles for interpreting QoS markings, for instance.  There's a proposal (RFC 4594) for uniform markings that ISP's and Enterprises are free to implement, or equally free not to.

So you can set your QoS bits to AF22 thinking that this will do what you want, and there's a good chance you'd be wrong: the only person who knows for sure is the Enterprise network architect that provisioned the entire infrastructure.  Everyone else is just pulling numbers out of their hat.

For *exactly* the same reasons that "PermitRootLogin", "PermitEmptyPasswords", and "IgnoreRhosts" aren't user-settable (because end-users shouldn't be making decisions that potentially impact performance and/or security organization-wide), the QoS bits shouldn't be user-settable either.

If users can wreak havoc by having their own profile containing "PermitRootLogin yes/PermitEmptyPasswords yes", then be aware that they can negatively impact organization-wide networking (especially VoIP and IPTV services) by making poor choices elsewhere.

Make the compelling argument to me that users should be allowed to have their own .sshd_config file, and put into it those latter two settings, and I'll concede that by the same reasoning they should be allowed to set their own QoS policy.

But the bottom line is that QoS markings are as much a part of a site's singular and standardized architectural policy as the type and degree of enforcement of their security measures.
Comment 16 Gary T. Giesen 2010-08-27 06:51:29 AEST
You're confusing the settings for the daemon (sshd_config, which obviously only root should be able to change) with the settings for the client (ssh_config) when someone makes an outbound connection.

The settings for the daemon can't be bypassed since obviously it requires root privileges to launch it to listen on port 22.

The settings for the client should be freely settable by the user, just as it is with the -S option for telnet. I have no problems with having smart defaults in ssh_config, but they definitely should be able to be overridden.

In the end, there's no sense having a setting which provides no security whatsoever (but looks like it does). If a user is malicious, they can compile their own ssh client with the settings they want and bypass your config anyways. Since the kernel doesn't enforce any privileges on the setting of the DSCP markings, you shouldn't either. Thus it only makes sense to provide a configurable default.

Keep in mind it's up to the network to trust and enforce DSCP markings, so that's the proper place for these kind of access controls to appear. Otherwise you'll need to convince the various *nix vendors to require privileges on setting DSCP markings.
Comment 17 Philip Prindeville 2010-08-27 10:11:05 AEST
(In reply to comment #16)
> You're confusing the settings for the daemon (sshd_config, which
> obviously only root should be able to change) with the settings for the
> client (ssh_config) when someone makes an outbound connection.

I'm not confusing them at all, and if you reread my comments you'll see that I explicitly said "make the compelling argument to me that users should be allowed to have their own .sshd_config file".  Not sure what was unclear about that.

Moving on...

> The settings for the daemon can't be bypassed since obviously it
> requires root privileges to launch it to listen on port 22.

Or you could read all users' settings and just use the most permissive of the union...  Or individual users could run their own instances on ports other than 22...  since your argument was that users could have their own binary clients, I'm saying why not take it a step further and have them run their own servers...


> The settings for the client should be freely settable by the user, just
> as it is with the -S option for telnet. I have no problems with having
> smart defaults in ssh_config, but they definitely should be able to be
> overridden.

You're missing the point that this is largely IRRELEVANT.  The network doesn't care if the client or the server is sending mislabeled packets, they both have exactly the same potential to be disruptive to other traffic types like VoIP and IPTV.

I would make the argument that letting the user (and client) set the encryption algorithm, cipher strength, etc. was a serious mistake.  If the user uses too weak an encryption standard, his connection can be eavesdropped or even subverted.

> In the end, there's no sense having a setting which provides no
> security whatsoever (but looks like it does). If a user is malicious,
> they can compile their own ssh client with the settings they want and
> bypass your config anyways. Since the kernel doesn't enforce any
> privileges on the setting of the DSCP markings, you shouldn't either.
> Thus it only makes sense to provide a configurable default.

Did you read comment #9?

> Keep in mind it's up to the network to trust and enforce DSCP markings,
> so that's the proper place for these kind of access controls to appear.
> Otherwise you'll need to convince the various *nix vendors to require
> privileges on setting DSCP markings.

Again, you're missing the obvious: the settings are more easily and better trusted when they're set by someone who actually had a hand in setting the site network policy!  They are more likely to work.

You're thinking of these as a per-host setting that only affects the host, and they're not.  It's a setting that affects the entire network, and should be set in a consistent and coherent way organization-wide.

It's not about what the user might set just because he can.  It's about what should be set by the site administrators because they're actually the most clueful.
Comment 18 Philip Prindeville 2010-08-27 15:06:43 AEST
(In reply to comment #16)

> In the end, there's no sense having a setting which provides no
> security whatsoever (but looks like it does). If a user is malicious,
> they can compile their own ssh client with the settings they want and
> bypass your config anyways. Since the kernel doesn't enforce any
> privileges on the setting of the DSCP markings, you shouldn't either.
> Thus it only makes sense to provide a configurable default.

This is a specious argument.

Look at the man page for libresolv.

The path to /etc/resolv.conf is hardwired in the binary, and that file isn't writable by users.

Yes, users could link against their own version of libresolv, but what would be the point?  They'd just be opening themselves to pointing to the wrong server, an unreliable server, or perhaps even a server that's been compromised and exposes them to DNS-based exploits.

Similarly, there's no interest in having users have their own binaries for ssh that can inject packets with detrimental QoS markings, because they will be making things worse for themselves in the end.  And there's no interest in users having their own QoS settings just as there's no interest in their having their own /etc/resolv.conf file.

Yes, you can do it... but why?  What does it really get you?
Comment 19 Damien Miller 2010-08-27 15:41:07 AEST
resolv.conf is not world writable because it is used by system daemons that shouldn't be subject to reconfiguration by users, not for any of the reasons you cited.

Attempting to enforce DSCP policy in ssh(1) is ineffective and therefore misguided. When I get around to looking at this patch again, I don't plan on merging this part.
Comment 20 Philip Prindeville 2010-08-27 16:32:30 AEST
(In reply to comment #19)
> resolv.conf is not world writable because it is used by system daemons
> that shouldn't be subject to reconfiguration by users, not for any of
> the reasons you cited.

It's not just that resolv.conf isn't writable, there's also no way to override where it looks for the configuration file.  It would have been easy, for instance, to have an environment variable that contained the path to an alternate file to use, but this was deliberately not supported.

> Attempting to enforce DSCP policy in ssh(1) is ineffective and
> therefore misguided. When I get around to looking at this patch again,
> I don't plan on merging this part.

Well, tell you what: let's compare notes from your experiences with all of the site-wide rollouts of QoS that you've done with my experiences, and maybe we can base our conclusions on actual empirical data and not supposition.
Comment 21 Philip Prindeville 2010-10-01 03:26:48 AEST
Looking at this in a similar historical context...

I remember when DNS was first deployed. Prior to it, we had used HOSTS files that we picked up via FTP from the NIC at USC.

A lot of programs even had the addresses of servers compiled into them.

When DNS came out, there were libraries you could link against, but not all programs were released immediately using them.  Indeed, some programs even used their own resolver libraries and had root name servers coded into them.

After a while, the load on the root name servers became too high, and these programs needed to be hunted down and replaced.  Such programs also didn't leverage caching.

At no time, however, did the system-wide libraries (libresolv.a, etc) allow the user to specify an alternate location for the resolver configuration, or to override the name servers to query.

Which turned out to be fortuitous.

Not only can incorrect configuration of the resolver lead to heavy loading on the root name servers, or poor performance due to a lack of local caching... but:

* by diverting DNS queries to a highjacked server, I can intercept a lot of your network traffic (email, for instance, by returning bogus MX records);

* I can subvert your PKI by pointing you at bogus trusted certificate authorities;

* I can cause a denial of service attack by pointing you at bogus NTP servers and over time skewing your clock such that loses critical sync with other server.

etc.

So generating a DNS query is not a privileged operation.  It's a rather banal operation, and superficially affects only the process making the query.  In this respect, it bears a good deal of parallel to the discussion we're having about what's the big deal with setting QoS bits.

Yet through a bit of misconfiguration by the user, it can overwhelm critical network infrastructure (the root name servers), and can open doors up to network attack.

Similarly, misconfiguration of QoS bits by the user can deprive other critical network services of reserved bandwidth, etc.

Why don't we initially just commit it as is, and if someone comes forward with a usage case that absolutely requires per-user configuration of QoS settings, we'll change it then.

Because it's easier to add new configuration capability later once it's proven it's needed, than to revoke it once software is deployed in the field.
Comment 22 Philip Prindeville 2010-10-30 04:10:04 AEDT
Will this go into 6.0 and when is 6.0 due out?
Comment 23 Damien Miller 2010-11-05 14:35:06 AEDT
Created attachment 1949 [details]
/home/djm/ssh-qos.diff

revised QoS diff. factors out common code, removes client-side policy restriction on QoS
Comment 24 Philip Prindeville 2010-11-05 15:20:40 AEDT
(In reply to comment #23)
> Created attachment 1949 [details]
> /home/djm/ssh-qos.diff
> 
> revised QoS diff. factors out common code, removes client-side policy
> restriction on QoS

I would rewrite parse_ipqos() explicitly use hex values only.
Comment 25 Damien Miller 2010-11-14 10:32:42 AEDT
Patch applied - thanks. This will be in OpenSSH 5.7
Comment 26 Damien Miller 2011-01-24 12:34:00 AEDT
Move resolved bugs to CLOSED after 5.7 release
Comment 27 Gary T. Giesen 2011-08-31 14:58:18 AEST
Was there a reason you left out AF21?
Comment 28 Gary T. Giesen 2011-08-31 14:59:42 AEST
Was there a reason you left out AF21?(In reply to comment #27)
> Was there a reason you left out AF21?

Or rather, it's mislabeled:

906 	

	{ "af14", IPTOS_DSCP_AF21 },

907 	

	{ "af22", IPTOS_DSCP_AF22 },

908 	

	{ "af23", IPTOS_DSCP_AF23 },
Comment 29 Philip Prindeville 2011-08-31 15:31:25 AEST
(In reply to comment #28)
> Was there a reason you left out AF21?(In reply to comment #27)
> > Was there a reason you left out AF21?
> 
> Or rather, it's mislabeled:
> 
> 906     
> 
>     { "af14", IPTOS_DSCP_AF21 },
> 
> 907     
> 
>     { "af22", IPTOS_DSCP_AF22 },
> 
> 908     
> 
>     { "af23", IPTOS_DSCP_AF23 },

The question must be for Damien: he modified my patch. It was correct in my version.
Comment 30 Damien Miller 2011-09-02 09:53:13 AEST
Yeah, my bad. I'll commit a fix for the 6.0 release.