[GitHub] qpid-dispatch pull request #224: DISPATCH-884 - Added schema property protoc...

2017-12-08 Thread ganeshmurthy 
Github user ganeshmurthy commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/224#discussion_r155841697
  
--- Diff: python/qpid_dispatch/management/qdrouter.json ---
@@ -515,7 +515,12 @@
 "type": "string",
 "description": "Specifies the enabled ciphers so the 
SSL Ciphers can be hardened. In other words, use this field to disable weak 
ciphers. The ciphers are specified in the format understood by the OpenSSL 
library. For example, ciphers can be set to 
ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP; -- The full list 
of allowed ciphers can be viewed using the openssl ciphers command",
 "create": true
-},
+},  
+"protocols": {
+"type": "string",
+"description": "This list is a space separated string 
of the allowed TLS protocols. The current possibilities are TLSv1 TLSv1.1 
TLSv1.2. For example, if you want to permit only TLS V.1.1 and TLSv1.2, your 
value for the protocols would be TLSv1.1 TLSv1.2. If this attribute is not set, 
then all the TLS protocols are allowed.",
--- End diff --

Your description for protocols - "The TLS protocols that this sslProfile 
can use. You can specify a list of one or more of TLSv1, TLSv1.1, or TLSv1.2. 
To specify multiple protocols, separate the protocols with a space. For 
example, to permit the sslProfile to use TLS v1.1 and TLS v1.2 only, you would 
set the value to TLSv1.1 TLSv1.2. If you do not specify a value, the sslProfile 
uses the TLS protocol specified by the system-wide configuration."

looks good to me. I will update.


---

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

[GitHub] qpid-dispatch pull request #224: DISPATCH-884 - Added schema property protoc...

2017-12-08 Thread ganeshmurthy 
Github user ganeshmurthy commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/224#discussion_r155841178
  
--- Diff: python/qpid_dispatch/management/qdrouter.json ---
@@ -515,7 +515,12 @@
 "type": "string",
 "description": "Specifies the enabled ciphers so the 
SSL Ciphers can be hardened. In other words, use this field to disable weak 
ciphers. The ciphers are specified in the format understood by the OpenSSL 
library. For example, ciphers can be set to 
ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP; -- The full list 
of allowed ciphers can be viewed using the openssl ciphers command",
 "create": true
-},
+},  
+"protocols": {
+"type": "string",
+"description": "This list is a space separated string 
of the allowed TLS protocols. The current possibilities are TLSv1 TLSv1.1 
TLSv1.2. For example, if you want to permit only TLS V.1.1 and TLSv1.2, your 
value for the protocols would be TLSv1.1 TLSv1.2. If this attribute is not set, 
then all the TLS protocols are allowed.",
--- End diff --

The proton API requires that I use TLSv1 and not TLSV1.0
Please take a look at the doc of this function

https://github.com/apache/qpid-proton/blob/master/proton-c/include/proton/ssl.h#L242

I will gladly change it to TLSv1.0 if proton does it. This is a question 
for @ssorj 


---

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

[GitHub] qpid-dispatch pull request #224: DISPATCH-884 - Added schema property protoc...

2017-12-08 Thread ganeshmurthy 
Github user ganeshmurthy commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/224#discussion_r155839890
  
--- Diff: python/qpid_dispatch/management/qdrouter.json ---
@@ -515,7 +515,12 @@
 "type": "string",
 "description": "Specifies the enabled ciphers so the 
SSL Ciphers can be hardened. In other words, use this field to disable weak 
ciphers. The ciphers are specified in the format understood by the OpenSSL 
library. For example, ciphers can be set to 
ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP; -- The full list 
of allowed ciphers can be viewed using the openssl ciphers command",
 "create": true
-},
+},  
+"protocols": {
+"type": "string",
+"description": "This list is a space separated string 
of the allowed TLS protocols. The current possibilities are TLSv1 TLSv1.1 
TLSv1.2. For example, if you want to permit only TLS V.1.1 and TLSv1.2, your 
value for the protocols would be TLSv1.1 TLSv1.2. If this attribute is not set, 
then all the TLS protocols are allowed.",
--- End diff --

setting type to "type": ["TLSv1", "TLSv1.1", "TLSv1.2"] will mean that only 
one in the list is allowed. That is how router schema syntax works. 

For example look at "stripAnnotations". It type is set to "type": ["in", 
"out", "both", "no"] which means stripAnnotations can have only one of the 4 
values "in" or "out" or "both" or "no"


---

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

[GitHub] qpid-dispatch pull request #224: DISPATCH-884 - Added schema property protoc...

2017-12-08 Thread bhardesty 
Github user bhardesty commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/224#discussion_r155832041
  
--- Diff: python/qpid_dispatch/management/qdrouter.json ---
@@ -515,7 +515,12 @@
 "type": "string",
 "description": "Specifies the enabled ciphers so the 
SSL Ciphers can be hardened. In other words, use this field to disable weak 
ciphers. The ciphers are specified in the format understood by the OpenSSL 
library. For example, ciphers can be set to 
ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP; -- The full list 
of allowed ciphers can be viewed using the openssl ciphers command",
 "create": true
-},
+},  
+"protocols": {
+"type": "string",
+"description": "This list is a space separated string 
of the allowed TLS protocols. The current possibilities are TLSv1 TLSv1.1 
TLSv1.2. For example, if you want to permit only TLS V.1.1 and TLSv1.2, your 
value for the protocols would be TLSv1.1 TLSv1.2. If this attribute is not set, 
then all the TLS protocols are allowed.",
--- End diff --

Since the value is really a list of values, would it work to define the 
possible values in "type"? Then we wouldn't have to describe the permitted 
values in the description, making it simpler. Sort of like this:

`"type": ["TLSv1", "TLSv1.1", "TLSv1.2"]`


---

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

[GitHub] qpid-dispatch pull request #224: DISPATCH-884 - Added schema property protoc...

2017-12-08 Thread bhardesty 
Github user bhardesty commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/224#discussion_r155831204
  
--- Diff: python/qpid_dispatch/management/qdrouter.json ---
@@ -515,7 +515,12 @@
 "type": "string",
 "description": "Specifies the enabled ciphers so the 
SSL Ciphers can be hardened. In other words, use this field to disable weak 
ciphers. The ciphers are specified in the format understood by the OpenSSL 
library. For example, ciphers can be set to 
ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP; -- The full list 
of allowed ciphers can be viewed using the openssl ciphers command",
 "create": true
-},
+},  
+"protocols": {
+"type": "string",
+"description": "This list is a space separated string 
of the allowed TLS protocols. The current possibilities are TLSv1 TLSv1.1 
TLSv1.2. For example, if you want to permit only TLS V.1.1 and TLSv1.2, your 
value for the protocols would be TLSv1.1 TLSv1.2. If this attribute is not set, 
then all the TLS protocols are allowed.",
--- End diff --

A couple notes about the description:

I think it's most user-friendly to first define the attribute and then 
describe the syntax for using it. Something like this:

> The TLS protocols that this sslProfile can use. You can specify a list of 
one or more of TLSv1, TLSv1.1, or TLSv1.2. To specify multiple protocols, 
separate the protocols with a space. For example, to permit the sslProfile to 
use TLS v1.1 and TLS v1.2 only, you would set the value to TLSv1.1 TLSv1.2. If 
you do not specify a value, the sslProfile uses the TLS protocol specified by 
the system-wide configuration.

Instead of "TLSv1", it would be better to make the value "TLSv1.0". That 
would make it clear that it's referring to the 1.0 version, and not a superset 
of 1.x.

Also, we should define "system-wide configuration" - where would this be 
defined?


---

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

[GitHub] qpid-dispatch pull request #224: DISPATCH-884 - Added schema property protoc...

2017-12-08 Thread ganeshmurthy 
Github user ganeshmurthy closed the pull request at:

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


---

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

[GitHub] qpid-dispatch pull request #224: DISPATCH-884 - Added schema property protoc...

2017-12-07 Thread ssorj 
Github user ssorj commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/224#discussion_r155647815
  
--- Diff: src/connection_manager.c ---
@@ -45,6 +45,7 @@ struct qd_config_ssl_profile_t {
 char*ssl_certificate_file;
 char*ssl_private_key_file;
 char*ciphers;
+char*protocols;
--- End diff --

*ssl_protocols?


---

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

[GitHub] qpid-dispatch pull request #224: DISPATCH-884 - Added schema property protoc...

2017-12-07 Thread ssorj 
Github user ssorj commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/224#discussion_r155652198
  
--- Diff: include/qpid/dispatch/server.h ---
@@ -305,6 +305,12 @@ typedef struct qd_server_config_t {
  */
 char *ciphers;
 
+/**
+ * This list is a space separated string of the allowed TLS protocols. 
The current possibilities are TLSv1 TLSv1.1 TLSv1.2.
--- End diff --

A possible enhancement is to sub spaces for commas and document it as a 
comma-separated list.


---

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

[GitHub] qpid-dispatch pull request #224: DISPATCH-884 - Added schema property protoc...

2017-12-07 Thread ssorj 
Github user ssorj commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/224#discussion_r155651659
  
--- Diff: python/qpid_dispatch/management/qdrouter.json ---
@@ -515,7 +515,12 @@
 "type": "string",
 "description": "Specifies the enabled ciphers so the 
SSL Ciphers can be hardened. In other words, use this field to disable weak 
ciphers. The ciphers are specified in the format understood by the OpenSSL 
library. For example, ciphers can be set to 
ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP; -- The full list 
of allowed ciphers can be viewed using the openssl ciphers command",
 "create": true
-},
+},  
+"protocols": {
+"type": "string",
+"description": "This list is a space separated string 
of the allowed TLS protocols. The current possibilities are TLSv1 TLSv1.1 
TLSv1.2. For example, if you want to permit only TLS V.1.1 and TLSv1.2, your 
value for the protocols would be TLSv1.1 TLSv1.2. If this attribute is not set, 
then all the TLS protocols are allowed.",
--- End diff --

"then all the protocols are allowed" is not quite right.  If unset, it 
defers to the system-wide configuration.


---

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

[GitHub] qpid-dispatch pull request #224: DISPATCH-884 - Added schema property protoc...

2017-12-07 Thread ssorj 
Github user ssorj commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/224#discussion_r155647744
  
--- Diff: src/connection_manager.c ---
@@ -45,6 +45,7 @@ struct qd_config_ssl_profile_t {
 char*ssl_certificate_file;
 char*ssl_private_key_file;
 char*ciphers;
--- End diff --

*ssl_ciphers?


---

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

[GitHub] qpid-dispatch pull request #224: DISPATCH-884 - Added schema property protoc...

2017-12-07 Thread ssorj 
Github user ssorj commented on a diff in the pull request:

https://github.com/apache/qpid-dispatch/pull/224#discussion_r155649659
  
--- Diff: include/qpid/dispatch/server.h ---
@@ -305,6 +305,12 @@ typedef struct qd_server_config_t {
  */
 char *ciphers;
 
+/**
+ * This list is a space separated string of the allowed TLS protocols. 
The current possibilities are TLSv1 TLSv1.1 TLSv1.2.
+ * For example, if you want to permit only TLS V.1.1 and TLSv1.2, your 
value for the protocols would be TLSv1.1 TLSv1.2. If this attribute is not set, 
then all the TLS protocols are allowed.
--- End diff --

"TLS V.1.1" -> "TLSv1.1"


---

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

[GitHub] qpid-dispatch pull request #224: DISPATCH-884 - Added schema property protoc...

2017-11-22 Thread ganeshmurthy 
GitHub user ganeshmurthy opened a pull request:

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

DISPATCH-884 - Added schema property protocols to allow configurable …

…TLS protocol versions

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

$ git pull https://github.com/ganeshmurthy/qpid-dispatch DISPATCH-884-1

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

https://github.com/apache/qpid-dispatch/pull/224.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 #224


commit 3af608ee97aac0861d5a60d94c476aa40e9d4364
Author: Ganesh Murthy 
Date:   2017-11-22T15:47:09Z

DISPATCH-884 - Added schema property protocols to allow configurable TLS 
protocol versions




---

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