[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#706209: marked as done (pu: ejabberd/2.1.10-4+deb7u1)



Your message dated Sat, 12 Oct 2013 19:45:06 +0100
with message-id <1381603506.13031.32.camel@jacala.jungle.funky-badger.org>
and subject line Re: Bug#706209: unblock: ejabberd 2.1.10-5
has caused the Debian Bug report #706209,
regarding pu: ejabberd/2.1.10-4+deb7u1
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
706209: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=706209
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: freeze-exception

Please unblock package ejabberd 2.1.10-5.

It fixes one important bug [1] which prevents certain (correct) XMPP
client implementations (namely, the XMPP library used by git-annex) to
authenticate against the ejabberd server while using the SCRAM SHA-1
SASL authentication mechanism.

The patch has been tested to work OK both with a client which did not
expose the bug (Gajim 0.15.3) and the library which exposed it (the
test case from [2]) so I'm confident it does not break existing clients.

The debdiff between 2.1.10-4 and 2.1.10-5 is attached.

1. http://bugs.debian.org/705613
2. https://support.process-one.net/browse/EJAB-1632
diff -u ejabberd-2.1.10/debian/changelog ejabberd-2.1.10/debian/changelog
--- ejabberd-2.1.10/debian/changelog
+++ ejabberd-2.1.10/debian/changelog
@@ -1,3 +1,12 @@
+ejabberd (2.1.10-5) unstable; urgency=low
+
+  [ Konstantin Khomoutov ]
+  * Add patch fixing parsing of optional parameters in SCRAM SHA-1 headers
+    (closes: #705613, thanks to Stephen Röttger for both writing the
+    original patch and backporting it to 2.1.10).
+
+ -- Konstantin Khomoutov <flatworm@users.sourceforge.net>  Thu, 25 Apr 2013 15:31:59 +0000
+
 ejabberd (2.1.10-4) unstable; urgency=low
 
   [ Konstantin Khomoutov ]
diff -u ejabberd-2.1.10/debian/patches/series ejabberd-2.1.10/debian/patches/series
--- ejabberd-2.1.10/debian/patches/series
+++ ejabberd-2.1.10/debian/patches/series
@@ -9,0 +10 @@
+scram-optional-parameter-parsing-bugfix.patch
only in patch2:
unchanged:
--- ejabberd-2.1.10.orig/debian/patches/scram-optional-parameter-parsing-bugfix.patch
+++ ejabberd-2.1.10/debian/patches/scram-optional-parameter-parsing-bugfix.patch
@@ -0,0 +1,99 @@
+Description: Fix parsing SCRAM optional parameters
+ The server gave an authentication error, if optional parameters
+ were present in the GS2 Header. Specifically, the "a=" parameter,
+ that can be used by admins to login as a different user.
+ .
+ This patch is a backport of changes introduced by the commit
+ 9e9b0eae802ee0508db6780426954efd048e7976 in the upstream Git repository
+ to the ejabberd code base as of version 2.1.10.
+Author: Stephen Röttger <stephen.roettger@gmail.com>
+Forwarded: not-needed
+Bug: https://support.process-one.net/browse/EJAB-1632
+Last-Update: 2013-03-25
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+--- a/src/cyrsasl_scram.erl
++++ b/src/cyrsasl_scram.erl
+@@ -34,6 +34,8 @@
+ 
+ -include("ejabberd.hrl").
+ 
++-include("jlib.hrl").
++
+ -behaviour(cyrsasl).
+ 
+ -record(state, {step, stored_key, server_key, username, get_password, check_password,
+@@ -52,8 +54,12 @@
+     {ok, #state{step = 2, get_password = GetPassword}}.
+ 
+ mech_step(#state{step = 2} = State, ClientIn) ->
+-	case string:tokens(ClientIn, ",") of
+-	[CBind, UserNameAttribute, ClientNonceAttribute] when (CBind == "y") or (CBind == "n") ->
++	case re:split(ClientIn, ",", [{return, list}]) of
++	[_CBind, _AuthorizationIdentity, _UserNameAttribute, _ClientNonceAttribute, ExtensionAttribute | _]
++	when ExtensionAttribute /= [] ->
++		{error, <<"protocol-error-extension-not-supported">>};
++	[CBind, _AuthorizationIdentity, UserNameAttribute, ClientNonceAttribute | _]
++	when (CBind == "y") or (CBind == "n") ->
+ 		case parse_attribute(UserNameAttribute) of
+                 {error, Reason} ->
+ 			{error, Reason};
+@@ -100,32 +106,36 @@
+ 	case string:tokens(ClientIn, ",") of
+ 	[GS2ChannelBindingAttribute, NonceAttribute, ClientProofAttribute] ->
+ 		case parse_attribute(GS2ChannelBindingAttribute) of
+-		{$c, CVal} when (CVal == "biws") or (CVal == "eSws") ->
+-		    %% biws is base64 for n,, => channelbinding not supported
+-		    %% eSws is base64 for y,, => channelbinding supported by client only
+- 			Nonce = State#state.client_nonce ++ State#state.server_nonce,
+-			case parse_attribute(NonceAttribute) of
+-			{$r, CompareNonce} when CompareNonce == Nonce ->
+-				case parse_attribute(ClientProofAttribute) of
+-				{$p, ClientProofB64} ->
+-					ClientProof = base64:decode(ClientProofB64),
+-					AuthMessage = State#state.auth_message ++ "," ++ string:substr(ClientIn, 1, string:str(ClientIn, ",p=")-1),
+-					ClientSignature = scram:client_signature(State#state.stored_key, AuthMessage),
+-					ClientKey = scram:client_key(ClientProof, ClientSignature),
+-					CompareStoredKey = scram:stored_key(ClientKey),
+-					if CompareStoredKey == State#state.stored_key ->
+-						ServerSignature = scram:server_signature(State#state.server_key, AuthMessage),
+-						{ok, [{username, State#state.username}], "v=" ++ base64:encode_to_string(ServerSignature)};
+-					true ->
+-						{error, "bad-auth"}
++		{$c, CVal} ->
++			ChannelBindingSupport = string:left(jlib:decode_base64(CVal), 1),
++			if (ChannelBindingSupport == "n")
++			or (ChannelBindingSupport == "y") ->
++				Nonce = State#state.client_nonce ++ State#state.server_nonce,
++				case parse_attribute(NonceAttribute) of
++				{$r, CompareNonce} when CompareNonce == Nonce ->
++					case parse_attribute(ClientProofAttribute) of
++					{$p, ClientProofB64} ->
++						ClientProof = base64:decode(ClientProofB64),
++						AuthMessage = State#state.auth_message ++ "," ++ string:substr(ClientIn, 1, string:str(ClientIn, ",p=")-1),
++						ClientSignature = scram:client_signature(State#state.stored_key, AuthMessage),
++						ClientKey = scram:client_key(ClientProof, ClientSignature),
++						CompareStoredKey = scram:stored_key(ClientKey),
++						if CompareStoredKey == State#state.stored_key ->
++							ServerSignature = scram:server_signature(State#state.server_key, AuthMessage),
++							{ok, [{username, State#state.username}], "v=" ++ base64:encode_to_string(ServerSignature)};
++						true ->
++							{error, "bad-auth"}
++						end;
++					_Else ->
++						{error, "bad-protocol"}
+ 					end;
++				{$r, _} ->
++					{error, "bad-nonce"};
+ 				_Else ->
+ 					{error, "bad-protocol"}
+ 				end;
+-			{$r, _} ->
+-				{error, "bad-nonce"};
+-			_Else ->
+-				{error, "bad-protocol"}
++			true ->
++				{error, "bad-channel-binding"}
+ 			end;
+ 		_Else ->
+ 	   		{error, "bad-protocol"}

--- End Message ---
--- Begin Message ---
On Tue, 2013-10-08 at 17:51 +0400, Konstantin Khomoutov wrote:
> In addition to the changes introduced in 2.1.10-5, two more bugs have
> been fixed:
> * Disabled SSLv2 and weak cyphers in TLS driver [2].
> * Fixed rendering of angle brackets in logs produced for
>   multi-user chat (MUC) rooms when a plain-text format is enabled for
>   them (resulting in nicknames disappearing from these logs and similar
>   issues) [3].
> 
> I have verified both of these bugfixes work as intended.
> 
> Please see the attached debdiff.  It's a bit large but please notice
> that half of it is the unborn 2.1.10-5.

As far as I can tell, this was all handled via security as 2.1.10-4
+deb7u1; if that's not the case, please explain what's still missing.

Regards,

Adam

--- End Message ---

Reply to: