----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65090/#review195762 -----------------------------------------------------------
Fix it, then Ship it! src/master/flags.hpp Line 105 (original), 105 (patched) <https://reviews.apache.org/r/65090/#comment275064> Nit: in the description, I would recommend: s/This prevents this leakage/This patch prevents this leakage/ I think it's just a bit more clear. - Greg Mann On Jan. 18, 2018, 7:01 p.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65090/ > ----------------------------------------------------------- > > (Updated Jan. 18, 2018, 7:01 p.m.) > > > Review request for mesos and Greg Mann. > > > Bugs: MESOS-8413 > https://issues.apache.org/jira/browse/MESOS-8413 > > > Repository: mesos > > > Description > ------- > > Up until now the Mesos master flag `--zk` as well as the Mesos agent > flag `--master` would leak ZooKeeper authentication credentials in > both logs and results for the `/flags` endpoint, if the credentials > were part of the configuration url. > > This prevents this leakage if a user decides to store the ZooKeeper > url in a file and pass the file as a value to the flags mentioned > above (using the preffix `file://`). > > > Diffs > ----- > > src/master/flags.hpp dabb414560f506787b6e821a27af623c8da44b11 > src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71 > src/slave/flags.hpp 42c4861b5ecdc808d04bfe8f5c35572074fd2bdc > src/slave/main.cpp f38fec6028ade0e0a51fd2cce7470c5c36e66396 > > > Diff: https://reviews.apache.org/r/65090/diff/2/ > > > Testing > ------- > > ```sh > make -j12 check > > # We don't seem to test flags in unit tests anywhere, > # so additionally I ran: > > docker pull zookeeper > > cat <<EOF > /tmp/$USER/zk-stack.yml > version: '3.1' > services: > zoo1: > image: zookeeper > restart: always > hostname: zoo1 > ports: > - 2181:2181 > environment: > ZOO_MY_ID: 1 > ZOO_SERVERS: server.1=0.0.0.0:2888:3888 server.2=zoo2:2888:3888 > server.3=zoo3:2888:3888 > zoo2: > image: zookeeper > restart: always > hostname: zoo2 > ports: > - 2182:2181 > environment: > ZOO_MY_ID: 2 > ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=0.0.0.0:2888:3888 > server.3=zoo3:2888:3888 > zoo3: > image: zookeeper > restart: always > hostname: zoo3 > ports: > - 2183:2181 > environment: > ZOO_MY_ID: 2 > ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=zoo2:2888:3888 > server.3=0.0.0.0:2888:3888 > EOF > > docker-compose -f /tmp/$USER/zk-stack.yml up > > cd ${MESOS_BUILD_DIR} > > # This command should fail to launch because there is no file zk.conf > ./bin/mesos-master.sh \ > --work_dir=/tmp/$USER/mesos/master \ > --log_dir=/tmp/$USER/mesos/master/log \ > --ip=$PUBLIC_IP \ > --quorum=1 \ > --zk=file:///tmp/$USER/zk/zk.conf > > cat <<EOF > /tmp/$USER/zk/zk.conf > zk://$PUBLIC_IP:2181,$PUBLIC_IP:2182,$PUBLIC_IP:2183/mesos > EOF > > ./bin/mesos-master.sh \ > --work_dir=/tmp/$USER/mesos/master \ > --log_dir=/tmp/$USER/mesos/master/log \ > --ip=$PUBLIC_IP \ > --quorum=1 \ > --zk=`cat /tmp/$USER/zk/zk.conf` & > > [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == `cat > /tmp/$USER/zk/zk.conf` ]] > > kill %1 > > > ./bin/mesos-master.sh \ > --work_dir=/tmp/$USER/mesos/master \ > --log_dir=/tmp/$USER/mesos/master/log \ > --ip=$PUBLIC_IP \ > --quorum=1 \ > --zk=file:///tmp/$USER/zk/zk.conf & > > [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == > "/tmp/$USER/zk/zk.conf" ]] > > ./bin/mesos-agent.sh \ > --work_dir=/tmp/$USER/mesos/agent \ > --log_dir=/tmp/$USER/mesos/agent/log \ > --master=file:///tmp/$USER/zk/zk.conf & > > [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == > "/tmp/$USER/zk/zk.conf" ]] > > kill %2 > > ./bin/mesos-agent.sh \ > --work_dir=/tmp/$USER/mesos/agent \ > --log_dir=/tmp/$USER/mesos/agent/log \ > --zk=`cat /tmp/$USER/zk/zk.conf` & > > [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == `cat > /tmp/$USER/zk/zk.conf` ]] > > kill %2 > kill %1 > ``` > > > Thanks, > > Alexander Rojas > >
