Remove support for SHOPT_AUDIT and SHOPT_AUDITFILE?

[krader1961]

While working on issue #210 I noticed that the legacy build defined the following two symbols but the new Meson based build does not:

-DSHOPT_AUDIT
-DSHOPT_AUDITFILE="/etc/ksh_audit"

The source code implies this has something to do with recording keystrokes. But googling turns up a few pages such as https://www.ibm.com/developerworks/aix/library/au-korn93/ which says:

New features of the Korn Shell provide system administrators and management with the ability to monitor, track, record, and audit every command executed by any user of a system. This is different from the normal shell history, and provides detailed information that includes date, time, tty, user, and the command. This information can be stored locally or transmitted in real time to a remote logging system.

This blog post contains more detail. The author of that article concludes with this paragraph:

Most of the information provided in this post is not documented in a single place anywhere that I can find by searching the Internet. The ksh93 man page does not mention either the accounting or auditing facilities. Even the ksh93 source code is somewhat vague. I gleened most of this information by studying the code in ../src/cmd/ksh93/edit/history.c. If I got anything wrong, please let me know so that I can update this post.

I recommend removing this feature. The code protected by those symbols is so miniscule it is reasonable to assume the feature itself is a security hole. Furthermore, this "feature" is trivial to bypass. Too, I've never seen this feature in any other shell I've used or even proposed. Therefore the added complexity can't be justified.

[krader1961]

Not surprisingly you can also find people reporting problems building ksh with this feature enabled: https://access.redhat.com/solutions/234843

See also https://administratosphere.wordpress.com/2011/05/20/logging-every-shell-command/

[krader1961]

Based on feedback in some other issues it appears that at least a couple of distros have been building ksh with this feature enabled. The question is whether they knew they were doing so (i.e., enabled it deliberately) and is there anyone actually using this feature? I suspect the answers are "no" and "no". If you have reason to believe otherwise please speak up and comment on this issue.

[fpmurphy]

@krader1961. Auditing of shell usage is a required feature in most high security facilities such as DoD, NSA, NRO, etc. You are not getting feedback from the appropriate people because you are not engaging the appropriate people.. If anything, auditing in ksh93 should be enhanced.

[krader1961]

@fpmurphy, Okay, how do we get feedback from the appropriate people? I consider myself to be more security conscious than most IT people. I've read Bruce Schneier's books and subscribe to his blog. I agree that security, including auditing of actions performed, is important. But the existing ksh code guarded by SHOPT_AUDIT appears to be undocumented. Furthermore, the implementation seems to be inadequate for solving the problem it purports to address. Can you provide evidence my assessment is incorrect? I don't see an equivalent feature in zsh or bash. You seem to be claiming that you know that security conscious entities are explicitly using ksh due to the feature guarded by the SHOPT_AUDIT. Care to provide a citation for that assertion?

[krader1961]

I just learned that Red Hat had a customer using RHEL explicitly request this be enabled in the RH builds. Which means we have to retain it. So I'll just do the same thing I've done for some of the other features controlled by a SHOPT_\* symbol and just drop the SHOPT_AUDIT symbol so the feature is enabled in every ksh build.

[siteshwar]

We have to retain this feature as PCI audits require it.

[dannyweldon]

Not sure how secure this is though if any user can view the audit log and see the commands of other users who may have accidentally or carelessly embedded a password into the command line.

[kdudka]

@krader1961 I see that bash supports both libaudit and syslog. Just grep the source code for these strings.

@dannyweldon Why would anyone configure the audit log to be readable by arbitrary users? If ksh allows it, then the administrator can indeed create an insecure setup. But it does not mean that the ksh auditing feature itself is insecure.

[fpmurphy]

@krader1961. Your experience of shell development seems to be extremely limited. If you were the least bit security conscious and SDLC-aware, you would not be trampling like a bull in a china shop all over the existing ksh93 source code, chopping or modifying feature after feature because you do not feel anybody is using that particular feature. Organizations that use ksh93 (and other shells) use these shells in ways that you are obviously totally unaware of. Organizations expect stability whereby new versions of a shell do not break existing scripts, not the radical changes you are making. Nobody is going to ever touch ksh93.new if it breaks their existing scripts! Leave the SHOPT_AUDIT and all the other feature defines alone.

[krader1961]

@fpmurphy, Your feedback is not constructive. If you want ksh93 with 100% compatibility with the last stable release by AT&T you're welcome to use that version, bugs and all. The point of all this activity is to try and modernize the code base so that it is easier for the open source community to contribute to its maintenance. And that entails removing things that are no longer relevant.

So far we've only removed one feature, SHOPT_FIXEDARRAY, because it was untested and based on an inspection of the code not ready for prime time. If it turns out that anyone is using that feature it won't be much work to reinstate that code. The only other item removed is support for the long obsolete AT&T UWIN environment. 99% of the other changes involve simplifying the code by always enabling a feature, such as SHOPT_MULTIBYTE, so that users don't have to worry about whether or not a particular distro chose to build with the feature enabled. Also, removing support for things we no longer need to support like pre-ANSI C compilers.

And for your information I am more than "the least bit security conscious". While computer security is not my specialty I have read several books on the topic, have been reading Bruce Schneier's "CRYPTO-GRAM" emails almost as long he has been writing them, and have worked for companies like IBM and Google which take security extremely seriously.

The reason I opened this issue was to determine whether the complexity needed to be retained. Especially since it is trivial to bypass and therefore provides very little additional security by itself unless a lot of other steps have been taken to limit or audit the ability of an end user to execute commands or syscalls via means other than ksh.

[krader1961]

@fpmurphy, You also apparently were so eager to rant that you didn't read my previous comment where I said we should "drop the SHOPT_AUDIT symbol so the feature is enabled in every ksh build." That is, at the time you made your unhelpful comment, I had already agreed, approximately a full day earlier, this feature needed to be retained. There is a reason we're opening issues like this with a "RFC" label. We want to simplify the project by removing code that isn't needed. We don't want to break existing uses. Thus we're soliciting feedback before removing any code. Having said that it is clear the next release will have to be a major release because even if we made no changes that explicitly break backward compatibility a major release is warranted due to

a) the amount of time that has elapsed since every distro made their most recent ksh release, coupled with

b) other changes such as the build system going from Nmake to Meson, enabling most features unconditionally, and removing dependencies on things like VMALLOC (high on my TODO list).

[krader1961]

@kdudka, You wrote

I see that bash supports both libaudit and syslog. Just grep the source code for these strings.

Yes, but where is that documented outside of the source code? I have no objection to retaining, and improving, this feature. What I object to is retaining a feature that is undocumented and is of dubious value. Having said that I have no objection to retaining this, seemingly flawed, feature since the amount of source code is small and easy to understand (and potentially unit test).

[krader1961]

FFS! Look at that the src/cmd/ksh93/edit/history.c module. It requires that the user running ksh be able to open the SHOPT_AUDITFILE for reading. It then requires the user be able to open the actual audit file for writing if it is a local file. This only qualifies as a security feature if the SHOPT_AUDITFILE specifies a network path such as /dev/tcp/host/port per @fpmurphy's blog post. Which is not the same thing as explicitly supporting logging via syslog.

I was going to enable this "feature" unconditionally but am now inclined to just nuke that code. Security is hard. Half assed solutions like this one benefits no one other than bad actors (e.g., hackers).

[kdudka]

On Thursday, January 18, 2018 6:42:21 AM CET Kurtis Rader wrote: @fpmurphy, Your feedback is not constructive. If you want ksh93 with 100% compatibility with the last stable release by AT&T you're welcome to use that version, bugs and all. The point of all this activity is to try and modernize the code base so that it is easier for the open source community to contribute to its maintenance.

I disagree. By putting such responses, it is you who is not constructive. @fpmurphy wrote it maybe in a too expressive way but I totally agree with the message behind that: You often ask on GitHub whether someone uses the feature XY. If nobody replies at least with "I know someone who uses it", you blindly assume that the feature is unused. I found this approach quite problematic, given the fact that vast majority of current ksh users do not know about this GitHub project at all.

And that entails removing things that are no longer relevant.

The problem is that it should be ksh users who decide what is relevant and you have no reliable way to get feedback from the existing users of ksh.

So far we've only removed one feature, SHOPT_FIXEDARRAY, because it was untested and based on an inspection of the code not ready for prime time. If it turns out that anyone is using that feature it won't be much work to reinstate that code.

You are saying that it is easy to reinstate that code. Is not it then even easier to just keep the code there? You expect ksh users to give you the list of missing features after you release the new major version of ksh, so that you can reintroduce some features based on the feedback. But I do not think it is going to work like that... In most cases, the feedback will be just "My scripts broke after ksh update, so we had to downgrade it back." The users do not have man power to debug their scripts to discover what exactly broke them. Speaking for Red Hat, we have enough work with debugging breakages in the stable ksh packages. Needless to say that many ksh scripts are proprietary and cannot be shared with you (or even Red Hat) to help with the debugging.

[dannyweldon]

I have not tested this but would it be possible for the audit file to be setup as a named pipe with a process continually reading it and shipping lines off to syslog? That would then mitigate users being able to read or trash the file.

[dannyweldon]

Sorry, I just read that the audit file can ship files to syslog using /dev/udp or /dev/tcp, so that settles it as secure as far as I am concerned.

It may not have been documented in the ksh man page so that a casual user reading it would be unaware of the auditing facility.

[KeithBierman]

@kamil "You often ask on GitHub whether someone uses the feature XY. If nobody replies at least ​..." ​IMNSHO is a fair point. When I was at Sun, we had a pretty large collection of code (scripts, C, C++, Fortran, etc.) accumulated from customer engagements that we used as in-house test vehicles. I'd expect that RH would have a similar collection, and the RH contributors should be (perhaps silently, and implicitly) testing against it, mining it for usage, etc. I can appreciate that the *details* of such a repository may well be confidential. But if the RH contributors can confirm that it is not *ONLY* the discussion here on github which is driving that would be comforting ;> Keith Bierman [email protected] 303 997 2749

…

[krader1961]

@kdudka, I agree my choice of words was poor. I was trying to avoid name calling. That @fpmurphy was pointing out that a proposed removal of a feature was a bad idea is obviously constructive. What I take exception to is the verbal abuse (what you termed "too expressive") and mischaracterization of what has been done or will be done (e.g., "chopping or modifying feature after feature").

Are you okay with PR #370 by your coworker that alters the behavior of the echo builtin? If you are how is that different from any of the other changes being discussed?

Also, I am not "blindly assume(ing) that the feature is unused". That's the whole point of opening these issues with a RFC label. What I am doing is looking at the project and assessing the code and documentation. When I see a feature like this one which is undocumented and has at least one huge security hole it seems reasonable to ask if it should be maintained.

What I hear you saying, @kdudka, is that backward compatibility trumps all other concerns. Which I can appreciate from my time as a level three support engineer for Sequent and IBM (where we had customers which paid us for RHEL support 😺 ). But that stance would seem to imply that Red Hat would never, at least without changes to the code, adopt the ksh from the 2016-01-10-beta branch since it makes several commands builtins unconditionally:

$ type stty
stty is a shell builtin version of /bin/stty
$ type cat
cat is a shell builtin version of /bin/cat

Those commands are not builtins in the last release that it seems like every distro adopted: 93u+. Making them builtins that override the external commands of the same name on a user's system is most definitely a backward incompatible change. How is that any more acceptable than some of the changes I have proposed?

It seems like you're saying Red Hat will never adopt the code from this project unless it includes nothing other than bug fixes to the ksh 93u+ release. Which I can appreciate as the safest, least effort, stance for your company. However, that means the death of ksh.

You are saying that it is easy to reinstate that code. Is not it then even easier to just keep the code there?

No, because it had at least one block that was nominally predicated on the symbol but wasn't compiled: #if xSHOPT_FIXEDARRAY (see issue #234). It is also undocumented and untested. But since you insist I'll revert the commit that removed it. TBD is what to do about the aforementioned block of code.

The problem with all the #if something blocks of code (outside of platform specific tests) is that it makes it extremely hard to reason about the code, make it conform to a consistent style, lint it, etcetera. Which is why I want to see features such as the code guarded by SHOPT_AUDIT included unconditionally if the feature is deemed good enough for the next release.

@dannyweldon, You wrote

I just read that the audit file can ship files to syslog using /dev/udp or /dev/tcp, so that settles it as secure as far as I am concerned.

Sure, the ability to ship the audit info over a network connection makes it (sort of) secure. The problem is that the implementation even allows directly appending to a file that the user can modify via other mechanisms. That capability should not exist. Jeebus H. Christ! We should not be making it easy for hackers to create mischief.

You often ask on GitHub whether someone uses the feature XY.

What can we do, @kdudka, to improve visibility about the work being done in this project? We've already sent a couple of emails to the legacy mailing lists asking that people monitor this Github project. What else, in your opinion, should we be doing to let people who care about the future of ksh know that its development is no longer in a coma?

[siteshwar]

@KeithBierman We have some internal test cases that I have started running regularly against RHEL 7 copr builds. And I can confirm more than 90% of the test cases are still passing. Most of these test cases do not contain any confidential data so I plan to upstream them at some point. The only problem is that both the upstream test suite and our internal test cases only cover a subset of ksh functionality, so we will need more test cases and manual testing from community.

[krader1961]

Also, note that the ability to ship the data to a server via TCP or UDP is not secure. It's no better than the SMTP protocol from 20+ years ago (and before encrypting individual messages was common) when it was trivial to spoof emails. I'm still going to enable this code unconditionally because of all the people demanding that the feature be retained. But I'm seriously perplexed how anyone could think this is a trustworthy command audit facility. No IT security professional I know would rely on this feature.

[kdudka]

On Friday, January 19, 2018 6:08:37 AM CET Kurtis Rader wrote: Are you okay with PR #370 by your coworker that alters the behavior of the `echo` builtin? If you are how is that different from any of the other changes being discussed?

I am pretty confident that @siteshwar knows what he is doing. He has been successfully maintaining ksh in RHEL-6 and RHEL-7 for the past few minor releases. I do not think that the change is going to break something in our environments. But, if you think that it can cause problems elsewhere, please speak up now.

Also, I am not "blindly assume(ing) that the feature is unused". That's the whole point of opening these issues with a RFC label. What I am doing is looking at the project and assessing the code and documentation. When I see a feature like this one which is undocumented and has at least one huge security hole it seems reasonable to ask if it should be maintained.

That is a fair approach but not in the current situation. I would personally defer these steps to the point where you have released a few new versions of ksh that were successfully accepted by the downstream consumers and where you are already recognized as the new healthy upstream of ksh. We are not yet in this situation.

What I hear you saying, @kdudka, is that backward compatibility trumps all other concerns. Which I can appreciate from my time as a level three support engineer for Sequent and IBM (where we had customers which paid us for RHEL support 😺 ). But that stance would seem to imply that Red Hat would never, at least without changes to the code, adopt the `ksh` from the `2016-01-10-beta` branch since it makes several commands builtins unconditionally:

FYI we have not updated ksh to this version even in Fedora. @siteshwar would tell you why exactly. If I remember correctly, there were more issues with compatibility than just the built-ins.

It seems like you're saying Red Hat will never adopt the code from this project unless it includes nothing other than bug fixes to the ksh `93u+` release. Which I can appreciate as the safest, least effort, stance for your company. However, that means the death of ksh.

I think we are getting out of a technical discussion here. Our perception of the marketplace for ksh simply differs. You want to make ksh better at the cost of loosing compatibility with existing scripts. Red Hat's customers need to keep their legacy scripts running effortlessly. They are mainly interested to fix security issues and occasional crashes in ksh. We just need to find a way how to work together on our common goals, which I think is slowly happening now.

What can we do, @kdudka, to improve visibility about the work being done in this project? We've already sent a couple of emails to the legacy mailing lists asking that people monitor this Github project. What else, in your opinion, should we be doing to let people who care about the future of `ksh` know that the its development is no longer in a coma?

If I was working on this myself, I would start with releasing a few bug fix only releases of ksh. If they were accepted well by the community, it would attract more people to the Github project and allowed for a broader discussion with target consumers on the bigger changes.

[krader1961]

If I was working on this myself, I would start with releasing a few bug fix only releases of ksh.

Does that mean bug fixes against the ksh93u+ version that is what every distro seems to ship or the ksh93v- version that was open sourced? What would be the point of a release that only included bug fixes against the ksh93v- version since, AFAICT, that is not the version shipped by any mainstream distro? Therein lies the problem.

There may be some distros that are shipping the open sourced version (ksh93v-). But AFAICT all of the mainstream distros stayed with the ksh93u+ release. Which is not what was open sourced. And the transition from ksh93u+ to ksh93v- is most definitely a major release. So I don't see how we can have a "few bug fix only releases of ksh" that are relevant to companies like Red Hat without completely rolling back all changes made since the ksh93u+ release that are not bug fixes relevant to that release.

[krader1961]

@siteshwar, Please feel free to tell me my previous comment is wrong, ill-informed, or otherwise crap. I do agree with @kdudka that we need to produce a stable release ASAP. It seems to me like the only way to address the concerns of @kdudka on behalf of Red Hat is to ignore every change other than bug fixes since the ksh93u+ release. But that will not be acceptable to users who now rely on the new features.

[siteshwar]

I am not sure what do you mean when you say that ksh93u+ was not open source. I presume you mean it was not made available on GitHub. The source code for ksh93u+ was still available and patches were shared over mailing list etc. It has been open source since almost 2 decades.

I quickly went through release log for ksh93v- and with a few exceptions I can see most of it is only about bug fixes. I think we should try to stablize ksh93v- and do a release based on it. I believe what @kdudka meant was we should keep any user facing changes to minimal. Ofcourse there will be some user facing changes due to build system change, ksh93v- release etc, but we should avoid making any delibrate behavior change (like removing any obsolete features).

[kdudka]

On Monday, January 22, 2018 6:35:59 AM CET Siteshwar Vashisht wrote: I am not sure what do you mean when you say that `ksh93u+` was not open source. I presume you mean it was not made available on GitHub. The source code for `ksh93u+` was still available and patches were shared over mailing list etc. It has been open source since almost 2 decades.

To be honest, I am not familiar with ksh versioning and do not know where it is documented. Still I am pretty sure that the version of ksh we have been distributing in our packages is fully open source.

I quickly went through release log for `ksh93v-` and with a few exceptions I can only see most of it is only about bug fixes. I think we should try to stablize `ksh93v-` and do a release based on it. I believe what @kdudka meant was we should keep any user facing changes to minimal. Ofcourse there will be some user facing changes due to build system change, `ksh93v-` release etc, but we should avoid making any delibrate behavior change (like removing any obsolete features).

Exactly (assuming ksh93v- is the publicly available version you started with).

[krader1961]

Yeah, sorry for my confusing use of terminology. By "open source" in this context I meant that the official ksh source was no longer under the sole control of AT&T. Yes, the source has been available for distros to fork and package as they wished, without having to obtain a license from AT&T, for a long time. Whether you could legally call it the Korn shell if you did anything other than fix a bug is a question for a lawyer but I'd bet the answer is "no".

I quickly went through release log for ksh93v- and with a few exceptions I can see most of it is only about bug fixes.

One big, potentially backward incompatible, change I see is that some, not all, of the commands in src/cmd/builtin are now unconditional builtins (ksh built from the legacy beta branch):

$ darwin.i386-64/bin/ksh
ksh> type stty
stty is a shell builtin version of /bin/stty
ksh> type cp
cp is a shell builtin version of /bin/cp
ksh> type pty
darwin.i386-64/bin/ksh: whence: pty: not found

This is not currently true for the Meson built ksh since we haven't setup the build rules for that directory. Note that a few of the pty related unit tests had to be disabled because that depend on ksh behavior that only works if stty is a builtin. For example, changing the word-erase and kill-line characters. I can't find any mention of that change in src/cmd/ksh93/RELEASE.

we should keep any user facing changes to minimal

I mostly agree with this. And this comment from the 93v release notes is why I haven't done anything about removing the bash emulation:

14-06-02 +When compiled with the SHOPT_BASH and run with the name bash, the shell now uses dynamic scoping for name() and function name. In addition the builtins declare and local are supported. The SHOPT_BASH option is on by default in the Makefile. More work remains with the bash compatibility option.

This is another significant user noticeable, backward incompatible, change.

Those features (e.g., adding the local keyword) are worth keeping and removing them is too likely to be noticed by anyone using ksh93v rather than ksh93u (which is what is on all my systems). And yet we definitely do not want to support trying to be 100% bash compatible when the ksh binary is invoked as bash. We need to disentangle those two aspects of that change.

[KeithBierman]

Interesting. MacOS X 10.13.2 aka uname -a Darwin kdesk1-3.local 17.3.0 Darwin Kernel Version 17.3.0: Thu Nov 9 18:09:22 PST 2017; root:xnu-4570.31.3~1/RELEASE_X86_64 x86_64 echo $KSH_VERSION Version AJM 93u+ 2012-08-01 $ type echo echo is a shell builtin $ type cp cp is a tracked alias for /bin/cp $ type stty stty is a tracked alias for /bin/stty $ and the same for CentOS 7.

[krader1961]

Also, clarification of my previous comment. The modules in src/cmd/builtin are not ksh builtins. They are standalone commands. The actual builtins we're not currently incorporating into the ksh binary are in src/lib/libcmd.

[krader1961]

Something is also decidedly weird with the ksh93v build. Most, but not all, of the commands in src/lib/libcmd are builtins. And there is at least one surprising command included: ls. For example:

$ type chmod
chmod is a shell builtin version of /bin/chmod
$ type chown
chown is a tracked alias for /usr/sbin/chown
$ type ls
ls is a shell builtin version of /bin/ls

TBD is why they aren't all builtins.

[dannyweldon]

TBD is why they aren't all builtins.

Raised as #380