Page MenuHomeFreeBSD
Differential D49742

openssh: Fix snprintf const char *fmt detection
AcceptedPublic
Actions

Authored by jlduran on Wed, Apr 9, 6:56 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 22, 8:45 PM
Unknown Object (File)
Thu, Apr 17, 2:47 AM
Unknown Object (File)
Tue, Apr 15, 4:09 AM
Unknown Object (File)
Tue, Apr 15, 4:09 AM
Unknown Object (File)
Tue, Apr 15, 4:09 AM
Unknown Object (File)
Tue, Apr 15, 4:09 AM
Unknown Object (File)
Tue, Apr 15, 4:09 AM
Unknown Object (File)
Mon, Apr 14, 2:31 AM
View All 13 Files
Subscribers
None

Details

Reviewers
emaste
Summary

The script in configure.ac that detects if the fmt argument is const char * or just char * fails to detect it when compiled with Clang and _FORTIFY_SOURCE=2.
As a temporary workaround, run the configure script --without-hardening to correctly detect it.
Note that this does not disable any source fortification when building OpenSSH, it is done just so it generates a valid conf.h header (#define SNPRINTF_CONST const).
An upstream problem report has been filed reporting this issue.

Upstream PR: https://bugzilla.mindrot.org/show_bug.cgi?id=3809

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

jlduran requested review of this revision.Wed, Apr 9, 6:56 PM
jlduran created this revision.
jlduran mentioned this in D49739: OpenSSH: retire DSA support.Wed, Apr 9, 6:59 PM
emaste added a comment.Wed, Apr 9, 8:03 PM

I think this is fine, but we should have a comment explaining that this is working around a bug* and does not in fact disable hardening options in the FreeBSD build.

  • we should have a FreeBSD bug report or upstream report for this issue and ideally reference it in the comment
jlduran added a comment.Thu, Apr 10, 7:08 PM
In D49742#1134186, @emaste wrote:

I think this is fine, but we should have a comment explaining that this is working around a bug* and does not in fact disable hardening options in the FreeBSD build.

  • we should have a FreeBSD bug report or upstream report for this issue and ideally reference it in the comment

I agree.
Upstream meaning OpenSSH or Clang? While OpenSSH can change the detection script, the issue is still there. Should I open an internal bug report as well:

$ gcc -D_FORTIFY_SOURCE=2 conftest.c

Works as intended, with Clang it fails also on our source fortification upstream, NetBSD.

emaste added a comment.Fri, Apr 11, 1:49 AM

Upstream meaning OpenSSH or Clang?

I meant OpenSSH - i.e. is the problem in our headers or implementation, or is OpenSSH doing something strange? On one hand it does actually work with const so it seems to be the test is at fault.

jlduran added a comment.Fri, Apr 11, 11:38 AM
In D49742#1134602, @emaste wrote:

Upstream meaning OpenSSH or Clang?

I meant OpenSSH - i.e. is the problem in our headers or implementation, or is OpenSSH doing something strange? On one hand it does actually work with const so it seems to be the test is at fault.

Reported here: https://bugzilla.mindrot.org/show_bug.cgi?id=3809

I'll update the commit message with your suggestions shortly. Thank you!

jlduran retitled this revision from openssh: Disable toolchain hardening flags to openssh: Fix snprintf const chat *fmt detection.Fri, Apr 11, 11:56 AM
jlduran edited the summary of this revision. (Show Details)
jlduran retitled this revision from openssh: Fix snprintf const chat *fmt detection to openssh: Fix snprintf const char *fmt detection.
emaste accepted this revision.Fri, Apr 11, 12:45 PM
This revision is now accepted and ready to land.Fri, Apr 11, 12:45 PM
jlduran added a comment.Thu, Apr 17, 7:22 PM

I'll wait a few days for this one, upstream is already taking a look at the problem report.

emaste added a comment.Thu, Apr 17, 8:27 PM

👍

Revision Contents
Changeset List

PathSize
crypto/
openssh/
1 line

Diff 153414

View Options

crypto/openssh/freebsd-configure.sh

Loading...