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

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

Subscribers

People subscribed via source and target branches