Add TLS support for client-bricks communication #967

prashanthpai · 2018-07-06T08:56:33Z

This change enables setting up TLS for client to bricks communication.

Unlike glusterd1 which requires two separate options - client.ssl and
server.ssl to be turned on, turning TLS on for a volume in glusterd2
is a single volume set operation. TLS can be enabled for a volume as
follows:

$ glustercli volume set <volname> --advanced tls on

To achieve this, when xlator shared ojects are loaded during startup,
options present in transport layer (socket.so and rdma.so) are injected
into list of options loaded from protocol layer (server.so and
client.so). The client and brick volfile expects the transport options
to be in client and server xlator sections.

This PR also tests that client authorization works as expected which can be used as follows:

$ glustercli volume set test --advanced server.ssl-allow <client-CN-name>

This is dependent on: https://review.gluster.org/#/c/20472

Other fixes:

Add support for xlator options containing dots
volume-set: Validate expanded group options (bug fix)
Set default boolean value properly for group option
Fix bug in validating internet addresses

Closes #214
Closes #971

prashanthpai · 2018-07-06T09:03:24Z

Tested as follows:

Setting up certs

# openssl genrsa -out /etc/ssl/glusterfs.key 2048
# openssl req -new -x509 -key /etc/ssl/glusterfs.key -subj /CN=Anyone -out /etc/ssl/glusterfs.pem
# cp /etc/ssl/glusterfs.pem /etc/ssl/glusterfs.ca

Enabling TLS for a volume

glustercli volume set test --advanced tls on

$ glustercli volume info test

Volume Name: test
Type: Distribute
Volume ID: 12eb8aa0-8c2a-4aeb-b957-aba2d74f58b3
State: Created
Transport-type: tcp
Options:
    client.transport.socket.ssl-enabled: on
    server.transport.socket.ssl-enabled: on
Number of Bricks: 1
Brick1: 127.0.0.1:/export/brick1/data

This shall be added to e2e tests.

rishubhjain · 2018-07-06T09:07:29Z

glusterd2/commands/volumes/volume-option.go

@@ -43,7 +43,7 @@ func optionSetValidate(c transaction.TxnCtx) error {
 		return err
 	}

-	if err := validateXlatorOptions(req.Options, &volinfo); err != nil {
+	if err := validateXlatorOptions(options, &volinfo); err != nil {
 		return fmt.Errorf("Validation failed for volume option:: %s", err.Error())


TRIVIAL : @prashanthpai @Madhu-1 Should this statement start with lower case as well? Not part of changes made in this PR though.

This change explicitly adds 'ssl-allow' options to the server xlator's options table so that glusterd2 can see it as a settable option. This change also marks 'auth.allow' and 'auth.reject' options as a settable. Glusterd2 doesn't maintain a separate volume options table. Glusterd2 dynamically loads shared objects of xlators to read their option table and other information. Glusterd2 reads 'xlator_api_t' if available. If that's not available, it falls back to reading just the options table directly. In glusterd2, volume set operations are performed by users on keys of the format <xlator>.<option-name>. Glusterd2 uses xlator name set in 'xlator_api_t.identifier'. If that's not present it will use the shared object's file name as xlator name. Hence, it is important for 'xlator_api_t.identifier' to be set properly, and in this case, the proper value is "server". This name shall be used by users as prefix while setting volume options implemented in server xlator. The name will also be used in volfile. A user in glusterd2 can authorize a client over TLS as follows: $ glustercli volume set <volname> server.ssl-allow <client1-CN>[,<clientN-CN>] gd2 References: gluster/glusterd2#971 gluster/glusterd2#214 gluster/glusterd2#967 Updates: bz#1193929 Change-Id: I59ef58acb8d51917e6365a83be03e79ae7c5ad17 Signed-off-by: Prashanth Pai <ppai@redhat.com>

kshlm

LGTM. A small change suggested for easier understand ability.

kshlm · 2018-07-12T04:21:04Z

glusterd2/xlator/options/utils.go

+		}
+		graph = tmp[0]
+		xlator = tmp[1]
+		optName = k[len(graph)+1+len(xlator)+1:]


strings.Join(tmp[2:], ".") would be more readable here.

The options passed to validation function was the original group option present in the client request. This is incorrect and validation used to fail as group options aren't present in xlators. With this change, the expanded group options are passed to validation function. Signed-off-by: Prashanth Pai <ppai@redhat.com>

An empty default value for boolean type is to be treated as false. This will be used during resetting options to its default value. Signed-off-by: Prashanth Pai <ppai@redhat.com>

...that is used in setting volume options such as this one: server.auth.addr.*.allow 127.0.0.1 Signed-off-by: Prashanth Pai <ppai@redhat.com>

Volume set/reset options can be of one of these forms: <graph>.<xlator>.<option-name> <xlator>.<option-name> As xlator option name such as `transport.socket.ssl-enabled` can themselves contain dots, for the options.SplitKey() function it was hard to figure out which is graph and which is xlator. This change ensures that graph and xlator component are properly split for xlator options that contain dots. Signed-off-by: Prashanth Pai <ppai@redhat.com>

This change enables setting up TLS for client to bricks communication. Unlike glusterd1 which requires two separate options - `client.ssl` and `server.ssl` to be turned on, turning TLS on for a volume in glusterd2 is a single volume set operation. TLS can be enabled for a volume as follows: $ glustercli volume set <volname> --advanced tls on To achieve this, when xlator shared ojects are loaded during startup, options present in transport layer (socket.so and rdma.so) are injected into list of options loaded from protocol layer (server.so and client.so). The client and brick volfile expects the transport options to be in client and server xlator sections. Signed-off-by: Prashanth Pai <ppai@redhat.com>

kshlm · 2018-07-12T12:12:06Z

glusterd2/commands/volumes/grouped-options.go

+	"tls": {"tls",
+		[]api.VolumeOption{
+			{Name: "server.transport.socket.ssl-enabled", OnValue: "on"},
+			{Name: "client.transport.socket.ssl-enabled", OnValue: "on"},


We should possibly try to get the socket.ssl-* options renamed to socket.tls-*. Makes this more the option more consistent.

This change explicitly adds 'ssl-allow' options to the server xlator's options table so that glusterd2 can see it as a settable option. This change also marks 'auth.allow' and 'auth.reject' options as a settable. Glusterd2 doesn't maintain a separate volume options table. Glusterd2 dynamically loads shared objects of xlators to read their option table and other information. Glusterd2 reads 'xlator_api_t' if available. If that's not available, it falls back to reading just the options table directly. In glusterd2, volume set operations are performed by users on keys of the format <xlator>.<option-name>. Glusterd2 uses xlator name set in 'xlator_api_t.identifier'. If that's not present it will use the shared object's file name as xlator name. Hence, it is important for 'xlator_api_t.identifier' to be set properly, and in this case, the proper value is "server". This name shall be used by users as prefix while setting volume options implemented in server xlator. The name will also be used in volfile. A user in glusterd2 can authorize a client over TLS as follows: $ glustercli volume set <volname> server.ssl-allow <client1-CN>[,<clientN-CN>] gd2 References: gluster/glusterd2#971 gluster/glusterd2#214 gluster/glusterd2#967 Updates: bz#1193929 Change-Id: I59ef58acb8d51917e6365a83be03e79ae7c5ad17 Signed-off-by: Prashanth Pai <ppai@redhat.com>

prashanthpai requested review from kshlm and aravindavk and removed request for kshlm July 6, 2018 08:57

rishubhjain reviewed Jul 6, 2018

View reviewed changes

prashanthpai force-pushed the issue-214-part1 branch from 2dd63bf to fb0cf02 Compare July 6, 2018 09:40

atinmu added the GCS-Sprint1-review-pending label Jul 9, 2018

This comment has been minimized.

Sign in to view

prashanthpai force-pushed the issue-214-part1 branch 5 times, most recently from 586016b to 24f75e8 Compare July 10, 2018 02:35

prashanthpai mentioned this pull request Jul 10, 2018

Add TLS support for glusterfs handshake #980

Open

prashanthpai force-pushed the issue-214-part1 branch from 96048b8 to db33521 Compare July 10, 2018 10:52

prashanthpai force-pushed the issue-214-part1 branch from db33521 to 984e82b Compare July 11, 2018 04:13

kshlm reviewed Jul 12, 2018

View reviewed changes

prashanthpai added 5 commits July 12, 2018 10:33

options: Explicitly set default boolean value properly

773514c

An empty default value for boolean type is to be treated as false. This will be used during resetting options to its default value. Signed-off-by: Prashanth Pai <ppai@redhat.com>

Fix bug in validating internet addresses

bba06dd

...that is used in setting volume options such as this one: server.auth.addr.*.allow 127.0.0.1 Signed-off-by: Prashanth Pai <ppai@redhat.com>

prashanthpai force-pushed the issue-214-part1 branch from 984e82b to 4a241a8 Compare July 12, 2018 05:05

Merge branch 'master' into issue-214-part1

b181404

kshlm approved these changes Jul 12, 2018

View reviewed changes

kshlm merged commit a28efc6 into gluster:master Jul 12, 2018

prashanthpai mentioned this pull request Jul 17, 2018

Document TLS usage for client-brick communication #1023

Open

gluster-ant mentioned this pull request Mar 12, 2020

[bug:1193929] GlusterFS can be improved gluster/glusterfs#1000

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add TLS support for client-bricks communication #967

Add TLS support for client-bricks communication #967

prashanthpai commented Jul 6, 2018 •

edited

Loading

prashanthpai commented Jul 6, 2018 •

edited

Loading

rishubhjain Jul 6, 2018

prashanthpai Jul 6, 2018

This comment has been minimized.

kshlm left a comment

kshlm Jul 12, 2018

prashanthpai Jul 12, 2018

kshlm Jul 12, 2018 •

edited

Loading

Add TLS support for client-bricks communication #967

Add TLS support for client-bricks communication #967

Conversation

prashanthpai commented Jul 6, 2018 • edited Loading

prashanthpai commented Jul 6, 2018 • edited Loading

rishubhjain Jul 6, 2018

Choose a reason for hiding this comment

prashanthpai Jul 6, 2018

Choose a reason for hiding this comment

This comment has been minimized.

kshlm left a comment

Choose a reason for hiding this comment

kshlm Jul 12, 2018

Choose a reason for hiding this comment

prashanthpai Jul 12, 2018

Choose a reason for hiding this comment

kshlm Jul 12, 2018 • edited Loading

Choose a reason for hiding this comment

prashanthpai commented Jul 6, 2018 •

edited

Loading

prashanthpai commented Jul 6, 2018 •

edited

Loading

kshlm Jul 12, 2018 •

edited

Loading