0

In WPF the Password property of a PasswordBox is not a DependencyProperty so I can't bind to it directly. As a workaround I am using this PasswordHelper from https://www.wpftutorial.net/PasswordBox.html which attaches a PasswordHelper.Password to the PasswordBox so I can bind to it.

To prevent the password from staying in the DataContext as plain text I would like to use a converter which generates a salted hash of the password before saving it to the DataContext. As I need to save both the salt and the salted hash to the DataContext I am using a MultiBinding and a IMultiValueConverter converter StringToSaltedHashConverter.

My problem is that the Password and PasswordSalt properties of my DataContext are not getting updated when I fill the PasswordBox. I have checked with Snoop and both the Password and PasswordHelper.Password properties of the PasswordBoxare changing accordingly to what I type. Also my StringToSaltedHashConverter is being called and I am returning the correct values.

I have some other bindings on this form (username, first name, last name, gender...) and they all work fine. This is the only one that is not updating.

¿Why is my DataContext not getting updated?

XAML:

<PasswordBox x:Name="Password"
             Style="{DynamicResource PasswordBoxStyle1}"
             local:PasswordHelper.Attach="True">
    <local:PasswordHelper.Password>
        <MultiBinding Converter="{StaticResource StringToSaltedHashConverter}"
                      Mode="OneWayToSource">
            <Binding Path="Password" />
            <Binding Path="PasswordSalt" />
        </MultiBinding>
    </local:PasswordHelper.Password>
</PasswordBox>

Code behind:

public class StringToSaltedHashConverter : IMultiValueConverter
{
    public object Convert(object[] values, Type targetType, object parameter, CultureInfo culture)
    {
        throw new NotImplementedException();
    }

    public object[] ConvertBack(object value, Type[] targetTypes, object parameter, CultureInfo culture)
    {
        string str = value as string;
        string salt = Hash.CreateSalt();
        string hash = Hash.CreateHash(str, salt);
        object[] vs = { hash, salt };
        return vs;
    }
}
deiviz
  • 45
  • 1
  • 6
  • What are you going to solve with your converter? Just for having the plain password only in one place in memory (DependencyProperty) - Do you think that is 50% more secure than having it in two places (+DataContext)? – Sir Rufo Feb 14 '20 at 12:38
  • @deiviz: Did you change the type of the `Password` property to `object[]`? You cannot set a `string` property to an array. – mm8 Feb 14 '20 at 13:11
  • @mm8 No, I did not change the type of the `Password` property. The `PasswordHelper.Password` is converted back to two properties of the `DataContext`(`Password` and `PasswordSalt`). That is why there is an `object[]`. – deiviz Feb 26 '20 at 08:04
  • @deiviz: You can't set a `string` propert to an `object[]`. That's your problem. – mm8 Feb 26 '20 at 13:07

3 Answers3

1

This is not the way to go. PasswordHelper class and their likes should be banned from the internet. The PasswordBox.Password property is not bindable for a very good reason which is well documented. Accessing this property will create a plain text string representation which can be effordlessly retrieved with free tools e.g. Microsoft Process Explorer.

When you get the Password property value, you expose the password as plain text in memory. To avoid this potential security risk, use the SecurePassword property to get the password as a SecureString.

Setting this property to null causes the underlying password to be set to Empty.

You even store the plain salt value in memory - I really hope this is neither a commercial nor a public application. You have absolutely no control when the Garbage Collector will remove any value from the memory. Since String is an immutable type, it is very likely that multiple copies of the user's password will remain public in memory.

Recommended authentication in a Windows environment is to use the Windows User Authentication API.

You should at least clear the PasswordBox.Password property, by setting it null.

Encryption should not be done in the view.

And please, write responsible code! User data is not your data!

MainWindow.xaml

<Window>
  <Window.DataContext>
    <ViewModel />
  </Window.DataContext>

  <PasswordBox x:Name="PasswordBox" />
</Window>

MainWindow.xaml.cs

public partial class MainWindow : Window
{
  public MainWindow()
  {
    InitializeComponent();

    this.PasswordBox.PasswordChanged += OnPasswordChanged;
  }

  private void OnPasswordChanged(object sender, RoutedEventArgs e)
  {
    (this.DataContext as ViewModel).Password = this.PasswordBox.SecurePassword;
  }
}

ViewModel.cs

pucblic class ViewModel
{
  private Model Model { get; }

  private SecureString password;
  public SecureString Password
  {
    private get => this.password;
    public set
    {
      this.password = value;
      OnPasswordChanged();
    }
  }

  private void OnPasswordChanged()
  {
    // Hash password in the model e.g. to compare it to a hashed database value
    this.Model.TryLogin(this.Password);
  }
}

Model.cs

public class Model
{
  public bool TryLogin(SecureString password)
  {  
    IntPtr unmanagedString = Marshal.SecureStringToGlobalAllocUnicode(password);
    string hashedPassword = HashPassword(Marshal.PtrToStringUni(unmanagedString));

    return PasswordIsValid(hashedPassword);
  }

  private string HashPassword(string unsecurePassword)
  {
    // Hash algorithm
  }
}
Community
  • 1
  • 1
BionicCode
  • 1
  • 4
  • 28
  • 44
  • Converter direction: with OneWayToSource-Mode then only the ConvertBack method will be called. Why should he implement the other method? – Sir Rufo Feb 14 '20 at 12:48
  • I read it like he's binding the `PasswordBox.Password` property to the evil `PassswordHelper`. The binding direction is wrong since the helper is the target. – BionicCode Feb 14 '20 at 12:50
  • Why would you write back to this property instead of clearing it by setting it to `null`? – BionicCode Feb 14 '20 at 12:51
  • Yes, and the binding direction is OneWayToSource => PasswordHelper.Password property back to the ViewModel property. – Sir Rufo Feb 14 '20 at 12:53
  • Yes, you are right. He didn't even cleared the `PasswordBox`. I will remove the binding hint. – BionicCode Feb 14 '20 at 12:57
0

Before I need the password from a PasswordBox the User will tell me that he has finished with input by pressing a button.

The best option for me is to use the CommandParameter of the Button with the LoginCommand:

<StackPanel
        Width="300"
        HorizontalAlignment="Center"
        VerticalAlignment="Center">
    <Label Content="Username:" />
    <TextBox Text="{Binding Username}" />
    <Label Content="Password:" />
    <PasswordBox PasswordChanged="PasswordChanged" />
    <StackPanel
            Orientation="Horizontal">
        <Button
                Name="LoginButton"
                Command="{Binding LoginCommand}"
                Content="Login" />
    </StackPanel>
</StackPanel>

View code

private void PasswordChanged( object sender, RoutedEventArgs e )
{
    LoginButton.CommandParameter = ( sender as PasswordBox ).SecurePassword;
}

ViewModel Command

Login = ReactiveCommand.CreateFromTask( async ( SecureString password, CancellationToken cancellationToken ) =>
{
    var loggedIn = await AuthenticationService.LoginAsync( Username, password, cancellationToken );
    ...
} );
Sir Rufo
  • 18,395
  • 2
  • 39
  • 73
0

I just wanted to thank everyone for your answers and insighful comments. You made me realize that I should not be using a PasswordHelper and that I should not be trying to bind to the password at all.

Just in case anyone is having a similar problem with a MultiBinding not properly updating the DataContext, this can be fixed by adding the OneWayToSource mode to each of the bindings inside the MultiBinding.

<MultiBinding Converter="{StaticResource StringToSaltedHashConverter}" 
              Mode="OneWayToSource">
    <Binding Path="Password"
             Mode="OneWayToSource" />
    <Binding Path="PasswordSalt"
             Mode="OneWayToSource" />
</MultiBinding>
deiviz
  • 45
  • 1
  • 6