Hacker News Reader

Comment by jamilbk | Hacker News Reader

From the diff introducing the bug [1], the issue according to the analysis is that the function was refactored from this:

  void
  sigdie(const char *fmt,...)
  {
  #ifdef DO_LOG_SAFE_IN_SIGHAND
   va_list args;
  
   va_start(args, fmt);
   do_log(SYSLOG_LEVEL_FATAL, fmt, args);
   va_end(args);
  #endif
   _exit(1);
  }

to this:

  void
  sshsigdie(const char *file, const char *func, int line, const char *fmt, ...)
  {
   va_list args;
  
   va_start(args, fmt);
   sshlogv(file, func, line, 0, SYSLOG_LEVEL_FATAL, fmt, args);
   va_end(args);
   _exit(1);
  }

which lacks the #ifdef.

What could have prevented this? More eyes on the pull request? It's wild that software nearly the entire world relies on for secure access is maintained by seemingly just two people [2].

[1] https://github.com/openssh/openssh-portable/commit/752250caa...

[2] https://github.com/openssh/openssh-portable/graphs/contribut...

View on HN

It's always easy with hindsight to tell how to prevent something. In this case, a comment might have helped why the #ifdef was needed, eg

  void CloseAllFromTheHardWay(int firstfd) //Code here must be async-signal-safe! Locks may be in indeterminate state
  {
    struct rlimit lim;
    getrlimit(RLIMIT_NOFILE,&lim);

    for (int fd=(lim.rlim_cur == RLIM_INFINITY ? 1024 : lim.rlim_cur);fd>=firstfd;--fd)
      close(fd);
  }

Although to be honest, getrlimit isn't actually on the list here: https://man7.org/linux/man-pages/man7/signal-safety.7.html

But I hope that removing the comment or modifying code with a comment about async-signal-safe might have been noticed in review. The code you quoted only has the mention SAFE_IN_SIGHAND to suggest that this code might need to be async-signal-safe

The ifdef name was a big clue! "SIGHAND" is short for signal handler. Sure, there is an implicit connection here from "signal handler" to "all code must be async signal safe," but that association is pretty well known to most openssh authors and code reviewers. Oh well, mistakes happen.

OpenBSD refactored their system to use async signal safe re-entrent syslog functions, so it’s possible that the author of this code simply assumed that it was safe to make this change, forgetting (or completely unaware) that other platforms (which the openBSD ssh devs don’t actually claim to support) were still using async unsafe functions.

> What could have prevented this? More eyes on the pull request? It's wild that software nearly the entire world relies on for secure access is maintained by seemingly just two people [2].

It's open source. If you feel you could do a better job, then by all means, go ahead and fork it.

You're not entitled to anything from open source developers. They're allowed to make mistakes, and they're allowed to have as many or as few maintainers/reviewers as they wish.

https://gist.github.com/richhickey/1563cddea1002958f96e7ba95...

Strangely aggressive and unproductive response. GP has valid points. They are not acting entitled in any way.

I disagree. An obvious interpretation of what is quoted is entitlement.

How would you say someone should express such concerns without coming off as entitled? Or do you think they're not valid concerns? Or do you really think that if someone has such concerns, their only recourse is to start contributing to the project? That project of course being one of the most security-sensitive projects one could imagine.

> How would you say someone should express such concerns without coming off as entitled?

> Or do you really think that if someone has such concerns, their only recourse is to start contributing to the project?

Yes, I think one way to not come off as entitled when being critical to volunteers is to also offer volunteer work yourself.

And it's most helpful to provide feedback directly to the developers through their preferred means of communication.

> Or do you think they're not valid concerns?

Irrelevant what I think here, that's kind of the point. That's just my opinion.

> That project of course being one of the most security-sensitive projects one could imagine.

Agreed that the project is important. However, this is irrelevant, too, unless you're bolstering your "valid concerns" argument.

> Yes, I think one way to not come off as entitled when being critical to volunteers is to also offer volunteer work yourself.

So what level of contribution is the bar here? I mean, what's the commit count? Do I have to be developing core features for years? Does writing docs count? Do I have to volunteer for a particular project before I can in any way criticize it, or is just any open source work okay?

> And it's most helpful to provide feedback directly to the developers through their preferred means of communication.

This is not feedback meant directly for the developer - it's valid questions that were meant to spark a discussion here on HN. Of course, with users like you around, that's difficult.

> However, this is irrelevant, too, unless you're bolstering your "valid concerns" argument.

It is relevant, because it's absurd to think that just any developer can just go and contribute to such a project.

I'm not trying to come off as combative, it seems that you are.

All I offered was a way to not sound entitled. Personally, I certainly hold the opinions of someone that's helping me much higher than the opinion of someone that isn't.

Another approach to avoid sounding entitled could be to post a more thoughtful and comprehensive analysis on HN or a blog, rather than nitpicking a commit and posting broad questions like "what could have prevented this?" and insinuating that the volunteers need to do better.

Finally, if it's true that not "just any developer" can contribute to OpenSSH... well it's open-source. Fork it. Or build your own.

You come off as combative when you dismiss valid, innocent questions as being "entitled" or attributing insinuations to GP that simply aren't there.

> Finally, if it's true that not "just any developer" can contribute to OpenSSH... well it's open-source. Fork it. Or build your own.

What good would that do? Would that enable the forker to voice their complaints about the original OpenSSH that's used by literally everyone else without people like you chiming in?

By the by, is it at all relevant that OpenSSH development is funded by at least 1 non-profit and probably other sources as well? They're not volunteers.

(And even if they were volunteers, users are quite within their rights to voice concerns and criticisms about software in a constructive manner. If open source developers don't want to face that, they can not develop open source software.)

> You come off as combative when you dismiss valid, innocent questions as being "entitled" or attributing insinuations to GP that simply aren't there.

We've been over this. I disagree that there isn't insinuations or isn't entitlement in GP. It's okay to disagree.

> What good would that do? Would that enable the forker to voice their complaints about the original OpenSSH that's used by literally everyone else without people like you chiming in?

This can do a lot of good. This a solution to the problem that you have. If others agree with your critique and approach (which is likely), then they also will appreciate your project. This is how projects like Neovim started, and arguably why Neovim has been as successful as it is.

> By the by, is it at all relevant that OpenSSH development is funded by at least 1 non-profit and probably other sources as well? They're not volunteers.

I was under the impression that it was largely volunteer work, or at least, severely underpaid development which is pretty normal in the open source world. I will take your word on this one, I don't have the time to go look at non-profit financials.

> And even if they were volunteers, users are quite within their rights to voice concerns and criticisms about software in a constructive manner.

100% agree, the keywords being "constructive manner." Higher effort than nitpicking a commit and asking broad questions.

For what it's worth, I think you're probably doing more harm than good to the open source movement. Reconsider your approach, or focus it on the users who are actually acting entitled.

> All I offered was a way to not sound entitled

And I must've missed this bit in your original few comments.

> It's wild that software nearly the entire world relies on for secure access is maintained by seemingly just two people

obligatory xkcd https://xkcd.com/2347/

One of these:

1. Using a proper programming language that doesn't allow you to setup arbitrary functions as signal handlers (since that's obviously unsafe on common libcs...) - e.g. you can't do that in safe Rust, or Java, etc.

2. Using a well-implemented libc that doesn't cause memory corruption when calling async-signal-unsafe functions but only deadlocks (this is very easy to achieve by treating code running in signals as a separate thread for thread-local storage access purposes), and preferably also doesn't deadlock (this requires no global mutexes, or the ability to resume interrupted code holding a mutex)

3. Thinking when changing and accepting code, not like the people who committed and accepted [1] which just arbitrarily removes an #ifdef with no justification

4. Using simple well-engineered software written by good programmers instead of OpenSSH

This comes across as quite scathing critique of an open source tool that has provided an extremely high standard of security and reliability over decades, despite being built on technologies that don’t offer the guardrails outlined in points (1) and (2).

Point (3) seems like a personal attack on the developers/reviewer, who made human errors. Humans do in fact make mistakes, and the best build toolchain/test suite in the world won’t save you 100% of the time.

Point (4) seems to imply that OpenSSH is not well-engineered, simple, or written by good programmers. While all of that is fairly subjective, it is (I feel) needlessly unkind.

I’d invite you to recommend an alternative remote access technology with an equivalent track record of security and stability in this space — I’m not aware of any.

This is all great thinking.

Except for those of us who live in a world where most of their OS and utilities and libraries were originally written decades before Rust existed, and often even before Java existed. And where "legacy" C code pretty much underpins everything running on the public internet and which you need to connect to.

There's a very real risk that reimplementing every piece of code on a modern internet connected server in exciting new "safe" languages and libc-type things - by a bunch of "modern" programmers who do not have the learning experience of decades worth of mistakes - will end up with not just new implementation of old and fixed bugs and security problems, but also with new implementations that are incompatible in strange and hard to debug ways with every existing piece of software that uses SSH protocols as they are deployed in the field.

I, for one, and not going to install and put into production version 1.0 of some new OpenSSH replacement written in Rust out Go or Java, which has a non zero chance of strange edge case bugs that are different when connecting to SSH on different Linux/BSD/Windows distros or versions, across different cpu architectures, and probably have subtly different bugs when connecting to different cloud hyperscalers.

Reply on news.ycombinator.com