You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benno Evers <be...@mesosphere.com> on 2019/06/19 14:46:49 UTC

Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/
-----------------------------------------------------------

(Updated June 19, 2019, 2:46 p.m.)


Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.


Summary (updated)
-----------------

Updated SSL docs to include new libprocess flag.


Repository: mesos


Description (updated)
-------

Added a description of the new `--hostname_validation_algorithm` flag
and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
environment variable.


Diffs (updated)
-----

  docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 


Diff: https://reviews.apache.org/r/70795/diff/2/

Changes: https://reviews.apache.org/r/70795/diff/1-2/


Testing
-------


Thanks,

Benno Evers


Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

Posted by Till Toenshoff via Review Board <no...@reviews.apache.org>.

> On June 21, 2019, 1:34 a.m., Till Toenshoff wrote:
> > docs/ssl.md
> > Lines 194 (patched)
> > <https://reviews.apache.org/r/70795/diff/3/?file=2151430#file2151430line194>
> >
> >     I wonder if we should already start a deprecation of the `libprocess` scheme - that would be:
> >     - announcing that `openssl` will be standard soon on compatible boxes
> >     - announcing it to be gone at some point
> >     
> >     Or am I too eager for unification here?
> 
> Benno Evers wrote:
>     It's actually a pretty big change - the 'libprocess' behaviour was built, I assume, to "magically" work with normal certificates w/o IP addresses despite libprocess only knowing about IP addresses. In DC/OS we don't notice most of it, since there all our certificates *do* contain the correct IP address, but at least quite a few unit tests will break by switching the default.
>     
>     So I actually agree we should do this deprecation, but I'm not sure about the timeline.
> 
> Benno Evers wrote:
>     Created https://issues.apache.org/jira/browse/MESOS-9857 to track the change.

Great - next we would update all relevant documentation with a deprecation note and a reference of that ticket.

Right now I am contemplating doing this in a single run, right away, instead of multiple phases. Multiple phases would  which would allow us to have that `libprocess` default without having to warn about it. 

What do you think?

We would ...
- add a comment note here
- add an SSL flags description note
- possibly have the flags validation output a deprecation warning
- anything I forgot here?


- Till


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/#review216020
-----------------------------------------------------------


On June 21, 2019, 3:05 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> -----------------------------------------------------------
> 
> (Updated June 21, 2019, 3:05 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a description of the new `--hostname_validation_scheme` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_SCHEME`
> environment variable.
> 
> 
> Diffs
> -----
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70795/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

Posted by Benno Evers <be...@mesosphere.com>.

> On June 21, 2019, 1:34 a.m., Till Toenshoff wrote:
> > docs/ssl.md
> > Lines 194 (patched)
> > <https://reviews.apache.org/r/70795/diff/3/?file=2151430#file2151430line194>
> >
> >     I wonder if we should already start a deprecation of the `libprocess` scheme - that would be:
> >     - announcing that `openssl` will be standard soon on compatible boxes
> >     - announcing it to be gone at some point
> >     
> >     Or am I too eager for unification here?
> 
> Benno Evers wrote:
>     It's actually a pretty big change - the 'libprocess' behaviour was built, I assume, to "magically" work with normal certificates w/o IP addresses despite libprocess only knowing about IP addresses. In DC/OS we don't notice most of it, since there all our certificates *do* contain the correct IP address, but at least quite a few unit tests will break by switching the default.
>     
>     So I actually agree we should do this deprecation, but I'm not sure about the timeline.

Created https://issues.apache.org/jira/browse/MESOS-9857 to track the change.


- Benno


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/#review216020
-----------------------------------------------------------


On June 21, 2019, 3:05 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> -----------------------------------------------------------
> 
> (Updated June 21, 2019, 3:05 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a description of the new `--hostname_validation_scheme` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_SCHEME`
> environment variable.
> 
> 
> Diffs
> -----
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70795/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

Posted by Benno Evers <be...@mesosphere.com>.

> On June 21, 2019, 1:34 a.m., Till Toenshoff wrote:
> > docs/ssl.md
> > Lines 194 (patched)
> > <https://reviews.apache.org/r/70795/diff/3/?file=2151430#file2151430line194>
> >
> >     I wonder if we should already start a deprecation of the `libprocess` scheme - that would be:
> >     - announcing that `openssl` will be standard soon on compatible boxes
> >     - announcing it to be gone at some point
> >     
> >     Or am I too eager for unification here?

It's actually a pretty big change - the 'libprocess' behaviour was built, I assume, to "magically" work with normal certificates w/o IP addresses despite libprocess only knowing about IP addresses. In DC/OS we don't notice most of it, since there all our certificates *do* contain the correct IP address, but at least quite a few unit tests will break by switching the default.

So I actually agree we should do this deprecation, but I'm not sure about the timeline.


- Benno


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/#review216020
-----------------------------------------------------------


On June 20, 2019, 5:27 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> -----------------------------------------------------------
> 
> (Updated June 20, 2019, 5:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a description of the new `--hostname_validation_algorithm` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
> environment variable.
> 
> 
> Diffs
> -----
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70795/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

Posted by Till Toenshoff via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/#review216020
-----------------------------------------------------------


Fix it, then Ship it!




Great! 

Please don't forget to update the review description to match the changed naming.


docs/ssl.md
Lines 176 (patched)
<https://reviews.apache.org/r/70795/#comment302972>

    s/can not/cannot/



docs/ssl.md
Lines 194 (patched)
<https://reviews.apache.org/r/70795/#comment302973>

    I wonder if we should already start a deprecation of the `libprocess` scheme - that would be:
    - announcing that `openssl` will be standard soon on compatible boxes
    - announcing it to be gone at some point
    
    Or am I too eager for unification here?


- Till Toenshoff


On June 20, 2019, 5:27 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> -----------------------------------------------------------
> 
> (Updated June 20, 2019, 5:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a description of the new `--hostname_validation_algorithm` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
> environment variable.
> 
> 
> Diffs
> -----
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70795/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/
-----------------------------------------------------------

(Updated July 4, 2019, 6:58 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.


Changes
-------

Renamed `libprocess` -> `legacy`.


Repository: mesos


Description
-------

Added a description of the new `--hostname_validation_scheme` flag
and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_SCHEME`
environment variable.


Diffs (updated)
-----

  docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 


Diff: https://reviews.apache.org/r/70795/diff/5/

Changes: https://reviews.apache.org/r/70795/diff/4-5/


Testing
-------


Thanks,

Benno Evers


Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/
-----------------------------------------------------------

(Updated June 21, 2019, 3:05 p.m.)


Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.


Repository: mesos


Description (updated)
-------

Added a description of the new `--hostname_validation_scheme` flag
and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_SCHEME`
environment variable.


Diffs (updated)
-----

  docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 


Diff: https://reviews.apache.org/r/70795/diff/4/

Changes: https://reviews.apache.org/r/70795/diff/3-4/


Testing
-------


Thanks,

Benno Evers


Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/
-----------------------------------------------------------

(Updated June 20, 2019, 5:27 p.m.)


Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.


Repository: mesos


Description
-------

Added a description of the new `--hostname_validation_algorithm` flag
and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
environment variable.


Diffs (updated)
-----

  docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 


Diff: https://reviews.apache.org/r/70795/diff/3/

Changes: https://reviews.apache.org/r/70795/diff/2-3/


Testing
-------


Thanks,

Benno Evers


Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

Posted by Benno Evers <be...@mesosphere.com>.

> On June 20, 2019, 1:05 p.m., Jan-Philip Gehrcke wrote:
> > docs/ssl.md
> > Lines 145 (patched)
> > <https://reviews.apache.org/r/70795/diff/2/?file=2151355#file2151355line145>
> >
> >     "via DNS name" instead of "via hostname"?

I know that "DNS name" is more common in the context of x509 certification, but here I think it may be confusing for readers: It's the first only time a "DNS name" is mentioned in this document, and also there's no requirement that the DNS name must be resolved via DNS or stored in any DNS server.


- Benno


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/#review215998
-----------------------------------------------------------


On June 20, 2019, 5:27 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> -----------------------------------------------------------
> 
> (Updated June 20, 2019, 5:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a description of the new `--hostname_validation_algorithm` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
> environment variable.
> 
> 
> Diffs
> -----
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70795/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

Posted by Jan-Philip Gehrcke via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/#review215998
-----------------------------------------------------------


Fix it, then Ship it!




Great work, left a few comments. Thanks!


docs/ssl.md
Lines 132 (patched)
<https://reviews.apache.org/r/70795/#comment302930>

    Let's maybe call this VALIDATION_SCHEME instead of VALIDATION_ALGORITHM



docs/ssl.md
Lines 142 (patched)
<https://reviews.apache.org/r/70795/#comment302934>

    backticks around `openssl`?



docs/ssl.md
Lines 145 (patched)
<https://reviews.apache.org/r/70795/#comment302931>

    "via DNS name" instead of "via hostname"?



docs/ssl.md
Lines 147 (patched)
<https://reviews.apache.org/r/70795/#comment302932>

    Even if this consumes a little more space let's make clear what happens when `LIBPROCESS_SSL_VERIFY_IPADD` is false 
    
    (IIUC then the TLS handshake will fail when we use IP addresses for opening the connection)



docs/ssl.md
Lines 150 (patched)
<https://reviews.apache.org/r/70795/#comment302933>

    "Don't"
    or
    "Do not"



docs/ssl.md
Lines 154 (patched)
<https://reviews.apache.org/r/70795/#comment302935>

    backticks?



docs/ssl.md
Lines 177 (patched)
<https://reviews.apache.org/r/70795/#comment302936>

    Great!


- Jan-Philip Gehrcke


On June 19, 2019, 2:46 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> -----------------------------------------------------------
> 
> (Updated June 19, 2019, 2:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a description of the new `--hostname_validation_algorithm` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
> environment variable.
> 
> 
> Diffs
> -----
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70795/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>