Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added required libs and files for curl with HTTPs by default #1267

Merged
merged 1 commit into from Mar 28, 2017

Conversation

didacog
Copy link
Contributor

@didacog didacog commented Mar 28, 2017

This PR will add support for curl (HTTPS)
brief description of changes:

Added required LIBS and files to /usr/share/rear/conf/GNU/Linux.conf and remove them from /usr/share/rear/init/default/010_set_drlm_env.sh

This code has been tested on RHEL/CentOS, SLES/OpenSUSE and Debian/Ubuntu.

@jsmeix jsmeix requested review from gdha and jsmeix March 28, 2017 11:53
@jsmeix jsmeix added cleanup enhancement Adaptions and new features labels Mar 28, 2017
@jsmeix jsmeix added this to the ReaR v2.1 milestone Mar 28, 2017
Copy link
Member

@jsmeix jsmeix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I like it when the recovery system provides
by default support for "usually useful functionality".
Perhaps - if @gdha wants it - a config variable might be added
so that users who really want a minimal recovery system
could exclude things they do not really need in their
particular environment.
There is already COPY_AS_IS_EXCLUDE
but there is no LIBS_EXCLUDE.

@jsmeix jsmeix self-assigned this Mar 28, 2017
@didacog
Copy link
Contributor Author

didacog commented Mar 28, 2017

Hi @jsmeix,

Any suggestion will be appreciated, I just added the libs and files because now curl is added by default and just in order to work over HTTP and HTTPs without issues.

Regards,

@jsmeix
Copy link
Member

jsmeix commented Mar 28, 2017

I think it would be rather unexpected nowadays
when curl works via HTTP but not via HTTPS
so that I think this pull request should be merged "as is".

Later - preferably only when a user can show some evidence
that nowadays the recovery system is too big - we could add
support for things like LIBS_EXCLUDE / PROGS_EXCLUDE

FYI:
Since I impemented FIRMWARE_FILES support
the user can make his recovery system really noticeable
smaller compared to what it was before,
cf. #1216

@didacog
Copy link
Contributor Author

didacog commented Mar 28, 2017

Yes, I've seen that issue ;-)

Thx!

Copy link
Member

@gdha gdha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perfect @didacog thx

@jsmeix
Copy link
Member

jsmeix commented Mar 28, 2017

This is a follow up for #1248

@jsmeix
Copy link
Member

jsmeix commented Apr 5, 2017

@didacog
in
#1278 (comment)
I am thinking about whether or not the
current default setting in conf/GNU/Linux.conf

COPY_AS_IS=( ... /etc/ssl/certs/* /etc/pki/* )

could lead to security issues.

Reason:
As far as I understand it - in /etc/ssl/certs or /etc/pki
also self-generated private stuff could be stored
which would then by default get copied into the
ReaR recovery system.

But by default the ReaR recovery system should not contain
confidential or private stuff (as far as I remember what
@schlomo wrote a longer time ago) unless the user
had explicitly configured something.

@didacog
Copy link
Contributor Author

didacog commented Apr 5, 2017

@jsmeix

This is required by curl with https, there are stored de distribution provided certificates installed from packages, nothing confidential. Usually the public verified certs, and not private keys as far as I know.
For example the private keys are stored in /etc/ssl/private (not copied)
In /etc/pki maybe /etc/pki/tls/private should be excluded.

@gdha
Copy link
Member

gdha commented Apr 5, 2017

@didacog Good catch - try to exclude all private keys.

@didacog
Copy link
Contributor Author

didacog commented Apr 5, 2017

@gdha @jsmeix I will try to adjust it and send a new pull request excluding private keys location from /etc/pki.

@didacog
Copy link
Contributor Author

didacog commented Apr 5, 2017

I guess this should work:
In usr/share/rear/conf/GNU/Linux.conf:

COPY_AS_IS_EXCLUDE=( ${COPY_AS_IS_EXCLUDE[@]:-} dev/shm/\* /etc/pki/private/* )

Let me test and check if more stuff should be excluded before PR.

@jsmeix
Copy link
Member

jsmeix commented Apr 5, 2017

@didacog
when you work on it could you add some explanatory
comments directly in the code that tell about why
stuff is included and other stuff is excluded so that
others can even a longer time later still understand
the reasons behind, cf.
https://github.com/rear/rear/wiki/Coding-Style

To get comments at the right place inside longer bash arrays
I would suggest to do it somehow like this:

COPY_AS_IS=( ${COPY_AS_IS[@]:-} /dev /etc/inputr[c] /etc/protocols /etc/services /etc/rpc /etc/termcap /etc/terminfo /lib*/terminfo /usr/share/terminfo /etc/netconfig /etc/mke2
fs.conf /etc/*-release /etc/localtime /etc/magic /usr/share/misc/magic /etc/dracut.conf /etc/dracut.conf.d /usr/lib/dracut /sbin/modprobe.ksplice-orig /etc/sysctl.conf /etc/sys
ctl.d /etc/e2fsck.conf )
# Required by curl with https:
# There are stored the distribution provided certificates
# installed from packages, nothing confidential.
# Usually the public verified certs, and not private keys.
# The private keys are stored in /etc/ssl/private (not copied)
# In /etc/pki maybe /etc/pki/tls/private is excluded (see below).
COPY_AS_IS=( "${COPY_AS_IS[@]}" '/etc/ssl/certs/*' '/etc/pki/*' )
# exclude /dev/shm/*, due to the way we use tar the leading / should be omitted
COPY_AS_IS_EXCLUDE=( ${COPY_AS_IS_EXCLUDE[@]:-} dev/shm/\* )
# Exclude /etc/pki/tls/private (cf. above):
COPY_AS_IS_EXCLUDE=( "${COPY_AS_IS_EXCLUDE[@]}" '/etc/pki/private/*' )

Note that using ${VAR[@]} without double-quotes is problematic
so that one should use "${VAR[@]}" see 'Arrays' in "man bash"
and see #1068

Furthermore I wonder if bash globbing must be
avoided for values like /path/to/something/*
by using single quotes '/path/to/something/*'
or if bash globbing is intended by not using single quotes?

didacog added a commit to didacog/rear that referenced this pull request Apr 5, 2017
@didacog
Copy link
Contributor Author

didacog commented Apr 5, 2017

@jsmeix @gdha

I've sent a PR #1279 with the changes tested.

Finally, to exclude all priivate keys:

/etc/pki/tls/private 
/etc/pki/CA/private
/etc/pki/nssdb/key*.db

Regards,

@didacog didacog deleted the curl_https_support branch April 5, 2017 14:29
jsmeix added a commit that referenced this pull request Apr 5, 2017
Exclude possibly private keys in /etc/pki/tls/private
from being copied into the ReaR rescue/recovery system.
This is a follow up of
#1267
@jsmeix
Copy link
Member

jsmeix commented Apr 5, 2017

I "just merged" #1279
to get that possible security issue solved as fast as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants