diff options
25 files changed, 232 insertions, 186 deletions
diff --git a/src/DotNetOpenAuth.Test/App.config b/src/DotNetOpenAuth.Test/App.config index 38fcc7f..2182312 100644 --- a/src/DotNetOpenAuth.Test/App.config +++ b/src/DotNetOpenAuth.Test/App.config @@ -31,7 +31,7 @@ </relyingParty> <provider> <!--<store type=""/>--> - <security protectDownlevelReplayAttacks="false" minimumHashBitLength="7" maximumHashBitLength="302"> + <security protectDownlevelReplayAttacks="true" minimumHashBitLength="7" maximumHashBitLength="302"> <associations> <add type="HMAC-SHA1" lifetime="2.00:00:02" /> <add type="HMAC-SHA256" lifetime="14.00:00:14" /> diff --git a/src/DotNetOpenAuth.Test/Configuration/SectionTests.cs b/src/DotNetOpenAuth.Test/Configuration/SectionTests.cs index b49ada2..73aad6d 100644 --- a/src/DotNetOpenAuth.Test/Configuration/SectionTests.cs +++ b/src/DotNetOpenAuth.Test/Configuration/SectionTests.cs @@ -50,7 +50,7 @@ namespace DotNetOpenAuth.Test.Configuration { var op = DotNetOpenAuthSection.Configuration.OpenId.Provider; Assert.IsNull(op.ApplicationStore.CustomType); - Assert.IsFalse(op.SecuritySettings.ProtectDownlevelReplayAttacks); + Assert.IsTrue(op.SecuritySettings.ProtectDownlevelReplayAttacks); Assert.AreEqual(7, op.SecuritySettings.MinimumHashBitLength); Assert.AreEqual(302, op.SecuritySettings.MaximumHashBitLength); diff --git a/src/DotNetOpenAuth.Test/Messaging/Bindings/StandardReplayProtectionBindingElementTests.cs b/src/DotNetOpenAuth.Test/Messaging/Bindings/StandardReplayProtectionBindingElementTests.cs index e5710c3..eaf59e0 100644 --- a/src/DotNetOpenAuth.Test/Messaging/Bindings/StandardReplayProtectionBindingElementTests.cs +++ b/src/DotNetOpenAuth.Test/Messaging/Bindings/StandardReplayProtectionBindingElementTests.cs @@ -40,13 +40,13 @@ namespace DotNetOpenAuth.Test.Messaging.Bindings { /// </summary> [TestMethod] public void RandomCharactersTest() { - Assert.IsTrue(this.nonceElement.PrepareMessageForSending(this.message)); + Assert.IsNotNull(this.nonceElement.PrepareMessageForSending(this.message)); Assert.IsNotNull(this.message.Nonce, "No nonce was set on the message."); Assert.AreNotEqual(0, this.message.Nonce.Length, "The generated nonce was empty."); string firstNonce = this.message.Nonce; // Apply another nonce and verify that they are different than the first ones. - Assert.IsTrue(this.nonceElement.PrepareMessageForSending(this.message)); + Assert.IsNotNull(this.nonceElement.PrepareMessageForSending(this.message)); Assert.IsNotNull(this.message.Nonce, "No nonce was set on the message."); Assert.AreNotEqual(0, this.message.Nonce.Length, "The generated nonce was empty."); Assert.AreNotEqual(firstNonce, this.message.Nonce, "The two generated nonces are identical."); @@ -58,7 +58,7 @@ namespace DotNetOpenAuth.Test.Messaging.Bindings { [TestMethod] public void ValidMessageReceivedTest() { this.message.Nonce = "a"; - Assert.IsTrue(this.nonceElement.PrepareMessageForReceiving(this.message)); + Assert.IsNotNull(this.nonceElement.PrepareMessageForReceiving(this.message)); } /// <summary> @@ -68,7 +68,7 @@ namespace DotNetOpenAuth.Test.Messaging.Bindings { public void ValidMessageNoNonceReceivedTest() { this.message.Nonce = string.Empty; this.nonceElement.AllowZeroLengthNonce = true; - Assert.IsTrue(this.nonceElement.PrepareMessageForReceiving(this.message)); + Assert.IsNotNull(this.nonceElement.PrepareMessageForReceiving(this.message)); } /// <summary> @@ -78,7 +78,7 @@ namespace DotNetOpenAuth.Test.Messaging.Bindings { public void InvalidMessageNoNonceReceivedTest() { this.message.Nonce = string.Empty; this.nonceElement.AllowZeroLengthNonce = false; - Assert.IsTrue(this.nonceElement.PrepareMessageForReceiving(this.message)); + Assert.IsNotNull(this.nonceElement.PrepareMessageForReceiving(this.message)); } /// <summary> @@ -87,7 +87,7 @@ namespace DotNetOpenAuth.Test.Messaging.Bindings { [TestMethod, ExpectedException(typeof(ReplayedMessageException))] public void ReplayDetectionTest() { this.message.Nonce = "a"; - Assert.IsTrue(this.nonceElement.PrepareMessageForReceiving(this.message)); + Assert.IsNotNull(this.nonceElement.PrepareMessageForReceiving(this.message)); // Now receive the same message again. This should throw because it's a message replay. this.nonceElement.PrepareMessageForReceiving(this.message); diff --git a/src/DotNetOpenAuth.Test/Mocks/MockReplayProtectionBindingElement.cs b/src/DotNetOpenAuth.Test/Mocks/MockReplayProtectionBindingElement.cs index 11d7e67..bde8380 100644 --- a/src/DotNetOpenAuth.Test/Mocks/MockReplayProtectionBindingElement.cs +++ b/src/DotNetOpenAuth.Test/Mocks/MockReplayProtectionBindingElement.cs @@ -23,17 +23,17 @@ namespace DotNetOpenAuth.Test.Mocks { /// </summary> public Channel Channel { get; set; } - bool IChannelBindingElement.PrepareMessageForSending(IProtocolMessage message) { + MessageProtections? IChannelBindingElement.PrepareMessageForSending(IProtocolMessage message) { var replayMessage = message as IReplayProtectedProtocolMessage; if (replayMessage != null) { replayMessage.Nonce = "someNonce"; - return true; + return MessageProtections.ReplayProtection; } - return false; + return null; } - bool IChannelBindingElement.PrepareMessageForReceiving(IProtocolMessage message) { + MessageProtections? IChannelBindingElement.PrepareMessageForReceiving(IProtocolMessage message) { var replayMessage = message as IReplayProtectedProtocolMessage; if (replayMessage != null) { Assert.AreEqual("someNonce", replayMessage.Nonce, "The nonce didn't serialize correctly, or something"); @@ -42,10 +42,10 @@ namespace DotNetOpenAuth.Test.Mocks { throw new ReplayedMessageException(message); } this.messageReceived = true; - return true; + return MessageProtections.ReplayProtection; } - return false; + return null; } #endregion diff --git a/src/DotNetOpenAuth.Test/Mocks/MockSigningBindingElement.cs b/src/DotNetOpenAuth.Test/Mocks/MockSigningBindingElement.cs index 7056d2c..803e471 100644 --- a/src/DotNetOpenAuth.Test/Mocks/MockSigningBindingElement.cs +++ b/src/DotNetOpenAuth.Test/Mocks/MockSigningBindingElement.cs @@ -26,26 +26,26 @@ namespace DotNetOpenAuth.Test.Mocks { /// </summary> public Channel Channel { get; set; } - bool IChannelBindingElement.PrepareMessageForSending(IProtocolMessage message) { + MessageProtections? IChannelBindingElement.PrepareMessageForSending(IProtocolMessage message) { ITamperResistantProtocolMessage signedMessage = message as ITamperResistantProtocolMessage; if (signedMessage != null) { signedMessage.Signature = MessageSignature; - return true; + return MessageProtections.TamperProtection; } - return false; + return null; } - bool IChannelBindingElement.PrepareMessageForReceiving(IProtocolMessage message) { + MessageProtections? IChannelBindingElement.PrepareMessageForReceiving(IProtocolMessage message) { ITamperResistantProtocolMessage signedMessage = message as ITamperResistantProtocolMessage; if (signedMessage != null) { if (signedMessage.Signature != MessageSignature) { throw new InvalidSignatureException(message); } - return true; + return MessageProtections.TamperProtection; } - return false; + return null; } #endregion diff --git a/src/DotNetOpenAuth.Test/Mocks/MockTransformationBindingElement.cs b/src/DotNetOpenAuth.Test/Mocks/MockTransformationBindingElement.cs index 7a69a2d..4006f65 100644 --- a/src/DotNetOpenAuth.Test/Mocks/MockTransformationBindingElement.cs +++ b/src/DotNetOpenAuth.Test/Mocks/MockTransformationBindingElement.cs @@ -34,25 +34,25 @@ namespace DotNetOpenAuth.Test.Mocks { /// </summary> public Channel Channel { get; set; } - bool IChannelBindingElement.PrepareMessageForSending(IProtocolMessage message) { + MessageProtections? IChannelBindingElement.PrepareMessageForSending(IProtocolMessage message) { var testMessage = message as TestMessage; if (testMessage != null) { testMessage.Name = this.transform + testMessage.Name; - return true; + return MessageProtections.None; } - return false; + return null; } - bool IChannelBindingElement.PrepareMessageForReceiving(IProtocolMessage message) { + MessageProtections? IChannelBindingElement.PrepareMessageForReceiving(IProtocolMessage message) { var testMessage = message as TestMessage; if (testMessage != null) { StringAssert.StartsWith(testMessage.Name, this.transform); testMessage.Name = testMessage.Name.Substring(this.transform.Length); - return true; + return MessageProtections.None; } - return false; + return null; } #endregion diff --git a/src/DotNetOpenAuth.Test/OAuth/ChannelElements/PlaintextSigningBindingElementTest.cs b/src/DotNetOpenAuth.Test/OAuth/ChannelElements/PlaintextSigningBindingElementTest.cs index 22b6243..3447ddb 100644 --- a/src/DotNetOpenAuth.Test/OAuth/ChannelElements/PlaintextSigningBindingElementTest.cs +++ b/src/DotNetOpenAuth.Test/OAuth/ChannelElements/PlaintextSigningBindingElementTest.cs @@ -20,7 +20,7 @@ namespace DotNetOpenAuth.Test.ChannelElements ITamperResistantOAuthMessage message = new UnauthorizedTokenRequest(endpoint); message.ConsumerSecret = "cs"; message.TokenSecret = "ts"; - Assert.IsTrue(target.PrepareMessageForSending(message)); + Assert.IsNotNull(target.PrepareMessageForSending(message)); Assert.AreEqual("PLAINTEXT", message.SignatureMethod); Assert.AreEqual("cs&ts", message.Signature); } @@ -34,7 +34,7 @@ namespace DotNetOpenAuth.Test.ChannelElements message.TokenSecret = "ts"; message.SignatureMethod = "PLAINTEXT"; message.Signature = "cs&ts"; - Assert.IsTrue(target.PrepareMessageForReceiving(message)); + Assert.IsNotNull(target.PrepareMessageForReceiving(message)); } [TestMethod] @@ -46,7 +46,7 @@ namespace DotNetOpenAuth.Test.ChannelElements message.TokenSecret = "ts"; message.SignatureMethod = "ANOTHERALGORITHM"; message.Signature = "somethingelse"; - Assert.IsFalse(target.PrepareMessageForReceiving(message), "PLAINTEXT binding element should opt-out where it doesn't understand."); + Assert.AreEqual(MessageProtections.None, target.PrepareMessageForReceiving(message), "PLAINTEXT binding element should opt-out where it doesn't understand."); } [TestMethod] @@ -58,7 +58,7 @@ namespace DotNetOpenAuth.Test.ChannelElements message.TokenSecret = "ts"; // Since this is (non-encrypted) HTTP, so the plain text signer should not be used - Assert.IsFalse(target.PrepareMessageForSending(message)); + Assert.IsNull(target.PrepareMessageForSending(message)); Assert.IsNull(message.SignatureMethod); Assert.IsNull(message.Signature); } @@ -72,7 +72,7 @@ namespace DotNetOpenAuth.Test.ChannelElements message.TokenSecret = "ts"; message.SignatureMethod = "PLAINTEXT"; message.Signature = "cs%26ts"; - Assert.IsFalse(target.PrepareMessageForReceiving(message), "PLAINTEXT signature binding element should refuse to participate in non-encrypted messages."); + Assert.IsNull(target.PrepareMessageForReceiving(message), "PLAINTEXT signature binding element should refuse to participate in non-encrypted messages."); } } } diff --git a/src/DotNetOpenAuth.Test/OpenId/AuthenticationTests.cs b/src/DotNetOpenAuth.Test/OpenId/AuthenticationTests.cs index 82b512a..0167ecd 100644 --- a/src/DotNetOpenAuth.Test/OpenId/AuthenticationTests.cs +++ b/src/DotNetOpenAuth.Test/OpenId/AuthenticationTests.cs @@ -104,13 +104,12 @@ namespace DotNetOpenAuth.Test.OpenId { // notice the replay, we can get one of two exceptions thrown. // When the OP notices the replay we get a generic InvalidSignatureException. // When the RP notices the replay we get a specific ReplayMessageException. - Type expectedExceptionType = sharedAssociation || protocol.Version.Major < 2 ? typeof(ReplayedMessageException) : typeof(InvalidSignatureException); try { CoordinatingChannel channel = (CoordinatingChannel)rp.Channel; channel.Replay(response); - Assert.Fail("Expected exception {0} was not thrown.", expectedExceptionType.Name); + Assert.Fail("Expected ProtocolException was not thrown."); } catch (ProtocolException ex) { - Assert.IsInstanceOfType(ex, expectedExceptionType); + Assert.IsTrue(ex is ReplayedMessageException || ex is InvalidSignatureException, "A {0} exception was thrown instead of the expected {1} or {2}.", ex.GetType(), typeof(ReplayedMessageException).Name, typeof(InvalidSignatureException).Name); } } } else { diff --git a/src/DotNetOpenAuth.Test/OpenId/ChannelElements/ExtensionsBindingElementTests.cs b/src/DotNetOpenAuth.Test/OpenId/ChannelElements/ExtensionsBindingElementTests.cs index 948cd1a..764e830 100644 --- a/src/DotNetOpenAuth.Test/OpenId/ChannelElements/ExtensionsBindingElementTests.cs +++ b/src/DotNetOpenAuth.Test/OpenId/ChannelElements/ExtensionsBindingElementTests.cs @@ -61,13 +61,13 @@ namespace DotNetOpenAuth.Test.OpenId.ChannelElements { [TestMethod] public void PrepareMessageForSendingNonExtendableMessage() { IProtocolMessage request = new AssociateDiffieHellmanRequest(Protocol.Default.Version, OpenIdTestBase.OPUri); - Assert.IsFalse(this.rpElement.PrepareMessageForSending(request)); + Assert.IsNull(this.rpElement.PrepareMessageForSending(request)); } [TestMethod] public void PrepareMessageForSending() { this.request.Extensions.Add(new MockOpenIdExtension("part", "extra")); - Assert.IsTrue(this.rpElement.PrepareMessageForSending(this.request)); + Assert.IsNotNull(this.rpElement.PrepareMessageForSending(this.request)); string alias = GetAliases(this.request.ExtraData).Single(); Assert.AreEqual(MockOpenIdExtension.MockTypeUri, this.request.ExtraData["openid.ns." + alias]); @@ -80,7 +80,7 @@ namespace DotNetOpenAuth.Test.OpenId.ChannelElements { this.request.ExtraData["openid.ns.mock"] = MockOpenIdExtension.MockTypeUri; this.request.ExtraData["openid.mock.Part"] = "part"; this.request.ExtraData["openid.mock.data"] = "extra"; - Assert.IsTrue(this.rpElement.PrepareMessageForReceiving(this.request)); + Assert.IsNotNull(this.rpElement.PrepareMessageForReceiving(this.request)); MockOpenIdExtension ext = this.request.Extensions.OfType<MockOpenIdExtension>().Single(); Assert.AreEqual("part", ext.Part); Assert.AreEqual("extra", ext.Data); diff --git a/src/DotNetOpenAuth.Test/OpenId/ChannelElements/SigningBindingElementTests.cs b/src/DotNetOpenAuth.Test/OpenId/ChannelElements/SigningBindingElementTests.cs index 78b96e6..6b99541 100644 --- a/src/DotNetOpenAuth.Test/OpenId/ChannelElements/SigningBindingElementTests.cs +++ b/src/DotNetOpenAuth.Test/OpenId/ChannelElements/SigningBindingElementTests.cs @@ -35,7 +35,7 @@ namespace DotNetOpenAuth.Test.OpenId.ChannelElements { message.ProviderEndpoint = new Uri("http://provider"); signedMessage.UtcCreationDate = DateTime.Parse("1/1/2009"); signedMessage.AssociationHandle = association.Handle; - Assert.IsTrue(signer.PrepareMessageForSending(message)); + Assert.IsNotNull(signer.PrepareMessageForSending(message)); Assert.AreEqual("0wOdvNgzCZ5I5AzbU58Nq2Tg8EJZ7QoNz4gpx2r7jII=", signedMessage.Signature); } @@ -53,7 +53,7 @@ namespace DotNetOpenAuth.Test.OpenId.ChannelElements { response.ExtraData["someunsigned"] = "value"; response.ExtraData["openid.somesigned"] = "value"; - Assert.IsTrue(sbe.PrepareMessageForSending(response)); + Assert.IsNotNull(sbe.PrepareMessageForSending(response)); ITamperResistantOpenIdMessage signedResponse = (ITamperResistantOpenIdMessage)response; // Make sure that the extra parameters are signed. diff --git a/src/DotNetOpenAuth/Configuration/ProviderSecuritySettingsElement.cs b/src/DotNetOpenAuth/Configuration/ProviderSecuritySettingsElement.cs index 8181a236..11b4859 100644 --- a/src/DotNetOpenAuth/Configuration/ProviderSecuritySettingsElement.cs +++ b/src/DotNetOpenAuth/Configuration/ProviderSecuritySettingsElement.cs @@ -58,10 +58,10 @@ namespace DotNetOpenAuth.Configuration { } /// <summary> - /// Gets or sets a value indicating whether the Provider should take special care to protect OpenID 1.x relying parties - /// against replay attacks. + /// Gets or sets a value indicating whether the Provider should take special care + /// to protect OpenID 1.x relying parties against replay attacks. /// </summary> - [ConfigurationProperty(ProtectDownlevelReplayAttacksConfigName, DefaultValue = false)] + [ConfigurationProperty(ProtectDownlevelReplayAttacksConfigName, DefaultValue = ProviderSecuritySettings.ProtectDownlevelReplayAttacksDefault)] public bool ProtectDownlevelReplayAttacks { get { return (bool)this[ProtectDownlevelReplayAttacksConfigName]; } set { this[ProtectDownlevelReplayAttacksConfigName] = value; } diff --git a/src/DotNetOpenAuth/Messaging/Bindings/StandardExpirationBindingElement.cs b/src/DotNetOpenAuth/Messaging/Bindings/StandardExpirationBindingElement.cs index 2722881..f37bef7 100644 --- a/src/DotNetOpenAuth/Messaging/Bindings/StandardExpirationBindingElement.cs +++ b/src/DotNetOpenAuth/Messaging/Bindings/StandardExpirationBindingElement.cs @@ -55,34 +55,26 @@ namespace DotNetOpenAuth.Messaging.Bindings { /// Sets the timestamp on an outgoing message. /// </summary> /// <param name="message">The outgoing message.</param> - /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False otherwise. - /// </returns> - public bool PrepareMessageForSending(IProtocolMessage message) { + public MessageProtections? PrepareMessageForSending(IProtocolMessage message) { IExpiringProtocolMessage expiringMessage = message as IExpiringProtocolMessage; if (expiringMessage != null) { expiringMessage.UtcCreationDate = DateTime.UtcNow; - return true; + return MessageProtections.Expiration; } - return false; + return null; } /// <summary> /// Reads the timestamp on a message and throws an exception if the message is too old. /// </summary> /// <param name="message">The incoming message.</param> - /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False if the operation did not apply to this message. - /// </returns> /// <exception cref="ExpiredMessageException">Thrown if the given message has already expired.</exception> /// <exception cref="ProtocolException"> /// Thrown when the binding element rules indicate that this message is invalid and should /// NOT be processed. /// </exception> - public bool PrepareMessageForReceiving(IProtocolMessage message) { + public MessageProtections? PrepareMessageForReceiving(IProtocolMessage message) { IExpiringProtocolMessage expiringMessage = message as IExpiringProtocolMessage; if (expiringMessage != null) { // Yes the UtcCreationDate is supposed to always be in UTC already, @@ -92,10 +84,10 @@ namespace DotNetOpenAuth.Messaging.Bindings { throw new ExpiredMessageException(expirationDate, expiringMessage); } - return true; + return MessageProtections.Expiration; } - return false; + return null; } #endregion diff --git a/src/DotNetOpenAuth/Messaging/Bindings/StandardReplayProtectionBindingElement.cs b/src/DotNetOpenAuth/Messaging/Bindings/StandardReplayProtectionBindingElement.cs index 8061a25..0c320e8 100644 --- a/src/DotNetOpenAuth/Messaging/Bindings/StandardReplayProtectionBindingElement.cs +++ b/src/DotNetOpenAuth/Messaging/Bindings/StandardReplayProtectionBindingElement.cs @@ -100,27 +100,22 @@ namespace DotNetOpenAuth.Messaging.Bindings { /// Applies a nonce to the message. /// </summary> /// <param name="message">The message to apply replay protection to.</param> - /// <returns>True if the message protection was applied. False otherwise.</returns> - public bool PrepareMessageForSending(IProtocolMessage message) { + public MessageProtections? PrepareMessageForSending(IProtocolMessage message) { IReplayProtectedProtocolMessage nonceMessage = message as IReplayProtectedProtocolMessage; if (nonceMessage != null) { nonceMessage.Nonce = this.GenerateUniqueFragment(); - return true; + return MessageProtections.ReplayProtection; } - return false; + return null; } /// <summary> /// Verifies that the nonce in an incoming message has not been seen before. /// </summary> /// <param name="message">The incoming message.</param> - /// <returns> - /// True if the message nonce passed replay detection checks. - /// False if the message did not have a nonce that could be checked at all. - /// </returns> /// <exception cref="ReplayedMessageException">Thrown when the nonce check revealed a replayed message.</exception> - public bool PrepareMessageForReceiving(IProtocolMessage message) { + public MessageProtections? PrepareMessageForReceiving(IProtocolMessage message) { IReplayProtectedProtocolMessage nonceMessage = message as IReplayProtectedProtocolMessage; if (nonceMessage != null && nonceMessage.Nonce != null) { ErrorUtilities.VerifyProtocol(nonceMessage.Nonce.Length > 0 || this.AllowZeroLengthNonce, MessagingStrings.InvalidNonceReceived); @@ -129,10 +124,10 @@ namespace DotNetOpenAuth.Messaging.Bindings { throw new ReplayedMessageException(message); } - return true; + return MessageProtections.ReplayProtection; } - return false; + return null; } #endregion diff --git a/src/DotNetOpenAuth/Messaging/Channel.cs b/src/DotNetOpenAuth/Messaging/Channel.cs index 719cb2c..1e27061 100644 --- a/src/DotNetOpenAuth/Messaging/Channel.cs +++ b/src/DotNetOpenAuth/Messaging/Channel.cs @@ -643,13 +643,14 @@ namespace DotNetOpenAuth.Messaging { MessageProtections appliedProtection = MessageProtections.None; foreach (IChannelBindingElement bindingElement in this.outgoingBindingElements) { - if (bindingElement.PrepareMessageForSending(message)) { + MessageProtections? elementProtection = bindingElement.PrepareMessageForSending(message); + if(elementProtection.HasValue) { Logger.DebugFormat("Binding element {0} applied to message.", bindingElement.GetType().FullName); // Ensure that only one protection binding element applies to this message // for each protection type. - ErrorUtilities.VerifyProtocol((appliedProtection & bindingElement.Protection) == 0, MessagingStrings.TooManyBindingsOfferingSameProtection, bindingElement.Protection); - appliedProtection |= bindingElement.Protection; + ErrorUtilities.VerifyProtocol((appliedProtection & elementProtection.Value) == 0, MessagingStrings.TooManyBindingsOfferingSameProtection, elementProtection.Value); + appliedProtection |= elementProtection.Value; } else { Logger.DebugFormat("Binding element {0} did not apply to message.", bindingElement.GetType().FullName); } @@ -756,13 +757,23 @@ namespace DotNetOpenAuth.Messaging { MessageProtections appliedProtection = MessageProtections.None; foreach (IChannelBindingElement bindingElement in this.incomingBindingElements) { - if (bindingElement.PrepareMessageForReceiving(message)) { + MessageProtections? elementProtection = bindingElement.PrepareMessageForReceiving(message); + if (elementProtection.HasValue) { Logger.DebugFormat("Binding element {0} applied to message.", bindingElement.GetType().FullName); // Ensure that only one protection binding element applies to this message // for each protection type. - ErrorUtilities.VerifyInternal((appliedProtection & bindingElement.Protection) == 0, MessagingStrings.TooManyBindingsOfferingSameProtection, bindingElement.Protection); - appliedProtection |= bindingElement.Protection; + if ((appliedProtection & elementProtection.Value) != 0) { + // It turns out that this MAY not be a fatal error condition. + // But it may indicate a problem. + // Specifically, when this RP uses OpenID 1.x to talk to an OP, and both invent + // their own replay protection for OpenID 1.x, and the OP happens to reuse + // openid.response_nonce, then this RP may consider both the RP's own nonce and + // the OP's nonce and "apply" replay protection twice. This actually isn't a problem. + Logger.WarnFormat(MessagingStrings.TooManyBindingsOfferingSameProtection, elementProtection.Value); + } + + appliedProtection |= elementProtection.Value; } else { Logger.DebugFormat("Binding element {0} did not apply to message.", bindingElement.GetType().FullName); } diff --git a/src/DotNetOpenAuth/Messaging/IChannelBindingElement.cs b/src/DotNetOpenAuth/Messaging/IChannelBindingElement.cs index 7aa86fd..0405ebc 100644 --- a/src/DotNetOpenAuth/Messaging/IChannelBindingElement.cs +++ b/src/DotNetOpenAuth/Messaging/IChannelBindingElement.cs @@ -24,8 +24,11 @@ namespace DotNetOpenAuth.Messaging { Channel Channel { get; set; } /// <summary> - /// Gets the protection offered (if any) by this binding element. + /// Gets the protection commonly offered (if any) by this binding element. /// </summary> + /// <remarks> + /// This value is used to assist in sorting binding elements in the channel stack. + /// </remarks> MessageProtections Protection { get; } /// <summary> @@ -33,14 +36,14 @@ namespace DotNetOpenAuth.Messaging { /// </summary> /// <param name="message">The message to prepare for sending.</param> /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False otherwise. + /// The protections (if any) that this binding element applied to the message. + /// Null if this binding element did not even apply to this binding element. /// </returns> /// <remarks> /// Implementations that provide message protection must honor the /// <see cref="MessagePartAttribute.RequiredProtection"/> properties where applicable. /// </remarks> - bool PrepareMessageForSending(IProtocolMessage message); + MessageProtections? PrepareMessageForSending(IProtocolMessage message); /// <summary> /// Performs any transformation on an incoming message that may be necessary and/or @@ -48,8 +51,8 @@ namespace DotNetOpenAuth.Messaging { /// </summary> /// <param name="message">The incoming message to process.</param> /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False if the operation did not apply to this message. + /// The protections (if any) that this binding element applied to the message. + /// Null if this binding element did not even apply to this binding element. /// </returns> /// <exception cref="ProtocolException"> /// Thrown when the binding element rules indicate that this message is invalid and should @@ -59,6 +62,6 @@ namespace DotNetOpenAuth.Messaging { /// Implementations that provide message protection must honor the /// <see cref="MessagePartAttribute.RequiredProtection"/> properties where applicable. /// </remarks> - bool PrepareMessageForReceiving(IProtocolMessage message); + MessageProtections? PrepareMessageForReceiving(IProtocolMessage message); } } diff --git a/src/DotNetOpenAuth/OAuth/ChannelElements/OAuthHttpMethodBindingElement.cs b/src/DotNetOpenAuth/OAuth/ChannelElements/OAuthHttpMethodBindingElement.cs index 8be6c30..afc99d9 100644 --- a/src/DotNetOpenAuth/OAuth/ChannelElements/OAuthHttpMethodBindingElement.cs +++ b/src/DotNetOpenAuth/OAuth/ChannelElements/OAuthHttpMethodBindingElement.cs @@ -37,7 +37,7 @@ namespace DotNetOpenAuth.OAuth.ChannelElements { /// True if the <paramref name="message"/> applied to this binding element /// and the operation was successful. False otherwise. /// </returns> - public bool PrepareMessageForSending(IProtocolMessage message) { + public MessageProtections? PrepareMessageForSending(IProtocolMessage message) { var oauthMessage = message as ITamperResistantOAuthMessage; if (oauthMessage != null) { @@ -47,12 +47,12 @@ namespace DotNetOpenAuth.OAuth.ChannelElements { } else if ((transmissionMethod & HttpDeliveryMethods.GetRequest) != 0) { oauthMessage.HttpMethod = "GET"; } else { - return false; + return null; } - return true; + return MessageProtections.None; } else { - return false; + return null; } } @@ -69,8 +69,8 @@ namespace DotNetOpenAuth.OAuth.ChannelElements { /// Thrown when the binding element rules indicate that this message is invalid and should /// NOT be processed. /// </exception> - public bool PrepareMessageForReceiving(IProtocolMessage message) { - return false; + public MessageProtections? PrepareMessageForReceiving(IProtocolMessage message) { + return null; } #endregion diff --git a/src/DotNetOpenAuth/OAuth/ChannelElements/SigningBindingElementBase.cs b/src/DotNetOpenAuth/OAuth/ChannelElements/SigningBindingElementBase.cs index 6607162..e26e25c 100644 --- a/src/DotNetOpenAuth/OAuth/ChannelElements/SigningBindingElementBase.cs +++ b/src/DotNetOpenAuth/OAuth/ChannelElements/SigningBindingElementBase.cs @@ -73,8 +73,7 @@ namespace DotNetOpenAuth.OAuth.ChannelElements { /// Signs the outgoing message. /// </summary> /// <param name="message">The message to sign.</param> - /// <returns>True if the message was signed. False otherwise.</returns> - public bool PrepareMessageForSending(IProtocolMessage message) { + public MessageProtections? PrepareMessageForSending(IProtocolMessage message) { var signedMessage = message as ITamperResistantOAuthMessage; if (signedMessage != null && this.IsMessageApplicable(signedMessage)) { if (this.SignatureCallback != null) { @@ -86,26 +85,25 @@ namespace DotNetOpenAuth.OAuth.ChannelElements { signedMessage.SignatureMethod = this.signatureMethod; Logger.DebugFormat("Signing {0} message using {1}.", message.GetType().Name, this.signatureMethod); signedMessage.Signature = this.GetSignature(signedMessage); - return true; + return MessageProtections.TamperProtection; } - return false; + return null; } /// <summary> /// Verifies the signature on an incoming message. /// </summary> /// <param name="message">The message whose signature should be verified.</param> - /// <returns>True if the signature was verified. False if the message had no signature.</returns> /// <exception cref="InvalidSignatureException">Thrown if the signature is invalid.</exception> - public bool PrepareMessageForReceiving(IProtocolMessage message) { + public MessageProtections? PrepareMessageForReceiving(IProtocolMessage message) { var signedMessage = message as ITamperResistantOAuthMessage; if (signedMessage != null && this.IsMessageApplicable(signedMessage)) { Logger.DebugFormat("Verifying incoming {0} message signature of: {1}", message.GetType().Name, signedMessage.Signature); if (!string.Equals(signedMessage.SignatureMethod, this.signatureMethod, StringComparison.Ordinal)) { Logger.WarnFormat("Expected signature method '{0}' but received message with a signature method of '{1}'.", this.signatureMethod, signedMessage.SignatureMethod); - return false; + return MessageProtections.None; } if (this.SignatureCallback != null) { @@ -119,10 +117,10 @@ namespace DotNetOpenAuth.OAuth.ChannelElements { throw new InvalidSignatureException(message); } - return true; + return MessageProtections.TamperProtection; } - return false; + return null; } #endregion diff --git a/src/DotNetOpenAuth/OAuth/ChannelElements/SigningBindingElementChain.cs b/src/DotNetOpenAuth/OAuth/ChannelElements/SigningBindingElementChain.cs index 0d0f641..448248f 100644 --- a/src/DotNetOpenAuth/OAuth/ChannelElements/SigningBindingElementChain.cs +++ b/src/DotNetOpenAuth/OAuth/ChannelElements/SigningBindingElementChain.cs @@ -95,18 +95,15 @@ namespace DotNetOpenAuth.OAuth.ChannelElements { /// Prepares a message for sending based on the rules of this channel binding element. /// </summary> /// <param name="message">The message to prepare for sending.</param> - /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False otherwise. - /// </returns> - public bool PrepareMessageForSending(IProtocolMessage message) { + public MessageProtections? PrepareMessageForSending(IProtocolMessage message) { foreach (IChannelBindingElement signer in this.signers) { - if (signer.PrepareMessageForSending(message)) { - return true; + MessageProtections? result = signer.PrepareMessageForSending(message); + if (result.HasValue) { + return result; } } - return false; + return null; } /// <summary> @@ -114,18 +111,15 @@ namespace DotNetOpenAuth.OAuth.ChannelElements { /// validates an incoming message based on the rules of this channel binding element. /// </summary> /// <param name="message">The incoming message to process.</param> - /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False if the operation did not apply to this message. - /// </returns> - public bool PrepareMessageForReceiving(IProtocolMessage message) { + public MessageProtections? PrepareMessageForReceiving(IProtocolMessage message) { foreach (IChannelBindingElement signer in this.signers) { - if (signer.PrepareMessageForReceiving(message)) { - return true; + MessageProtections? result = signer.PrepareMessageForReceiving(message); + if (result.HasValue) { + return result; } } - return false; + return null; } #endregion diff --git a/src/DotNetOpenAuth/OpenId/ChannelElements/BackwardCompatibilityBindingElement.cs b/src/DotNetOpenAuth/OpenId/ChannelElements/BackwardCompatibilityBindingElement.cs index 9d53cc7..81dd0ec 100644 --- a/src/DotNetOpenAuth/OpenId/ChannelElements/BackwardCompatibilityBindingElement.cs +++ b/src/DotNetOpenAuth/OpenId/ChannelElements/BackwardCompatibilityBindingElement.cs @@ -51,15 +51,11 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// Prepares a message for sending based on the rules of this channel binding element. /// </summary> /// <param name="message">The message to prepare for sending.</param> - /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False otherwise. - /// </returns> /// <remarks> /// Implementations that provide message protection must honor the /// <see cref="MessagePartAttribute.RequiredProtection"/> properties where applicable. /// </remarks> - public bool PrepareMessageForSending(IProtocolMessage message) { + public MessageProtections? PrepareMessageForSending(IProtocolMessage message) { SignedResponseRequest request = message as SignedResponseRequest; if (request != null && request.Version.Major < 2) { request.AddReturnToArguments(ProviderEndpointParameterName, request.Recipient.AbsoluteUri); @@ -69,10 +65,10 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { request.AddReturnToArguments(ClaimedIdentifierParameterName, authRequest.ClaimedIdentifier); } - return true; + return MessageProtections.None; } - return false; + return null; } /// <summary> @@ -80,10 +76,6 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// validates an incoming message based on the rules of this channel binding element. /// </summary> /// <param name="message">The incoming message to process.</param> - /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False if the operation did not apply to this message. - /// </returns> /// <exception cref="ProtocolException"> /// Thrown when the binding element rules indicate that this message is invalid and should /// NOT be processed. @@ -92,7 +84,7 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// Implementations that provide message protection must honor the /// <see cref="MessagePartAttribute.RequiredProtection"/> properties where applicable. /// </remarks> - public bool PrepareMessageForReceiving(IProtocolMessage message) { + public MessageProtections? PrepareMessageForReceiving(IProtocolMessage message) { IndirectSignedResponse response = message as IndirectSignedResponse; if (response != null && response.Version.Major < 2) { // Although GetReturnToArgument may return null if the parameters are not signed, @@ -113,10 +105,10 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { } } - return true; + return MessageProtections.None; } - return false; + return null; } #endregion diff --git a/src/DotNetOpenAuth/OpenId/ChannelElements/ExtensionsBindingElement.cs b/src/DotNetOpenAuth/OpenId/ChannelElements/ExtensionsBindingElement.cs index e0e60a7..3132a84 100644 --- a/src/DotNetOpenAuth/OpenId/ChannelElements/ExtensionsBindingElement.cs +++ b/src/DotNetOpenAuth/OpenId/ChannelElements/ExtensionsBindingElement.cs @@ -85,15 +85,11 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// Prepares a message for sending based on the rules of this channel binding element. /// </summary> /// <param name="message">The message to prepare for sending.</param> - /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False otherwise. - /// </returns> /// <remarks> /// Implementations that provide message protection must honor the /// <see cref="MessagePartAttribute.RequiredProtection"/> properties where applicable. /// </remarks> - public bool PrepareMessageForSending(IProtocolMessage message) { + public MessageProtections? PrepareMessageForSending(IProtocolMessage message) { ErrorUtilities.VerifyArgumentNotNull(message, "message"); var extendableMessage = message as IProtocolMessageWithExtensions; @@ -131,10 +127,10 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { // Add the extension parameters to the base message for transmission. extendableMessage.AddExtraParameters(extensionManager.GetArgumentsToSend(includeOpenIdPrefix)); - return true; + return MessageProtections.None; } - return false; + return null; } /// <summary> @@ -142,10 +138,6 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// validates an incoming message based on the rules of this channel binding element. /// </summary> /// <param name="message">The incoming message to process.</param> - /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False if the operation did not apply to this message. - /// </returns> /// <exception cref="ProtocolException"> /// Thrown when the binding element rules indicate that this message is invalid and should /// NOT be processed. @@ -154,7 +146,7 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// Implementations that provide message protection must honor the /// <see cref="MessagePartAttribute.RequiredProtection"/> properties where applicable. /// </remarks> - public bool PrepareMessageForReceiving(IProtocolMessage message) { + public MessageProtections? PrepareMessageForReceiving(IProtocolMessage message) { var extendableMessage = message as IProtocolMessageWithExtensions; if (extendableMessage != null) { // We have a helper class that will do all the heavy-lifting of organizing @@ -183,10 +175,10 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { } } - return true; + return MessageProtections.None; } - return false; + return null; } #endregion diff --git a/src/DotNetOpenAuth/OpenId/ChannelElements/ReturnToNonceBindingElement.cs b/src/DotNetOpenAuth/OpenId/ChannelElements/ReturnToNonceBindingElement.cs index a1b98b8..b6876fa 100644 --- a/src/DotNetOpenAuth/OpenId/ChannelElements/ReturnToNonceBindingElement.cs +++ b/src/DotNetOpenAuth/OpenId/ChannelElements/ReturnToNonceBindingElement.cs @@ -19,6 +19,20 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// in order to protect against replay attacks. /// </summary> /// <remarks> + /// <para>This nonce goes beyond the OpenID 1.x spec, but adds to security. + /// Since this library's Provider implementation also provides special nonce + /// protection for 1.0 messages, this security feature overlaps with that one. + /// This means that if an RP from this library were talking to an OP from this + /// library, but the Identifier being authenticated advertised the OP as a 1.x + /// OP, then both RP and OP might try to use a nonce for protecting the assertion. + /// There's no problem with that--it will still all work out. And it would be a + /// very rare combination of elements anyway. + /// </para> + /// <para> + /// This binding element deactivates itself for OpenID 2.0 (or later) messages + /// since they are automatically protected in the protocol by the Provider's + /// openid.response_nonce parameter. + /// </para> /// <para>In the messaging stack, this binding element looks like an ordinary /// transform-type of binding element rather than a protection element, /// due to its required order in the channel stack and that it exists @@ -29,7 +43,7 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// The parameter of the callback parameter we tack onto the return_to URL /// to store the replay-detection nonce. /// </summary> - private const string NonceParameter = "dnoi.request_nonce"; + internal const string NonceParameter = "dnoi.request_nonce"; /// <summary> /// The length of the generated nonce's random part. @@ -100,24 +114,20 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// Prepares a message for sending based on the rules of this channel binding element. /// </summary> /// <param name="message">The message to prepare for sending.</param> - /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False otherwise. - /// </returns> /// <remarks> /// Implementations that provide message protection must honor the /// <see cref="MessagePartAttribute.RequiredProtection"/> properties where applicable. /// </remarks> - public bool PrepareMessageForSending(IProtocolMessage message) { + public MessageProtections? PrepareMessageForSending(IProtocolMessage message) { // We only add a nonce to 1.x auth requests. SignedResponseRequest request = message as SignedResponseRequest; if (request != null && request.Version.Major < 2) { request.AddReturnToArguments(NonceParameter, CustomNonce.NewNonce().Serialize()); - return true; + return MessageProtections.ReplayProtection; } - return false; + return null; } /// <summary> @@ -125,10 +135,6 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// validates an incoming message based on the rules of this channel binding element. /// </summary> /// <param name="message">The incoming message to process.</param> - /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False if the operation did not apply to this message. - /// </returns> /// <exception cref="ProtocolException"> /// Thrown when the binding element rules indicate that this message is invalid and should /// NOT be processed. @@ -137,7 +143,7 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// Implementations that provide message protection must honor the /// <see cref="MessagePartAttribute.RequiredProtection"/> properties where applicable. /// </remarks> - public bool PrepareMessageForReceiving(IProtocolMessage message) { + public MessageProtections? PrepareMessageForReceiving(IProtocolMessage message) { IndirectSignedResponse response = message as IndirectSignedResponse; if (response != null && response.Version.Major < 2) { string nonceValue = response.GetReturnToArgument(NonceParameter); @@ -153,10 +159,10 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { throw new ReplayedMessageException(message); } - return true; + return MessageProtections.ReplayProtection; } - return false; + return null; } #endregion diff --git a/src/DotNetOpenAuth/OpenId/ChannelElements/ReturnToSignatureBindingElement.cs b/src/DotNetOpenAuth/OpenId/ChannelElements/ReturnToSignatureBindingElement.cs index 147d552..146c9c3 100644 --- a/src/DotNetOpenAuth/OpenId/ChannelElements/ReturnToSignatureBindingElement.cs +++ b/src/DotNetOpenAuth/OpenId/ChannelElements/ReturnToSignatureBindingElement.cs @@ -72,6 +72,10 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// Gets the protection offered (if any) by this binding element. /// </summary> /// <value><see cref="MessageProtections.None"/></value> + /// <remarks> + /// No message protection is reported because this binding element + /// does not protect the entire message -- only a part. + /// </remarks> public MessageProtections Protection { get { return MessageProtections.None; } } @@ -80,23 +84,21 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// Prepares a message for sending based on the rules of this channel binding element. /// </summary> /// <param name="message">The message to prepare for sending.</param> - /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False otherwise. - /// </returns> /// <remarks> /// Implementations that provide message protection must honor the /// <see cref="MessagePartAttribute.RequiredProtection"/> properties where applicable. /// </remarks> - public bool PrepareMessageForSending(IProtocolMessage message) { + public MessageProtections? PrepareMessageForSending(IProtocolMessage message) { SignedResponseRequest request = message as SignedResponseRequest; if (request != null) { request.AddReturnToArguments(ReturnToSignatureHandleParameterName, this.secretManager.CurrentHandle); request.AddReturnToArguments(ReturnToSignatureParameterName, this.GetReturnToSignature(request.ReturnTo)); - return true; + + // We return none because we are not signing the entire message (only a part). + return MessageProtections.None; } - return false; + return null; } /// <summary> @@ -104,10 +106,6 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// validates an incoming message based on the rules of this channel binding element. /// </summary> /// <param name="message">The incoming message to process.</param> - /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False if the operation did not apply to this message. - /// </returns> /// <exception cref="ProtocolException"> /// Thrown when the binding element rules indicate that this message is invalid and should /// NOT be processed. @@ -116,7 +114,7 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// Implementations that provide message protection must honor the /// <see cref="MessagePartAttribute.RequiredProtection"/> properties where applicable. /// </remarks> - public bool PrepareMessageForReceiving(IProtocolMessage message) { + public MessageProtections? PrepareMessageForReceiving(IProtocolMessage message) { IndirectSignedResponse response = message as IndirectSignedResponse; if (response != null) { @@ -135,11 +133,11 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { Logger.WarnFormat("The return_to signature failed verification."); } - return true; + return MessageProtections.None; } } - return false; + return null; } #endregion diff --git a/src/DotNetOpenAuth/OpenId/ChannelElements/SigningBindingElement.cs b/src/DotNetOpenAuth/OpenId/ChannelElements/SigningBindingElement.cs index abaee58..493244a 100644 --- a/src/DotNetOpenAuth/OpenId/ChannelElements/SigningBindingElement.cs +++ b/src/DotNetOpenAuth/OpenId/ChannelElements/SigningBindingElement.cs @@ -18,6 +18,7 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { using DotNetOpenAuth.OpenId.Messages; using DotNetOpenAuth.OpenId.Provider; using DotNetOpenAuth.OpenId.RelyingParty; + using System.Web; /// <summary> /// Signs and verifies authentication assertions. @@ -95,11 +96,7 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// Prepares a message for sending based on the rules of this channel binding element. /// </summary> /// <param name="message">The message to prepare for sending.</param> - /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False otherwise. - /// </returns> - public bool PrepareMessageForSending(IProtocolMessage message) { + public MessageProtections? PrepareMessageForSending(IProtocolMessage message) { var signedMessage = message as ITamperResistantOpenIdMessage; if (signedMessage != null) { Logger.DebugFormat("Signing {0} message.", message.GetType().Name); @@ -107,10 +104,10 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { signedMessage.AssociationHandle = association.Handle; signedMessage.SignedParameterOrder = this.GetSignedParameterOrder(signedMessage); signedMessage.Signature = GetSignature(signedMessage, association); - return true; + return MessageProtections.TamperProtection; } - return false; + return null; } /// <summary> @@ -118,15 +115,11 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { /// validates an incoming message based on the rules of this channel binding element. /// </summary> /// <param name="message">The incoming message to process.</param> - /// <returns> - /// True if the <paramref name="message"/> applied to this binding element - /// and the operation was successful. False if the operation did not apply to this message. - /// </returns> /// <exception cref="ProtocolException"> /// Thrown when the binding element rules indicate that this message is invalid and should /// NOT be processed. /// </exception> - public bool PrepareMessageForReceiving(IProtocolMessage message) { + public MessageProtections? PrepareMessageForReceiving(IProtocolMessage message) { var signedMessage = message as ITamperResistantOpenIdMessage; if (signedMessage != null) { Logger.DebugFormat("Verifying incoming {0} message signature of: {1}", message.GetType().Name, signedMessage.Signature); @@ -161,10 +154,10 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { } } - return true; + return MessageProtections.TamperProtection; } - return false; + return null; } #endregion @@ -271,7 +264,27 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { private Association GetAssociation(ITamperResistantOpenIdMessage signedMessage) { if (this.IsOnProvider) { // We're on a Provider to either sign (smart/dumb) or verify a dumb signature. - return this.GetSpecificAssociation(signedMessage) ?? this.GetDumbAssociationForSigning(); + bool signing = string.IsNullOrEmpty(signedMessage.Signature); + + if (signing) { + // If the RP has no replay protection, coerce use of a private association + // instead of a shared one (if security settings indicate) + // to protect the authenticating user from replay attacks. + bool forcePrivateAssociation = this.opSecuritySettings.ProtectDownlevelReplayAttacks + && this.IsRelyingPartyVulnerableToReplays(null, (IndirectSignedResponse)signedMessage); + + if (forcePrivateAssociation) { + if (!string.IsNullOrEmpty(signedMessage.AssociationHandle)) { + signingLogger.Info("An OpenID 1.x authentication request with a shared association handle will be responded to with a private association in order to provide OP-side replay protection."); + } + + return this.GetDumbAssociationForSigning(); + } else { + return this.GetSpecificAssociation(signedMessage) ?? this.GetDumbAssociationForSigning(); + } + } else { + return this.GetSpecificAssociation(signedMessage); + } } else { // We're on a Relying Party verifying a signature. IDirectedProtocolMessage directedMessage = (IDirectedProtocolMessage)signedMessage; @@ -284,6 +297,47 @@ namespace DotNetOpenAuth.OpenId.ChannelElements { } /// <summary> + /// Determines whether the relying party sending an authentication request is + /// vulnerable to replay attacks. + /// </summary> + /// <param name="request">The request message from the Relying Party. Useful, but may be null for conservative estimate results.</param> + /// <param name="response">The response message to be signed.</param> + /// <returns> + /// <c>true</c> if the relying party is vulnerable; otherwise, <c>false</c>. + /// </returns> + private bool IsRelyingPartyVulnerableToReplays(SignedResponseRequest request, IndirectSignedResponse response) { + ErrorUtilities.VerifyArgumentNotNull(response, "response"); + + // OpenID 2.0 includes replay protection as part of the protocol. + if (response.Version.Major >= 2) { + return false; + } + + // This library's RP may be on the remote end, and may be using 1.x merely because + // discovery on the Claimed Identifier suggested this was a 1.x OP. + // Since this library's RP has a built-in request_nonce parameter for replay + // protection, we'll allow for that. + var returnToArgs = HttpUtility.ParseQueryString(response.ReturnTo.Query); + if (!string.IsNullOrEmpty(returnToArgs[ReturnToNonceBindingElement.NonceParameter])) { + return false; + } + + // If the OP endpoint _AND_ RP return_to URL uses HTTPS then no one + // can steal and replay the positive assertion. + // We can only ascertain this if the request message was handed to us + // so we know what our own OP endpoint is. If we don't have a request + // message, then we'll default to assuming it's insecure. + if (request != null) { + if (request.Recipient.IsTransportSecure() && response.Recipient.IsTransportSecure()) { + return false; + } + } + + // Nothing left to protect against replays. RP is vulnerable. + return true; + } + + /// <summary> /// Gets a specific association referenced in a given message's association handle. /// </summary> /// <param name="signedMessage">The signed message whose association handle should be used to lookup the association to return.</param> diff --git a/src/DotNetOpenAuth/OpenId/Messages/IndirectSignedResponse.cs b/src/DotNetOpenAuth/OpenId/Messages/IndirectSignedResponse.cs index 0ef0aef..2bb6be2 100644 --- a/src/DotNetOpenAuth/OpenId/Messages/IndirectSignedResponse.cs +++ b/src/DotNetOpenAuth/OpenId/Messages/IndirectSignedResponse.cs @@ -251,6 +251,7 @@ namespace DotNetOpenAuth.OpenId.Messages { /// <example>2005-05-15T17:11:51ZUNIQUE</example> [SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Justification = "Called by messaging framework via reflection.")] [MessagePart("openid.response_nonce", IsRequired = true, AllowEmpty = false, RequiredProtection = ProtectionLevel.Sign, MinVersion = "2.0")] + [MessagePart("openid.response_nonce", IsRequired = false, AllowEmpty = false, RequiredProtection = ProtectionLevel.Sign, MaxVersion = "1.1")] private string ResponseNonce { get { string uniqueFragment = ((IReplayProtectedProtocolMessage)this).Nonce; diff --git a/src/DotNetOpenAuth/OpenId/Provider/ProviderSecuritySettings.cs b/src/DotNetOpenAuth/OpenId/Provider/ProviderSecuritySettings.cs index ffdcc64..5930751 100644 --- a/src/DotNetOpenAuth/OpenId/Provider/ProviderSecuritySettings.cs +++ b/src/DotNetOpenAuth/OpenId/Provider/ProviderSecuritySettings.cs @@ -14,6 +14,16 @@ namespace DotNetOpenAuth.OpenId.Provider { /// </summary> public sealed class ProviderSecuritySettings : SecuritySettings { /// <summary> + /// The default value for the <see cref="ProtectDownlevelReplayAttacks"/> property. + /// </summary> + internal const bool ProtectDownlevelReplayAttacksDefault = true; + + /// <summary> + /// The default value for the <see cref="SignOutgoingExtensions"/> property. + /// </summary> + internal const bool SignOutgoingExtensionsDefault = true; + + /// <summary> /// The subset of association types and their customized lifetimes. /// </summary> private IDictionary<string, TimeSpan> associationLifetimes = new Dictionary<string, TimeSpan>(); @@ -23,7 +33,8 @@ namespace DotNetOpenAuth.OpenId.Provider { /// </summary> internal ProviderSecuritySettings() : base(true) { - this.SignOutgoingExtensions = true; + this.SignOutgoingExtensions = SignOutgoingExtensionsDefault; + this.ProtectDownlevelReplayAttacks = ProtectDownlevelReplayAttacksDefault; } /// <summary> @@ -38,12 +49,12 @@ namespace DotNetOpenAuth.OpenId.Provider { /// Gets or sets a value indicating whether OpenID 1.x relying parties that may not be /// protecting their users from replay attacks are protected from /// replay attacks by this provider. - /// *** This property is a placeholder for a feature that has not been written yet. *** /// </summary> + /// <value>The default value is <c>true</c>.</value> /// <remarks> /// <para>Nonces for protection against replay attacks were not mandated /// by OpenID 1.x, which leaves users open to replay attacks.</para> - /// <para>This feature works by preventing associations from being formed + /// <para>This feature works by preventing associations from being used /// with OpenID 1.x relying parties, thereby forcing them into /// "dumb" mode and verifying every claim with this provider. /// This gives the provider an opportunity to verify its own nonce |