Merge ~utkarsh/ubuntu/+source/openldap:lp1921562-openldap-focal into ubuntu/+source/openldap:ubuntu/focal-devel

Proposed by Utkarsh Gupta on 2021-04-08
Status: Merged
Approved by: Christian Ehrhardt  on 2021-04-08
Approved revision: bad0f80f7759e2f2b371c899e0a2975f9dfa7c49
Merged at revision: bad0f80f7759e2f2b371c899e0a2975f9dfa7c49
Proposed branch: ~utkarsh/ubuntu/+source/openldap:lp1921562-openldap-focal
Merge into: ubuntu/+source/openldap:ubuntu/focal-devel
Diff against target: 182 lines (+160/-0)
3 files modified
debian/changelog (+8/-0)
debian/patches/ITS-8650-loop-on-incomplete-TLS-handshake.patch (+151/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
Christian Ehrhardt  2021-04-08 Approve on 2021-04-08
Canonical Server Team 2021-04-08 Pending
Sergio Durigan Junior 2021-04-08 Pending
Ubuntu Server Dev import team 2021-04-08 Pending
Review via email: mp+400754@code.launchpad.net

Description of the change

Hello,

This MP intends to fix LP: #1921562 for Focal. And as discussed in the standup a few days ago, I am requesting review from Sergio explicitly :)

A note for the reviewer: given the very nature of the bug, it might get a bit hard to reproduce so there aren't really very "direct" or "clear" steps to reproduce and thus the "Test Plan" section mentions only what the reporter wrote. Hope this makes sense and is OK for this case?

That said, I've asked the reporter to verify the fix as well; but since Vincent already mentioned, he has been running a similar patched version in production for a few weeks w/o any problems now, so it should be good at that front.

And finally, PPA could be found at https://launchpad.net/~utkarsh/+archive/ubuntu/experimental-dump.

Requesting you to review and sponsor the upload if it makes sense. TIA! \o/

To post a comment you must log in.
Christian Ehrhardt  (paelzer) wrote :

I verified that change matches what we have applied since 2.4.49+dfsg-4

Also I cleaned up the bug in regard to Groovy

Changelog LTGM

I've checked upstream git if there are important pre-requisites 2.4.49+dfsg-2ubuntu1.7 to when it was applied (none) or follow up fixes that we'd need (none)

7cf7aa3 already is a backport by upstream to 2.4.50 from the original 735e1ab1 on 2.5.0 - that looks safe as well.

Build log and PPA did not reveal any concerns in a quick (superficial) check (and arm builds were not yet complete).

Yeah - other than the "hard to test" I think this is fine.
+1

P.S. As a general hint, building everything in one PPA seems to be appealing for being easy, but from being burnt by things that behave differently by being built against other packages in there I'd recommend you one PPA per case.

review: Approve
Christian Ehrhardt  (paelzer) wrote :

To ssh://git.launchpad.net/ubuntu/+source/openldap
 * [new tag] upload/2.4.49+dfsg-2ubuntu1.8 -> upload/2.4.49+dfsg-2ubuntu1.8

Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading openldap_2.4.49+dfsg-2ubuntu1.8.dsc: done.
  Uploading openldap_2.4.49+dfsg-2ubuntu1.8.debian.tar.xz: done.
  Uploading openldap_2.4.49+dfsg-2ubuntu1.8_source.buildinfo: done.
  Uploading openldap_2.4.49+dfsg-2ubuntu1.8_source.changes: done.
Successfully uploaded packages.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index bc4b682..58a944d 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,11 @@
6+openldap (2.4.49+dfsg-2ubuntu1.8) focal; urgency=medium
7+
8+ * d/p/ITS-8650-loop-on-incomplete-TLS-handshake.patch:
9+ Import upstream patch to properly retry gnutls_handshake() after it
10+ returns GNUTLS_E_AGAIN. (ITS#8650) (LP: #1921562)
11+
12+ -- Utkarsh Gupta <utkarsh.gupta@canonical.com> Thu, 08 Apr 2021 09:52:01 +0530
13+
14 openldap (2.4.49+dfsg-2ubuntu1.7) focal-security; urgency=medium
15
16 * SECURITY UPDATE: DoS via malicious packet
17diff --git a/debian/patches/ITS-8650-loop-on-incomplete-TLS-handshake.patch b/debian/patches/ITS-8650-loop-on-incomplete-TLS-handshake.patch
18new file mode 100644
19index 0000000..a3efa77
20--- /dev/null
21+++ b/debian/patches/ITS-8650-loop-on-incomplete-TLS-handshake.patch
22@@ -0,0 +1,151 @@
23+From 7cf7aa3141e07fe280609cda6a68dd6380ec49bc Mon Sep 17 00:00:00 2001
24+From: Howard Chu <hyc@openldap.org>
25+Date: Sun, 12 Apr 2020 22:18:51 +0100
26+Subject: [PATCH] ITS#8650 loop on incomplete TLS handshake
27+
28+Always retry ldap_int_tls_connect() if it didn't complete,
29+regardless of blocking or non-blocking socket. Code from
30+ITS#7428 was wrong to only retry for async.
31+---
32+ libraries/libldap/tls2.c | 113 ++++++++++++++++++---------------------
33+ 1 file changed, 53 insertions(+), 60 deletions(-)
34+
35+diff --git a/libraries/libldap/tls2.c b/libraries/libldap/tls2.c
36+index dfccff044a..c1f15cbc18 100644
37+--- a/libraries/libldap/tls2.c
38++++ b/libraries/libldap/tls2.c
39+@@ -892,78 +892,71 @@ ldap_int_tls_start ( LDAP *ld, LDAPConn *conn, LDAPURLDesc *srv )
40+ ld->ld_errno = LDAP_SUCCESS;
41+ ret = ldap_int_tls_connect( ld, conn, host );
42+
43++ /* this mainly only happens for non-blocking io
44++ * but can also happen when the handshake is too
45++ * big for a single network message.
46++ */
47++ while ( ret > 0 ) {
48+ #ifdef LDAP_USE_NON_BLOCKING_TLS
49+- while ( ret > 0 ) { /* this should only happen for non-blocking io */
50+- int wr=0;
51++ if ( async ) {
52++ struct timeval curr_time_tv, delta_tv;
53++ int wr=0;
54+
55+- if ( sb->sb_trans_needs_read ) {
56+- wr=0;
57+- } else if ( sb->sb_trans_needs_write ) {
58+- wr=1;
59+- }
60+- Debug( LDAP_DEBUG_TRACE, "ldap_int_tls_start: ldap_int_tls_connect needs %s\n",
61+- wr ? "write": "read", 0, 0);
62+-
63+- ret = ldap_int_poll( ld, sd, &tv, wr);
64+- if ( ret < 0 ) {
65+- ld->ld_errno = LDAP_TIMEOUT;
66+- break;
67+- } else {
68+- /* ldap_int_poll called ldap_pvt_ndelay_off if not async */
69+- if ( !async ) {
70+- ber_sockbuf_ctrl( sb, LBER_SB_OPT_SET_NONBLOCK, (void*)1 );
71++ if ( sb->sb_trans_needs_read ) {
72++ wr=0;
73++ } else if ( sb->sb_trans_needs_write ) {
74++ wr=1;
75+ }
76+- ret = ldap_int_tls_connect( ld, conn, host );
77+- if ( ret > 0 ) { /* need to call tls_connect once more */
78+- struct timeval curr_time_tv, delta_tv;
79++ Debug1( LDAP_DEBUG_TRACE, "ldap_int_tls_start: ldap_int_tls_connect needs %s\n",
80++ wr ? "write": "read" );
81+
82+- /* This is mostly copied from result.c:wait4msg(), should
83+- * probably be moved into a separate function */
84++ /* This is mostly copied from result.c:wait4msg(), should
85++ * probably be moved into a separate function */
86+ #ifdef HAVE_GETTIMEOFDAY
87+- gettimeofday( &curr_time_tv, NULL );
88++ gettimeofday( &curr_time_tv, NULL );
89+ #else /* ! HAVE_GETTIMEOFDAY */
90+- time( &curr_time_tv.tv_sec );
91+- curr_time_tv.tv_usec = 0;
92++ time( &curr_time_tv.tv_sec );
93++ curr_time_tv.tv_usec = 0;
94+ #endif /* ! HAVE_GETTIMEOFDAY */
95+
96+- /* delta = curr - start */
97+- delta_tv.tv_sec = curr_time_tv.tv_sec - start_time_tv.tv_sec;
98+- delta_tv.tv_usec = curr_time_tv.tv_usec - start_time_tv.tv_usec;
99+- if ( delta_tv.tv_usec < 0 ) {
100+- delta_tv.tv_sec--;
101+- delta_tv.tv_usec += 1000000;
102+- }
103++ /* delta = curr - start */
104++ delta_tv.tv_sec = curr_time_tv.tv_sec - start_time_tv.tv_sec;
105++ delta_tv.tv_usec = curr_time_tv.tv_usec - start_time_tv.tv_usec;
106++ if ( delta_tv.tv_usec < 0 ) {
107++ delta_tv.tv_sec--;
108++ delta_tv.tv_usec += 1000000;
109++ }
110+
111+- /* tv0 < delta ? */
112+- if ( ( tv0.tv_sec < delta_tv.tv_sec ) ||
113+- ( ( tv0.tv_sec == delta_tv.tv_sec ) &&
114+- ( tv0.tv_usec < delta_tv.tv_usec ) ) )
115+- {
116+- ret = -1;
117+- ld->ld_errno = LDAP_TIMEOUT;
118+- break;
119+- } else {
120+- /* timeout -= delta_time */
121+- tv0.tv_sec -= delta_tv.tv_sec;
122+- tv0.tv_usec -= delta_tv.tv_usec;
123+- if ( tv0.tv_usec < 0 ) {
124+- tv0.tv_sec--;
125+- tv0.tv_usec += 1000000;
126+- }
127+- start_time_tv.tv_sec = curr_time_tv.tv_sec;
128+- start_time_tv.tv_usec = curr_time_tv.tv_usec;
129+- }
130+- tv = tv0;
131+- Debug( LDAP_DEBUG_TRACE, "ldap_int_tls_start: ld %p %ld s %ld us to go\n",
132+- (void *)ld, (long) tv.tv_sec, (long) tv.tv_usec );
133++ /* tv0 < delta ? */
134++ if ( ( tv0.tv_sec < delta_tv.tv_sec ) ||
135++ ( ( tv0.tv_sec == delta_tv.tv_sec ) &&
136++ ( tv0.tv_usec < delta_tv.tv_usec ) ) )
137++ {
138++ ret = -1;
139++ ld->ld_errno = LDAP_TIMEOUT;
140++ break;
141++ }
142++ /* timeout -= delta_time */
143++ tv0.tv_sec -= delta_tv.tv_sec;
144++ tv0.tv_usec -= delta_tv.tv_usec;
145++ if ( tv0.tv_usec < 0 ) {
146++ tv0.tv_sec--;
147++ tv0.tv_usec += 1000000;
148++ }
149++ start_time_tv.tv_sec = curr_time_tv.tv_sec;
150++ start_time_tv.tv_usec = curr_time_tv.tv_usec;
151++ tv = tv0;
152++ Debug3( LDAP_DEBUG_TRACE, "ldap_int_tls_start: ld %p %ld s %ld us to go\n",
153++ (void *)ld, (long) tv.tv_sec, (long) tv.tv_usec );
154++ ret = ldap_int_poll( ld, sd, &tv, wr);
155++ if ( ret < 0 ) {
156++ ld->ld_errno = LDAP_TIMEOUT;
157++ break;
158+ }
159+ }
160+- }
161+- /* Leave it nonblocking if async */
162+- if ( !async && ld->ld_options.ldo_tm_net.tv_sec >= 0 ) {
163+- ber_sockbuf_ctrl( sb, LBER_SB_OPT_SET_NONBLOCK, NULL );
164+- }
165+ #endif /* LDAP_USE_NON_BLOCKING_TLS */
166++ ret = ldap_int_tls_connect( ld, conn, host );
167++ }
168+
169+ if ( ret < 0 ) {
170+ if ( ld->ld_errno == LDAP_SUCCESS )
171+--
172+2.20.1
173+
174diff --git a/debian/patches/series b/debian/patches/series
175index da7a415..cb35faa 100644
176--- a/debian/patches/series
177+++ b/debian/patches/series
178@@ -42,3 +42,4 @@ CVE-2020-36228.patch
179 CVE-2020-36230.patch
180 CVE-2020-36229.patch
181 CVE-2021-27212.patch
182+ITS-8650-loop-on-incomplete-TLS-handshake.patch

Subscribers

People subscribed via source and target branches