Merge lp:~hloeung/charm-haproxy/bump-global-dh-param into lp:charm-haproxy

Proposed by Haw Loeung
Status: Merged
Approved by: Haw Loeung
Approved revision: 144
Merged at revision: 143
Proposed branch: lp:~hloeung/charm-haproxy/bump-global-dh-param
Merge into: lp:charm-haproxy
Diff against target: 18 lines (+2/-2)
1 file modified
config.yaml (+2/-2)
To merge this branch: bzr merge lp:~hloeung/charm-haproxy/bump-global-dh-param
Reviewer Review Type Date Requested Status
Tom Haddon Approve
Canonical IS Reviewers Pending
Review via email: mp+388543@code.launchpad.net

Commit message

Bump DH params used, it's year 2020

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

143. By Haw Loeung

Fixed description

Revision history for this message
Tom Haddon (mthaddon) wrote :

One comment about the description field.

I also have a question about the fact that java 7 is the default on trusty. This will break new deployments on trusty that don't specify global_default_dh_param explicitly. I think our options are to:

1) Drop support for trusty
2) Update the code to fallback to 1024 if java 7 or lower is installed despite the setting of this config option
3) Leave it as is and essentially require anyone deploying with java 7 to set this to 1024 manually.

I don't see any references to java or openjdk in the charm code - do we know why this is called out explicitly? What is java potentially being used for here? Looks like this description was added in revno 88 on 205-02-20.

Revision history for this message
Haw Loeung (hloeung) wrote :

> One comment about the description field.
>
> I also have a question about the fact that java 7 is the default on trusty.
> This will break new deployments on trusty that don't specify
> global_default_dh_param explicitly. I think our options are to:
>
> 1) Drop support for trusty
> 2) Update the code to fallback to 1024 if java 7 or lower is installed despite
> the setting of this config option
> 3) Leave it as is and essentially require anyone deploying with java 7 to set
> this to 1024 manually.

Actually, it's not if the Java package is installed on the same unit as HAProxy but rather if you think or know you want to support Java 7 and lower clients, then you need to make sure the dhparam here is 1024.

> I don't see any references to java or openjdk in the charm code - do we know
> why this is called out explicitly? What is java potentially being used for
> here? Looks like this description was added in revno 88 on 205-02-20.

The description looks like it's one taken straight out of the HAProxy manual:

| http://cbonte.github.io/haproxy-dconv/1.8/configuration.html#3.2-tune.ssl.default-dh-param

But it looks like that has been updated, default in HAProxy 2.2 is now 2048:

| http://cbonte.github.io/haproxy-dconv/2.2/configuration.html#3.2-tune.ssl.default-dh-param

144. By Haw Loeung

Fixed comment per review

Revision history for this message
Tom Haddon (mthaddon) wrote :

Lgtm, thx

review: Approve
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 143

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'config.yaml'
2--- config.yaml 2019-07-02 22:08:28 +0000
3+++ config.yaml 2020-08-28 03:54:21 +0000
4@@ -40,12 +40,12 @@
5 description: |
6 Whether to enable the stats UNIX socket.
7 global_default_dh_param:
8- default: 1024
9+ default: 2048
10 type: int
11 description: |
12 Sets the maximum size of the Diffie-Hellman parameters used for generating
13 the ephemeral/temporary Diffie-Hellman key in case of DHE key exchange.
14- Default value if 1024, higher values will increase the CPU load, and values
15+ Default value if 2048, higher values will increase the CPU load. Values
16 greater than 1024 bits are not supported by Java 7 and earlier clients. This
17 config key will be ignored if the installed haproxy package has no SSL support.
18 global_default_bind_ciphers:

Subscribers

People subscribed via source and target branches

to all changes: