Raise the default min TLS version to 1.2 to improve baseline configuration security,
Metadata Update from @firstyear: - Issue assigned to firstyear
Metadata Update from @firstyear: - Custom field component adjusted to None - Custom field origin adjusted to None - Custom field reviewstatus adjusted to None - Custom field type adjusted to None - Custom field version adjusted to None - Issue set to the milestone: 1.4 backlog
[06/Oct/2017:08:07:51.208181797 +1000] - INFO - Security Initialization - slapd_ssl_init2 - Configured SSL version range: min: TLS1.2, max: TLS1.3
<img alt="0001-Ticket-49395-Raise-min-TLS-version-to-1.2.patch" src="/389-ds-base/issue/raw/files/0af22a1dea2fb06b9a3445ea7a98255be89406fcf3e065308511aac5db9ac35c-0001-Ticket-49395-Raise-min-TLS-version-to-1.2.patch" />
Metadata Update from @firstyear: - Custom field reviewstatus adjusted to review (was: None)
The only issue with this is that it could break old customer apps that for some reason might still need to use "insecure" protocols. This is why we did not set it to TLS 1.1 when this feature was first added. We just need to be careful when we change the default behavior of the server - that's all.
That being said, I would think it would be safe to do for 1.4, but we should discuss this with the rest of the team.
Yeah, I would be targeting 1.4 here.
I think it's important to do this, because we should be encouraging secure protocol use, and admins with affected applications can still downgrade as needed.
@lkrispen @tbordaz @mhonek What do you all think?
@mhonek Has commented that he approves. Anyone else? I would like this acked and merged soon please :)
Matus had also made sime other suggestions in his comment which should be followed in other tickets. Do we have these tickets ? If not and we close this one we will forget them. Otherwise acc.
@mhonek From your email comments about TLS, can we open some tickets up?
@tbordaz has suggested that if sslVersionMin < DEFAULT_MIN, that we log a warning syaing the admin should upgrade. This only occurs if admin has set the value. Update the patch to have this warning.
So I went to fix this up quickly yesterday, and it looks like when you change TLS DEF MIN, it actually is a minimum limit of tls, even if the user sets TLS1.0. I think there is some logic issues in ssl.c about how this value is handled.....
It is time to increase this default. I am going to add it to this issue (PR to follow shortly):
https://pagure.io/389-ds-base/issue/50362
Once the PR is out, we can close this.
It is time to increase this default. I am going to add it to this issue (PR to follow shortly): https://pagure.io/389-ds-base/issue/50362 Once the PR is out, we can close this.
We should probably just use NSS default, which is basically what crypto team thinks is the best -- aka system-wide crypto policies. There's actually an issue for that #48785.
@mhonek I tottaly agree with this.
@mhonek I totally agree with this.
The patch I am working currently does use the NSS defaults. It's tries to honor a requested range, but otherwise it does use what NSS says in the MIN and MAX - so this should be covered. Waiting on a NSS bug regarding FIPS & TLS1.3 before sending that PR out for review. Once that NSS issue is resolved I will verify this is the new behavior.
Is this covered by the system policy now?
Yes. In Fedora default one is still TLS1.0: https://gitlab.com/redhat-crypto/fedora-crypto-policies/blob/master/policies/DEFAULT.pol TLS1.2 is in the NEXT policy: https://gitlab.com/redhat-crypto/fedora-crypto-policies/blob/master/policies/NEXT.pol RHEL8 by default uses TLS1.2: https://git.centos.org/rpms/crypto-policies/blob/c8/f/SPECS/crypto-policies.spec#_14 https://gitlab.com/redhat-crypto/fedora-crypto-policies/blob/next-default/policies/DEFAULT.pol
Okay, so in that case, can we then close this because we don't need to act on it then?
I think we should force the minimum to TLS 1.2, because even in F32 is still defaulting to TLS 1.0 for the minimum. But our health check tool complains about it. We shouldn't ship the server in a state that health check is already complaining about it
https://pagure.io/389-ds-base/pull-request/50826
Please review...
Metadata Update from @mreynolds: - Issue assigned to mreynolds (was: firstyear) - Issue set to the milestone: 1.4.1 (was: 1.4 backlog)
Commit e034c29 relates to this ticket
d971bac..2a1a0d7 389-ds-base-1.4.2 -> 389-ds-base-1.4.2
51c604f..07c4799 389-ds-base-1.4.1 -> 389-ds-base-1.4.1
Metadata Update from @mreynolds: - Issue close_status updated to: fixed - Issue status updated to: Closed (was: Open)
389-ds-base is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in 389-ds-base's github repository.
This issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/2454
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: fixed)
Login to comment on this ticket.