LB-64, Add TLS support #85

Shwetajain148 · 2017-02-14T10:26:57Z

No description provided.

mchaudhary · 2017-02-16T03:47:03Z

@Shwetajain148 next time if you want to change the formatting please do it in separate PR. It is very hard to review the code.

Shwetajain148 · 2017-02-16T13:40:43Z

@mchaudhary @mostlyjason This PR consists of two commits.
1- Fix formatting
2- Add TLS Support to the script to send logs securely

In the first commit, I have fixed the indentation. You can see the tab changes here: Fix Indentation

In second commit you can compare the actual file changes to support TLS. Please go through link here: Add TLS Support to the script to send logs securely

mostlyjason · 2017-02-16T15:31:31Z

Linux Script/configure-linux.sh

+        echo "DOWNLOADING CERTIFICATE"
+        mkdir -pv /etc/rsyslog.d/keys/ca.d
+        curl -O https://logdog.loggly.com/media/logs-01.loggly.com_sha12.crt
+        sudo cp -Prf logs-01.loggly.com_sha12.crt /etc/rsyslog.d/keys/ca.d/logs-01.loggly.com_sha12.crt


DOn't forget to clean up the local directory when done

@mostlyjason Done.

mostlyjason · 2017-02-16T15:34:11Z

Linux Script/configure-linux.sh

@@ -215,18 +219,23 @@ removeLogglyConf()
  case "$LINUX_DIST_IN_LOWER_CASE" in
  *"ubuntu"* )
  echo "INFO: Operating system is Ubuntu."
+  PKG_MGR="apt-get"


What are these for?

@mostlyjason : In the case of Rsyslog version 7, we need to install rsyslog-gnutls package. So I have initialised the package manager variable for each Linux distribution. Please refer: Line-554

mostlyjason · 2017-02-16T15:34:11Z

Linux Script/configure-linux.sh

          #          -------------------------------------------------------
        # Define the template used for sending logs to Loggly. Do not change this format.
-          \$template LogglyFormat,\"<%pri%>%protocol-version% %timestamp:::date-rfc3339% %HOSTNAME% %app-name% %procid% %msgid% [$2@$3] %msg%\n\"
+          \$template LogglyFormat,\"<%pri%>%protocol-version% %timestamp:::date-rfc3339% %HOSTNAME% %app-name% %procid% %msgid% [$LOGGLY_AUTH_TOKEN@$LOGGLY_DISTRIBUTION_ID tag=\\\"Rsyslog\\\"] %msg%\n\"


Let's use the tag from the tls documentation

This tag is in the non-tls configuration. I have used the same tag "RsyslogTLS" for the TLS configurations as mentioned in the tls doc. See Tls configurations here: 7-Conf 8+Conf

mostlyjason · 2017-02-16T18:51:22Z

Linux Script/configure-linux.sh

-	   exit 1
-	fi
+  #This script needs to be run as root
+  if [[ $EUID -ne 0 ]]; then


What was wrong with the formatting of the indents before? I noticed that you are changing it but is there any particular reason?

@mostlyjason Formatting is done by IntelliJ IDE automatically. If you want then I can revert it back to the previous indentation and will only raise PR with actual file changes.

Shwetajain148 · 2017-02-20T13:23:24Z

Linux Script/configure-linux.sh

+        )
+        # Send messages to Loggly over TCP using the template.
+        action(type=\"omfwd\" protocol=\"tcp\" target=\"$LOGS_01_HOST\" port=\"$LOGGLY_SYSLOG_TLS_PORT\" template=\"LogglyFormat\" StreamDriver=\"gtls\" StreamDriverMode=\"1\" StreamDriverAuthMode=\"x509/name\" StreamDriverPermittedPeers=\"*.loggly.com\")
+        "


These are the TLS configurations for Rsyslog version 7- and 8+. If user sets up for Non-TLS logging then inputStr_NO_TLS will be installed.

Shwetajain148 · 2017-02-20T13:25:02Z

Linux Script/configure-linux.sh

+            SUPPRESS_PROMPT="true"
+            ;;
+            --insecure )
+            LOGGLY_TLS_SENDING="false"


Command line parameter to setup non-tls sending.

psquickitprageet · 2017-02-21T15:21:55Z

Linux Script/configure-linux.sh

+        logMsgToConfigSysLog "INFO" "INFO: Initiating Configure Loggly for Linux."
+
+        if [ "$LINUX_ENV_VALIDATED" = "false" ]; then
+        checkLinuxLogglyCompatibility


@Shwetajain148: This statement should appear inside the IF block. Make the same change at all the places where this kind of indentation problem is present.

…of Non-TLS

Shwetajain148 · 2017-03-02T12:48:24Z

@mchaudhary @mostlyjason Please review the script.

Added the TLS Support.
Implemented the port accessibility for port 6514 by default
If the user adds the --insecure in his command then the accessibility of port 514 will be checked

mostlyjason · 2017-03-08T18:17:56Z

Linux Script/configure-linux.sh

+	curl -O https://logdog.loggly.com/media/logs-01.loggly.com_sha12.crt
+	sudo cp -Prf logs-01.loggly.com_sha12.crt /etc/rsyslog.d/keys/ca.d/logs-01.loggly.com_sha12.crt
+	sudo rm logs-01.loggly.com_sha12.crt
+}


Can we log an error message if the certificate cannot be downloaded?

@Shwetajain148 sorry I had this review in pending for several days and forgot to submit it

mostlyjason · 2017-03-08T18:17:56Z

Linux Script/configure-linux.sh

-"
+	"
+if [ "$RSYSLOG_VERSION_TMP" -le "7" ]; then
+				/bin/bash -c "sudo $PKG_MGR install rsyslog-gnutls -y"


Can we log an error message if the package installation fails?

@mostlyjason Added the error message. Please review.

Will this also pass if the package has been previously installed and they are running the script again?

Yes that should work. I will let @Shwetajain148 comment and then merge this tomorrow morning.

@mchaudhary @mostlyjason Yes. In the case of already installed package, when script will try to install it, will get the following output:

"rsyslog-gnutls is already the newest version.
0 upgraded, 0 newly installed, 0 to remove and 47 not upgraded."

And then the script will go for the next steps.

…tls package do not install

Shwetajain148 · 2017-03-09T13:53:23Z

Linux Script/configure-linux.sh

+				then
+				logMsgToConfigSysLog "ERROR" "ERROR: The rsyslog-gnutls package was not downloaded. Please download it and then run the script again."
+				exit 1
+				fi


@mostlyjason I could observe that TLS logging requires the rsyslog-gnutls package in case of rsyslog version 8 as well. Otherwise, logging didn't work.
Also, I have added the error message in case package doesn't install successfully.

Okay please test on all the various distributions as well

Shwetajain148 · 2017-03-09T13:55:08Z

Linux Script/configure-linux.sh

+	logMsgToConfigSysLog "ERROR" "ERROR: Certificate could not be downloaded."
+	exit 1
+	fi
+}


@mostlyjason I have added the error message in the case when the certificate could not be found.

mostlyjason · 2017-03-10T20:32:38Z

@mchaudhary I believe she has addressed all my comments so please merge whenever you're ready

Shwetajain148 closed this Feb 14, 2017

Shwetajain148 deleted the Shwetajain148:Add-TLS-Support branch Feb 14, 2017

Shwetajain148 restored the Shwetajain148:Add-TLS-Support branch Feb 14, 2017

Shwetajain148 reopened this Feb 14, 2017

Shwetajain148 changed the title ~~Add tls support~~ LB-64, Add TLS support Feb 14, 2017

Shwetajain148 force-pushed the Shwetajain148:Add-TLS-Support branch from 6704dd1 to 81b1606 Feb 16, 2017

mostlyjason reviewed Feb 16, 2017

View changes

mostlyjason suggested changes Feb 16, 2017

View changes

mostlyjason reviewed Feb 16, 2017

View changes

Shwetajain148 reviewed Feb 20, 2017

View changes

Linux Script/configure-linux.sh Outdated

SUPPRESS_PROMPT="true"

;;

--insecure )

LOGGLY_TLS_SENDING="false"

This comment has been minimized.

Sign in to view

Shwetajain148 Feb 20, 2017
Author Contributor

Command line parameter to setup non-tls sending.

psquickitprageet reviewed Feb 21, 2017

View changes

Shwetajain148 force-pushed the Shwetajain148:Add-TLS-Support branch 2 times, most recently from 674792d to b87a32d Feb 23, 2017

Check accessibility of port 6514 in case of TLS and port 514 in case …

b2c917d

…of Non-TLS

Shwetajain148 force-pushed the Shwetajain148:Add-TLS-Support branch from b87a32d to b2c917d Mar 2, 2017

mostlyjason suggested changes Mar 8, 2017

View changes

Add Error messages for the condition when certificate and rsyslog-gnu…

9c1d1cd

…tls package do not install

Shwetajain148 reviewed Mar 9, 2017

View changes

Shwetajain148 closed this Apr 18, 2017

loggly / install-script

LB-64, Add TLS support #85

LB-64, Add TLS support #85

Shwetajain148 commented Feb 14, 2017

mchaudhary commented Feb 16, 2017

Shwetajain148 commented Feb 16, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Shwetajain148 commented Mar 2, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mostlyjason commented Mar 10, 2017

loggly / install-script

Join GitHub today

GitHub is where the world builds software

LB-64, Add TLS support #85

LB-64, Add TLS support #85

Conversation

Shwetajain148 commented Feb 14, 2017

mchaudhary commented Feb 16, 2017

Shwetajain148 commented Feb 16, 2017

This comment has been minimized.

mostlyjason Feb 16, 2017 Contributor

This comment has been minimized.

Shwetajain148 Feb 20, 2017 Author Contributor

This comment has been minimized.

mostlyjason Feb 16, 2017 Contributor

This comment has been minimized.

Shwetajain148 Feb 20, 2017 Author Contributor

This comment has been minimized.

mostlyjason Feb 21, 2017 Contributor

This comment has been minimized.

mostlyjason Feb 16, 2017 Contributor

This comment has been minimized.

Shwetajain148 Feb 20, 2017 Author Contributor

This comment has been minimized.

mostlyjason Feb 16, 2017 Contributor

This comment has been minimized.

Shwetajain148 Feb 20, 2017 Author Contributor

This comment has been minimized.

Shwetajain148 Feb 20, 2017 Author Contributor

This comment has been minimized.

Shwetajain148 Feb 20, 2017 Author Contributor

This comment has been minimized.

psquickitprageet Feb 21, 2017

Shwetajain148 commented Mar 2, 2017

This comment has been minimized.

mostlyjason Mar 8, 2017 Contributor

This comment has been minimized.

mostlyjason Mar 8, 2017 Contributor

This comment has been minimized.

mostlyjason Mar 8, 2017 Contributor

This comment has been minimized.

Shwetajain148 Mar 9, 2017 Author Contributor

This comment has been minimized.

mostlyjason Mar 9, 2017 Contributor

This comment has been minimized.

mchaudhary Mar 9, 2017 Contributor

This comment has been minimized.

Shwetajain148 Mar 10, 2017 • edited Author Contributor

This comment has been minimized.

mostlyjason Mar 10, 2017 Contributor

This comment has been minimized.

Shwetajain148 Mar 9, 2017 Author Contributor

This comment has been minimized.

mostlyjason Mar 9, 2017 Contributor

This comment has been minimized.

Shwetajain148 Mar 9, 2017 Author Contributor

mostlyjason commented Mar 10, 2017

Essential cookies

Always active

Analytics cookies

mostlyjason Feb 16, 2017
Contributor

Shwetajain148 Feb 20, 2017
Author Contributor

mostlyjason Feb 16, 2017
Contributor

Shwetajain148 Feb 20, 2017
Author Contributor

mostlyjason Feb 21, 2017
Contributor

mostlyjason Feb 16, 2017
Contributor

Shwetajain148 Feb 20, 2017
Author Contributor

mostlyjason Feb 16, 2017
Contributor

Shwetajain148 Feb 20, 2017
Author Contributor

Shwetajain148 Feb 20, 2017
Author Contributor

Shwetajain148 Feb 20, 2017
Author Contributor

mostlyjason Mar 8, 2017
Contributor

mostlyjason Mar 8, 2017
Contributor

mostlyjason Mar 8, 2017
Contributor

Shwetajain148 Mar 9, 2017
Author Contributor

mostlyjason Mar 9, 2017
Contributor

mchaudhary Mar 9, 2017
Contributor

Shwetajain148 Mar 10, 2017 •

edited

Author Contributor

mostlyjason Mar 10, 2017
Contributor

Shwetajain148 Mar 9, 2017
Author Contributor

mostlyjason Mar 9, 2017
Contributor

Shwetajain148 Mar 9, 2017
Author Contributor