389-ds-base

Clone
Source Code
GIT

#48350 configure.ac add options for debbuging and security analysis / hardening.
Closed: wontfix None Opened 9 years ago by firstyear.

Closed: wontfix
Reopen Issue

This patch introduces three changes.

First, --enabled-debug enables -g3. This is useful for debugging, as well as a requirement of the third change in this ticket.

Second, fedora and RPM build ship with a number of hardening options. It will be useful for us to include these to develop against so that we can test our system "close" to what rpm-build is doing for us. This will help us pick up issues earlier, and lower the bar to entry for people who want to build with these options.

Third, we add the option to enable gcc's address sanitization [0]. This is similar to valring in being a checker for memory usage, however it detects potential overflows and misuse of memory. For example, it detected the following error:

==30761== ERROR: AddressSanitizer: global-buffer-overflow on address 0x7f5ecba8c2bf at pc 0x7f5ed5604517 bp 0x7fff1bb938f0 sp 0x7fff1bb938e0
READ of size 1 at 0x7f5ecba8c2bf thread T0
    #0 0x7f5ed5604516 in slapi_ldap_url_parse /home/wibrown/development/389ds/ds/ldap/servers/slapd/ldaputil.c:342
....

This will help us as developers find and isolate potential security issues in our code.
This is not a flag for production use, as it has a significant performance impact, and upon detecting a memory error, will TERMINATE the slapd process.

I would like to thank dblack@atlassian.com, for putting me onto GCC's address sanitization, and for discussing secure development with me.

[0] https://github.com/google/sanitizers/tree/master/address-sanitizer

To test ASAN:

{{{
sudo yum install -y libasan llvm
cat >> ~/.bashrc << EOF
export ASAN_SYMBOLIZER_PATH=/usr/bin/llvm-symbolizer
export ASAN_OPTIONS=symbolize=1
EOF

<cd to ds source>
./configure --enable-debug --enable-asan

}}}

Note that setup-ds.pl may fail to complete: If this is the case, it's likely that ns-slapd crashed because of an asan error. You can do:

{{{
setup-ds.pl
<hang>
ctrl - c
ns-slapd -d -D /prefix/etc/dirsrv/slapd-inst/
}}}

This will show you the crash and why it occured.

https://fedorahosted.org/389/ticket/48351 is the kind of defect that ASAN will allow us to detect and correct.

To ssh://git.fedorahosted.org/git/389/ds.git
20ebe8d..284b167 master -> master
commit 52c30951439a6157c50cd4f68cdc968263c3ec2c

It was asked if we could integrate asan into some rpm outputs for QE. Re-opening to track.

The patch itself looks good to me. I have a question about this comment. QE is testing the official bits built on brew. Are you suggesting to build the official build with --enable-asan?
{{{
Fix Description: This improves our ASAN integration with our rpms and systemd.
This way, we can give ASAN builds to QE for testing, and we can produce
non-instrumented builds for production releases.

Note that if you run ./configure --enable-asan; make rpms, this will create
an ASAN instrumented RPM.
}}}

No. We should never make an official build with ASAN.

QE just was an "official-looking" build that they can use in tests with asan.

For production releases we should not offer this.

That's why there are lots of warnings in the code if you turn it on.

Thanks for the explanation, William. Acked.

commit c9e4921
Writing objects: 100% (13/13), 2.99 KiB | 0 bytes/s, done.
Total 13 (delta 10), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
c39bed2..c9e4921 master -> master

Hi William,

make srpm fails for me:
{{{
if test -d "389-ds-base-1.3.5.3"; then find "389-ds-base-1.3.5.3" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -rf "389-ds-base-1.3.5.3" || { sleep 5 && rm -rf "389-ds-base-1.3.5.3"; }; else :; fi
test -d "389-ds-base-1.3.5.3" || mkdir "389-ds-base-1.3.5.3"
test -n "" \
|| find "389-ds-base-1.3.5.3" -type d ! -perm -755 \
-exec chmod u+rwx,go+rx {} \; -o \
! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
! -type d ! -perm -400 -exec chmod a+r {} \; -o \
! -type d ! -perm -444 -exec /bin/sh /home/vashirov/src/ds/install-sh -c -m a+r {} {} \; \
|| chmod -R a+r "389-ds-base-1.3.5.3"
tardir=389-ds-base-1.3.5.3 && tar --format=posix -chf - "$tardir" | BZIP2=${BZIP2--9} bzip2 -c >389-ds-base-1.3.5.3.tar.bz2
if test -d "389-ds-base-1.3.5.3"; then find "389-ds-base-1.3.5.3" -type d ! -perm -200 -exec chmod u+w {} ';' && rm -rf "389-ds-base-1.3.5.3" || { sleep 5 && rm -rf "389-ds-base-1.3.5.3"; }; else :; fi
/usr/bin/mkdir -p /home/vashirov/src/ds/rpmbuild/BUILD
/usr/bin/mkdir -p /home/vashirov/src/ds/rpmbuild/RPMS
/usr/bin/mkdir -p /home/vashirov/src/ds/rpmbuild/SOURCES
/usr/bin/mkdir -p /home/vashirov/src/ds/rpmbuild/SPECS
/usr/bin/mkdir -p /home/vashirov/src/ds/rpmbuild/SRPMS
cp 389-ds-base-1.3.5.3.tar.bz2 /home/vashirov/src/ds/rpmbuild/SOURCES
cp ./rpm/389-ds-base-git.sh /home/vashirov/src/ds/rpmbuild/SOURCES
cp ./rpm/389-ds-base-devel.README /home/vashirov/src/ds/rpmbuild/SOURCES
sed -e "s/VERSION/1.3.5.3/" -e "s/RELEASE/20160517195647/" -e "s/NUNC_STANS_ON/0/" < /home/vashirov/src/ds/rpm/389-ds-base.spec > /home/vashirov/src/ds/rpmbuild/SPECS/389-ds-base.spec
cd /home/vashirov/src/ds/rpmbuild; \
rpmbuild --define "_topdir /home/vashirov/src/ds/rpmbuild" \
-bs SPECS/389-ds-base.spec
error: parse error in expression
error: /home/vashirov/src/ds/rpmbuild/SPECS/389-ds-base.spec:26: bad %if condition
Makefile:10835: recipe for target 'srpm' failed
make: *** [srpm] Error 1
}}}
in spec file:
{{{
24 # This enables an ASAN build. This should not go to production, so we rename.
25 %global use_asan ASAN_ON
26 %if %{use_asan}
27 %global variant base-asan
28 %endif
}}}

Makefile.am:
{{{
./Makefile.am: sed -e "s/VERSION/$(RPM_VERSION)/" -e "s/RELEASE/$(RPM_RELEASE)/" -e "s/NUNC_STANS_ON/$(NUNC_STANS_ON)/" -e "s/ASAN_ON/$(ASAN_ON)/" < $(abs_builddir)/rpm/389-ds-base.spec > $(RPMBUILD)/SPECS/389-ds-base.spec
}}}
but in Makefile.in:
{{{
./Makefile.in: sed -e "s/VERSION/$(RPM_VERSION)/" -e "s/RELEASE/$(RPM_RELEASE)/" -e "s/NUNC_STANS_ON/$(NUNC_STANS_ON)/" < $(abs_builddir)/rpm/389-ds-base.spec > $(RPMBUILD)/SPECS/389-ds-base.spec
}}}

Another issue is start-dirsv:
{{{
grep -n enable_asan /usr/sbin/start-dirsrv
63: if test 1 -eq @enable_asan@; then
}}}
which sometimes causes fail to create an instance:
{{{
New instance fsl was created
/usr/sbin/start-dirsrv: line 63: test: @enable_asan@: integer expression expected
Server failed to start !!! Please check errors log for problems
}}}

Ahhh yes, I saw this issue the other day. I will provide patch.

That doesn't work for me. There are discrepancies in Makefile.am and Makefile.in. Calling automake to regenerate Makefile.in helps, but if we maintain Makefile.in in tree, it should be updated too (and probably configure).

As far as I'm concerned, all our configure artifacts (configure, Makefile.in, Makefile etc) should not even be in the tree. The process should be:

git clone repo
autoreconf
./configure
make
make install

We have too many platform variations between us developers, we all use different options, and every time we commit we have to not check in these bits, and that leads to inconsistencies that you have to fix running the exact work flow here anyway.

I vote we remove these.

Anyway, given this worked for you, are you happy for me to commit it, with the autoreconf in mind?

Closing as it was just autoreconf that was needed.

Replying to [comment:20 firstyear]:

Anyway, given this worked for you, are you happy for me to commit it, with the autoreconf in mind?
Sorry, missed your question, since I wasn't in CC or needinfo. Yes, now it works for me.

Metadata Update from @firstyear:
- Issue assigned to firstyear
- Issue set to the milestone: 1.3.5.5
7 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/1681

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)
4 years ago

Log in to comment on this ticket.

Metadata
None
None
None
normal
review?
None
Community
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.