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

[Benno Evers]

---
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.

[Till Toenshoff via Review Board]

> On June 21, 2019, 1:34 a.m., Till Toenshoff wrote:
> > docs/ssl.md
> > Lines 194 (patched)
> > 
> >
> > 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.

[Benno Evers]

> On June 21, 2019, 1:34 a.m., Till Toenshoff wrote:
> > docs/ssl.md
> > Lines 194 (patched)
> > 
> >
> > 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.

[Benno Evers]

---
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.

[Benno Evers]

> On June 21, 2019, 1:34 a.m., Till Toenshoff wrote:
> > docs/ssl.md
> > Lines 194 (patched)
> > 
> >
> > 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.

[Till Toenshoff via Review Board]

---
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)


s/can not/cannot/



docs/ssl.md
Lines 194 (patched)


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.

[Benno Evers]

> On June 20, 2019, 1:05 p.m., Jan-Philip Gehrcke wrote:
> > docs/ssl.md
> > Lines 145 (patched)
> > 
> >
> > "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.

[Benno Evers]

---
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.

[Jan-Philip Gehrcke via Review Board]

---
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)


Let's maybe call this VALIDATION_SCHEME instead of VALIDATION_ALGORITHM



docs/ssl.md
Lines 142 (patched)


backticks around `openssl`?



docs/ssl.md
Lines 145 (patched)


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



docs/ssl.md
Lines 147 (patched)


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)


"Don't"
or
"Do not"



docs/ssl.md
Lines 154 (patched)


backticks?



docs/ssl.md
Lines 177 (patched)


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
> 
>

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

[Benno Evers]

---
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