[GitHub] qpid-dispatch pull request #124: Allow passing the password via env variable

[dskarbek]

Github user dskarbek commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/124#discussion_r92076465
  
--- Diff: src/connection_manager.c ---
@@ -176,6 +176,46 @@ static void set_config_host(qd_server_config_t 
*config, qd_entity_t* entity)
 assert(config->host);
 }
 
+static char* qd_config_ssl_profile_get_password(qd_config_ssl_profile_t* 
ssl_profile)
+{
+char *pw = ssl_profile->ssl_password;
+if (!pw) return pw;
+
+/* if the "password" starts with "env:" or "env: " then the remaining
+ * text is the environment variable that contains the password
+ */
+if (strncmp(pw, "env:", 4) == 0) {
+char *env = pw + 4;
+/* skip the space if it is there */
+if (*env == ' ') ++env;
+
+const char* passwd = getenv(env);
+if (passwd) {
+free(ssl_profile->ssl_password);
+pw = ssl_profile->ssl_password = strdup(passwd);
+} else {
+qd_error(QD_ERROR_NOT_FOUND, "Failed to find a password in the 
environment variable '%s'", env);
--- End diff --

Oh, you mean for the case where that isn't an env variable, but actually is 
part of a password?  Hmm, that's true.  though, it seems like it would help you 
understand what is happening faster if we log it.  Is the log file less secure 
than the config file?  Cuz, in that case, they already have the password in the 
config file.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

[GitHub] qpid-dispatch pull request #124: Allow passing the password via env variable

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/qpid-dispatch/pull/124


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

[GitHub] qpid-dispatch pull request #124: Allow passing the password via env variable

[ted-ross]

Github user ted-ross commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/124#discussion_r92030610
  
--- Diff: src/connection_manager.c ---
@@ -176,6 +176,46 @@ static void set_config_host(qd_server_config_t 
*config, qd_entity_t* entity)
 assert(config->host);
 }
 
+static char* qd_config_ssl_profile_get_password(qd_config_ssl_profile_t* 
ssl_profile)
+{
+char *pw = ssl_profile->ssl_password;
+if (!pw) return pw;
+
+/* if the "password" starts with "env:" or "env: " then the remaining
+ * text is the environment variable that contains the password
+ */
+if (strncmp(pw, "env:", 4) == 0) {
+char *env = pw + 4;
+/* skip the space if it is there */
+if (*env == ' ') ++env;
+
+const char* passwd = getenv(env);
+if (passwd) {
+free(ssl_profile->ssl_password);
+pw = ssl_profile->ssl_password = strdup(passwd);
+} else {
+qd_error(QD_ERROR_NOT_FOUND, "Failed to find a password in the 
environment variable '%s'", env);
--- End diff --

This should definitely be logged.  It might be prudent to not include the 
password text in the log though.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

[GitHub] qpid-dispatch pull request #124: Allow passing the password via env variable

[dskarbek]

Github user dskarbek commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/124#discussion_r91815078
  
--- Diff: src/connection_manager.c ---
@@ -176,6 +176,46 @@ static void set_config_host(qd_server_config_t 
*config, qd_entity_t* entity)
 assert(config->host);
 }
 
+static char* qd_config_ssl_profile_get_password(qd_config_ssl_profile_t* 
ssl_profile)
+{
+char *pw = ssl_profile->ssl_password;
+if (!pw) return pw;
+
+/* if the "password" starts with "env:" or "env: " then the remaining
+ * text is the environment variable that contains the password
+ */
+if (strncmp(pw, "env:", 4) == 0) {
+char *env = pw + 4;
+/* skip the space if it is there */
+if (*env == ' ') ++env;
+
+const char* passwd = getenv(env);
+if (passwd) {
+free(ssl_profile->ssl_password);
+pw = ssl_profile->ssl_password = strdup(passwd);
+} else {
+qd_error(QD_ERROR_NOT_FOUND, "Failed to find a password in the 
environment variable '%s'", env);
+}
+return pw;
+}
+
+/* if the "password" starts with "literal:" or "literal: " then
+ * the remaining text is the password and the heading should be
+ * stripped off
+ */
+if (strncmp(pw, "literal:", 8) == 0) {
+/* skip the "literal:" header */
+pw += 8;
+/* skip the space if it is there */
+if (*pw == ' ') ++pw;
+/* return a pointer into the middle of the string where the 
literal password starts */
+return pw;
--- End diff --

Not a problem unless I'm wrong about the memory management of this pointer.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

[GitHub] qpid-dispatch pull request #124: Allow passing the password via env variable

[dskarbek]

Github user dskarbek commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/124#discussion_r91815006
  
--- Diff: src/connection_manager.c ---
@@ -176,6 +176,46 @@ static void set_config_host(qd_server_config_t 
*config, qd_entity_t* entity)
 assert(config->host);
 }
 
+static char* qd_config_ssl_profile_get_password(qd_config_ssl_profile_t* 
ssl_profile)
+{
+char *pw = ssl_profile->ssl_password;
+if (!pw) return pw;
+
+/* if the "password" starts with "env:" or "env: " then the remaining
+ * text is the environment variable that contains the password
+ */
+if (strncmp(pw, "env:", 4) == 0) {
+char *env = pw + 4;
+/* skip the space if it is there */
+if (*env == ' ') ++env;
+
+const char* passwd = getenv(env);
+if (passwd) {
+free(ssl_profile->ssl_password);
+pw = ssl_profile->ssl_password = strdup(passwd);
+} else {
+qd_error(QD_ERROR_NOT_FOUND, "Failed to find a password in the 
environment variable '%s'", env);
--- End diff --

or, would you rather just return the password string as is and not register 
an error in-case it is an actual password?  If it is referring to an env var 
that missed getting set there's next to zero chance it will work, so better to 
catch the error sooner at the source so we can log it appropriately, but this 
does mean that someone using "env:foobar" as a password will suddenly stop 
working.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

[GitHub] qpid-dispatch pull request #124: Allow passing the password via env variable

[dskarbek]

Github user dskarbek commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/124#discussion_r91814838
  
--- Diff: src/connection_manager.c ---
@@ -176,6 +176,46 @@ static void set_config_host(qd_server_config_t 
*config, qd_entity_t* entity)
 assert(config->host);
 }
 
+static char* qd_config_ssl_profile_get_password(qd_config_ssl_profile_t* 
ssl_profile)
+{
+char *pw = ssl_profile->ssl_password;
+if (!pw) return pw;
+
+/* if the "password" starts with "env:" or "env: " then the remaining
+ * text is the environment variable that contains the password
+ */
+if (strncmp(pw, "env:", 4) == 0) {
+char *env = pw + 4;
+/* skip the space if it is there */
+if (*env == ' ') ++env;
+
+const char* passwd = getenv(env);
+if (passwd) {
+free(ssl_profile->ssl_password);
+pw = ssl_profile->ssl_password = strdup(passwd);
--- End diff --

Hmm, or, if I'm correct that the config->ssl_password is never directly 
free'd, then I could just pass back the static getenv() returned string as the 
value (except I'd have to cast away const).  Then I don't have to change the 
string held in ssl_profile.  Thoughts?  Is my memory management correct here 
(and below) that the config->ssl_password is a reference to memory that it 
doesn't own and free is not called on it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org

[GitHub] qpid-dispatch pull request #124: Allow passing the password via env variable

[dskarbek]

GitHub user dskarbek opened a pull request:

https://github.com/apache/qpid-dispatch/pull/124

Allow passing the password via env variable

Allows for the password in the config to actually be the name of the 
environment variable that contains the password.  This is indicated by 
prefixing the value with "env:".  We also add the option to prefix with 
"literal:" which indicates that it is a raw password.  That way, you can still 
give "env:SHELL" as a password, if that's what you really want to use, you 
would just have to list it as "literal:env:SHELL" in the config.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/dskarbek/qpid-dispatch 
allow_password_in_env_var

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/qpid-dispatch/pull/124.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #124


commit f84a135084de49f900a845ff934e212b03ec7957
Author: Daniel Skarbek 
Date:   2016-12-09T01:02:39Z

Allow the password config to point to an environment variable with the 
actual password




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org