Bug#590873: openconnect < 2.25 does not verify SSL server certificates

[Adam D. Barratt]

On Mon, 2010-08-30 at 17:34 +0100, Dominic Hargreaves wrote:
> On Sun, Aug 29, 2010 at 10:42:28AM +0100, Adam D. Barratt wrote:
> 
> > I have to admit I'm not hugely happy with the "CSD trojan" messages but,
> > at least in terms of the configuration file setup, I'm not sure it's worth
> > diverging from upstream on.
> > 
> > Please go ahead.
> 
> Uploaded, please unblock openconnect 2.25-0.1

Unblocked.

Regards,

Adam



--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#590873: openconnect < 2.25 does not verify SSL server certificates

[Dominic Hargreaves]

On Sun, Aug 29, 2010 at 10:42:28AM +0100, Adam D. Barratt wrote:

> I have to admit I'm not hugely happy with the "CSD trojan" messages but,
> at least in terms of the configuration file setup, I'm not sure it's worth
> diverging from upstream on.
> 
> Please go ahead.

Uploaded, please unblock openconnect 2.25-0.1

Thanks,
Dominic.

-- 
Dominic Hargreaves | http://www.larted.org.uk/~dom/
PGP key 5178E2A5 from the.earth.li (keyserver,web,email)



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#590873: openconnect < 2.25 does not verify SSL server certificates

[Adam D. Barratt]

On Sat, August 28, 2010 12:59, Dominic Hargreaves wrote:
> On Sat, Aug 28, 2010 at 01:16:29PM +0200, Julien Cristau wrote:
>> On Sat, Aug 28, 2010 at 11:50:49 +0100, Dominic Hargreaves wrote:
>>
>> > On Sun, Aug 15, 2010 at 08:56:46PM +0100, Adam D. Barratt wrote:
>> > > On Sun, 2010-08-15 at 16:13 +0100, Dominic Hargreaves wrote:
>> > > > Given all this, might the best idea be allow an exception for the
>> > > > new upstream? The full changelog is:
>> > >
>> > > Most of the changes sound potentially worthy of inclusion.  What
>> does
>> > > the debdiff look like?
[...]
> The upstream changes are visible at
> 
> and also in the attached debdiff.

I have to admit I'm not hugely happy with the "CSD trojan" messages but,
at least in terms of the configuration file setup, I'm not sure it's worth
diverging from upstream on.

Please go ahead.

Regards,

Adam




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#590873: openconnect < 2.25 does not verify SSL server certificates

[Dominic Hargreaves]

On Sat, Aug 28, 2010 at 01:16:29PM +0200, Julien Cristau wrote:
> On Sat, Aug 28, 2010 at 11:50:49 +0100, Dominic Hargreaves wrote:
> 
> > On Sun, Aug 15, 2010 at 08:56:46PM +0100, Adam D. Barratt wrote:
> > > On Sun, 2010-08-15 at 16:13 +0100, Dominic Hargreaves wrote:
> > > > To the untrained eye, the diff between
> > > > 6732c0e8ccb4d57d6a970973f994a9d2d3509def
> > > > and
> > > > 3b2738befa7fe934d0d55b77fe1fcf28aafbe424
> > > > 
> > > > in upstream git is what's required for this, but the patch would need
> > > > a bit of work to apply cleanly. Note also that there
> > > > are some memory leaks fixed in 2.25 which might be a good idea to fix
> > > > too.
> > > > 
> > > > Given all this, might the best idea be allow an exception for the 
> > > > new upstream? The full changelog is:
> > > 
> > > Most of the changes sound potentially worthy of inclusion.  What does
> > > the debdiff look like?
> > 
> > File lists identical (after any substitutions)
> > 
> > Control files: lines which differ (wdiff format)
> > 
> > Installed-Size: [-196-] {+208+}
> > Version: [-2.22-1.1-] {+2.25-0.1+}
> > 
> The debdiff between both .dscs, not between the .debs.

Ah, it wasn't clear what was required.

> > Trivial interdiff (including reverted patch included upstream)
> > attached.
> > 
> This doesn't seem to be the full story, it has no upstream changes...

The upstream changes are visible at

and also in the attached debdiff.

Cheers,
Dominic.

-- 
Dominic Hargreaves | http://www.larted.org.uk/~dom/
PGP key 5178E2A5 from the.earth.li (keyserver,web,email)
diff -Nru openconnect-2.22/auth.c openconnect-2.25/auth.c
--- openconnect-2.22/auth.c 2010-03-07 22:10:55.0 +
+++ openconnect-2.25/auth.c 2010-05-15 09:23:37.0 +0100
@@ -523,6 +523,7 @@
if (vpninfo->password &&
!strcmp(opt->name, "password")) {
opt->value = strdup(vpninfo->password);
+   vpninfo->password = NULL;
if (!opt->value) {
ret = -ENOMEM;
goto out_ui;
diff -Nru openconnect-2.22/auth-dlg-settings.h 
openconnect-2.25/auth-dlg-settings.h
--- openconnect-2.22/auth-dlg-settings.h2010-03-07 22:10:55.0 
+
+++ openconnect-2.25/auth-dlg-settings.h2010-05-15 09:23:37.0 
+0100
@@ -31,15 +31,11 @@
 #define NM_OPENCONNECT_KEY_GATEWAY "gateway"
 #define NM_OPENCONNECT_KEY_COOKIE "cookie"
 #define NM_OPENCONNECT_KEY_GWCERT "gwcert"
-#define NM_OPENCONNECT_KEY_AUTHTYPE "authtype"
 #define NM_OPENCONNECT_KEY_USERCERT "usercert"
 #define NM_OPENCONNECT_KEY_CACERT "cacert"
 #define NM_OPENCONNECT_KEY_PRIVKEY "userkey"
 #define NM_OPENCONNECT_KEY_USERNAME "username"
 #define NM_OPENCONNECT_KEY_XMLCONFIG "xmlconfig"
 
-#define NM_OPENCONNECT_AUTHTYPE_CERT "cert"
-#define NM_OPENCONNECT_AUTHTYPE_CERT_TPM "cert-tpm"
-#define NM_OPENCONNECT_AUTHTYPE_PASSWORD "password"
 
 #endif /* __OPENCONNECT_AUTH_DLG_SETTINGS_H */
diff -Nru openconnect-2.22/cstp.c openconnect-2.25/cstp.c
--- openconnect-2.22/cstp.c 2010-03-07 22:10:55.0 +
+++ openconnect-2.25/cstp.c 2010-05-15 09:23:37.0 +0100
@@ -84,6 +84,7 @@
vpninfo->vpn_addr6 = vpninfo->vpn_netmask6 = NULL;
vpninfo->cstp_options = vpninfo->dtls_options = NULL;
vpninfo->vpn_domain = vpninfo->vpn_proxy_pac = NULL;
+   vpninfo->banner = NULL;
 
for (i=0; i<3; i++)
vpninfo->vpn_dns[i] = vpninfo->vpn_nbns[i] = NULL;
@@ -248,6 +249,8 @@
vpninfo->vpn_domain = new_option->value;
} else if (!strcmp(buf + 7, "MSIE-Proxy-PAC-URL")) {
vpninfo->vpn_proxy_pac = new_option->value;
+   } else if (!strcmp(buf + 7, "Banner")) {
+   vpninfo->banner = new_option->value;
} else if (!strcmp(buf + 7, "Split-Include")) {
struct split_include *inc = malloc(sizeof(*inc));
if (!inc)
diff -Nru openconnect-2.22/debian/changelog openconnect-2.25/debian/changelog
--- openconnect-2.22/debian/changelog   2010-08-28 12:58:22.0 +0100
+++ openconnect-2.25/debian/changelog   2010-08-28 12:58:23.0 +0100
@@ -1,3 +1,11 @@
+openconnect (2.25-0.1) unstable; urgency=low
+
+  * Non-maintainer upload.
+  * New upstream release (Closes: #566188)
+- always verify SSL server certificates (Closes: #590873)
+
+ -- Dominic Hargreaves   Sat, 28 Aug 2010 11:21:16 +0100
+
 openconnect (2.22-1.1) unstable; urgency=low
 
   * Non-maintainer upload.
diff -Nru openconnect-2.22/dtls.c openconnect-2.25/dtls.c
--- openconnect-2.22/dtls.c 2010-03-07 22:10:55.0 +
+++ openconnect-2.25/dtls.c 2010-05-15 09:23:37.0 +0100
@@ -35,6 +35,19 @@
 
 #

Bug#590873: openconnect < 2.25 does not verify SSL server certificates

[Julien Cristau]

On Sat, Aug 28, 2010 at 11:50:49 +0100, Dominic Hargreaves wrote:

> On Sun, Aug 15, 2010 at 08:56:46PM +0100, Adam D. Barratt wrote:
> > On Sun, 2010-08-15 at 16:13 +0100, Dominic Hargreaves wrote:
> > > To the untrained eye, the diff between
> > > 6732c0e8ccb4d57d6a970973f994a9d2d3509def
> > > and
> > > 3b2738befa7fe934d0d55b77fe1fcf28aafbe424
> > > 
> > > in upstream git is what's required for this, but the patch would need
> > > a bit of work to apply cleanly. Note also that there
> > > are some memory leaks fixed in 2.25 which might be a good idea to fix
> > > too.
> > > 
> > > Given all this, might the best idea be allow an exception for the 
> > > new upstream? The full changelog is:
> > 
> > Most of the changes sound potentially worthy of inclusion.  What does
> > the debdiff look like?
> 
> File lists identical (after any substitutions)
> 
> Control files: lines which differ (wdiff format)
> 
> Installed-Size: [-196-] {+208+}
> Version: [-2.22-1.1-] {+2.25-0.1+}
> 
The debdiff between both .dscs, not between the .debs.

> Trivial interdiff (including reverted patch included upstream)
> attached.
> 
This doesn't seem to be the full story, it has no upstream changes...

Cheers,
Julien


signature.asc
Description: Digital signature

Bug#590873: openconnect < 2.25 does not verify SSL server certificates

[Dominic Hargreaves]

On Sun, Aug 15, 2010 at 08:56:46PM +0100, Adam D. Barratt wrote:
> On Sun, 2010-08-15 at 16:13 +0100, Dominic Hargreaves wrote:
> > To the untrained eye, the diff between
> > 6732c0e8ccb4d57d6a970973f994a9d2d3509def
> > and
> > 3b2738befa7fe934d0d55b77fe1fcf28aafbe424
> > 
> > in upstream git is what's required for this, but the patch would need
> > a bit of work to apply cleanly. Note also that there
> > are some memory leaks fixed in 2.25 which might be a good idea to fix
> > too.
> > 
> > Given all this, might the best idea be allow an exception for the 
> > new upstream? The full changelog is:
> 
> Most of the changes sound potentially worthy of inclusion.  What does
> the debdiff look like?

File lists identical (after any substitutions)

Control files: lines which differ (wdiff format)

Installed-Size: [-196-] {+208+}
Version: [-2.22-1.1-] {+2.25-0.1+}

Trivial interdiff (including reverted patch included upstream)
attached.

I've tested 2.25-0.1 against a Cisco VPN service, and basic
functionality works fine.

Let me know if it's okay to upload.

Cheers,
Dominic.


-- 
Dominic Hargreaves | http://www.larted.org.uk/~dom/
PGP key 5178E2A5 from the.earth.li (keyserver,web,email)
reverted:
--- openconnect-2.22/ssl.c
+++ openconnect-2.22.orig/ssl.c
@@ -33,7 +33,7 @@
 #include 
 #if defined(__linux__)
 #include 
+#elif defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__APPLE__)
-#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || 
defined(__OpenBSD__) || defined(__APPLE__)
 #include 
 #include 
 #elif defined (__sun__)
reverted:
--- openconnect-2.22/version.c
+++ openconnect-2.22.orig/version.c
@@ -1 +1 @@
+char openconnect_version[] = "v2.22";
-char openconnect_version[] = "v2.22-unknown";
diff -u openconnect-2.22/debian/changelog openconnect-2.25/debian/changelog
--- openconnect-2.22/debian/changelog
+++ openconnect-2.25/debian/changelog
@@ -1,3 +1,11 @@
+openconnect (2.25-0.1) unstable; urgency=low
+
+  * Non-maintainer upload.
+  * New upstream release (Closes: #566188)
+- always verify SSL server certificates (Closes: #590873)
+
+ -- Dominic Hargreaves   Sat, 28 Aug 2010 11:21:16 +0100
+
 openconnect (2.22-1.1) unstable; urgency=low
 
   * Non-maintainer upload.

Bug#590873: openconnect < 2.25 does not verify SSL server certificates

[Dominic Hargreaves]

On Sun, Aug 15, 2010 at 08:56:46PM +0100, Adam D. Barratt wrote:
> On Sun, 2010-08-15 at 16:13 +0100, Dominic Hargreaves wrote:
> > To the untrained eye, the diff between
> > 6732c0e8ccb4d57d6a970973f994a9d2d3509def
> > and
> > 3b2738befa7fe934d0d55b77fe1fcf28aafbe424
> > 
> > in upstream git is what's required for this, but the patch would need
> > a bit of work to apply cleanly. Note also that there
> > are some memory leaks fixed in 2.25 which might be a good idea to fix
> > too.
> > 
> > Given all this, might the best idea be allow an exception for the 
> > new upstream? The full changelog is:
> 
> Most of the changes sound potentially worthy of inclusion.  What does
> the debdiff look like?

Hi,

I haven't had a look at this yet (just for context, I'm not the
maintainer - happy for them to speak up :)

I will try and look at this at the weekend though.

Cheers,
Domniic.

-- 
Dominic Hargreaves | http://www.larted.org.uk/~dom/
PGP key 5178E2A5 from the.earth.li (keyserver,web,email)



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#590873: openconnect < 2.25 does not verify SSL server certificates

[Adam D. Barratt]

On Sun, 2010-08-15 at 16:13 +0100, Dominic Hargreaves wrote:
> To the untrained eye, the diff between
> 6732c0e8ccb4d57d6a970973f994a9d2d3509def
> and
> 3b2738befa7fe934d0d55b77fe1fcf28aafbe424
> 
> in upstream git is what's required for this, but the patch would need
> a bit of work to apply cleanly. Note also that there
> are some memory leaks fixed in 2.25 which might be a good idea to fix
> too.
> 
> Given all this, might the best idea be allow an exception for the 
> new upstream? The full changelog is:

Most of the changes sound potentially worthy of inclusion.  What does
the debdiff look like?

Regards,

Adam



--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#590873: openconnect < 2.25 does not verify SSL server certificates

[Dominic Hargreaves]

On Thu, Jul 29, 2010 at 03:45:55PM -0400, Anders Kaseorg wrote:

> Versions of OpenConnect before 2.25 do not verify that the server SSL 
> certificate matches the server hostname, which enables an attacker to 
> perform an MITM attack on the connection.  This can be fixed by upgrading 
> to OpenConnect 2.25.
> 
> From the upstream changelog:
> 
> OpenConnect v2.25 — 2010-05-15
> • Always validate server certificate, even when no extra --cafile is 
>   provided.
> • Add --no-cert-check option to avoid certificate validation.
> • Check server hostname against its certificate.
> • Provide text-mode function for reviewing and accepting "invalid" 
>   certificates.
> • Fix libproxy detection on NetBSD.

To the untrained eye, the diff between
6732c0e8ccb4d57d6a970973f994a9d2d3509def
and
3b2738befa7fe934d0d55b77fe1fcf28aafbe424

in upstream git is what's required for this, but the patch would need
a bit of work to apply cleanly. Note also that there
are some memory leaks fixed in 2.25 which might be a good idea to fix
too.

Given all this, might the best idea be allow an exception for the 
new upstream? The full changelog is:

 * OpenConnect v2.25 -- 2010-05-15
  + Always validate server certificate, even when no extra
--cafile is provided.
  + Add --no-cert-check option to avoid certificate validation.
  + Check server hostname against its certificate.
  + Provide text-mode function for reviewing and accepting
"invalid" certificates.
  + Fix libproxy detection on NetBSD.
 * OpenConnect v2.24 -- 2010-05-07
  + Forget preconfigured password after a single attempt; don't
retry infinitely if it's failing.
  + Set $CISCO_BANNER environment variable when running script.
  + Better handling of passphrase failure on certificate files.
  + Fix NetBSD build (thanks to Pouya D. Tafti).
  + Fix DragonFly BSD build.
 * OpenConnect v2.23 -- 2010-04-09
  + Support "Cisco Secure Desktop" trojan in NetworkManager
auth-dialog.
  + Support proxy in NetworkManager auth-dialog.
  + Add --no-http-keepalive option to work around Cisco's
incompetence.
  + Fix build on Debian/kFreeBSD.
  + Fix crash on receiving HTTP 404 error.
  + Improve workaround for server certificates lacking SSL_SERVER
purpose, so that it also works with OpenSSL older than 0.9.8k.

And upstream git doesn't appear to have any subsequent regression
fixes.

Dominic.

-- 
Dominic Hargreaves | http://www.larted.org.uk/~dom/
PGP key 5178E2A5 from the.earth.li (keyserver,web,email)



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#590873: openconnect < 2.25 does not verify SSL server certificates

[Anders Kaseorg]

Package: openconnect
Version: 2.22-1.1
Severity: grave
Tags: security fixed-upstream

Versions of OpenConnect before 2.25 do not verify that the server SSL 
certificate matches the server hostname, which enables an attacker to 
perform an MITM attack on the connection.  This can be fixed by upgrading 
to OpenConnect 2.25.

From the upstream changelog:

OpenConnect v2.25 — 2010-05-15
• Always validate server certificate, even when no extra --cafile is 
  provided.
• Add --no-cert-check option to avoid certificate validation.
• Check server hostname against its certificate.
• Provide text-mode function for reviewing and accepting "invalid" 
  certificates.
• Fix libproxy detection on NetBSD.



--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org