502265 - Master password unrecognised if input by handwriting TIP on Tablet PC

Status: RESOLVED WORKSFORME

(Core :: Widget: Win32, defect)

Description

[wisty]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9.0.10) Gecko/2009042316 Firefox/3.0.10 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0(Windows;U;Windows NT 6.1;en-US;rv:1.9.2a1pre)gecko/20090703 Minefield/3.6a1pre(.NET CLR 3.5.30729)

Entering the master password using handwriting recognition from the Tablet PC TIP fails. The password is not recognised. If the same password (phrase) is typed one letter at a time using the TIP's soft keyboard the password is accepted.

Reproducible: Always

Steps to Reproduce:
1.Tools/Options/Security/Saved Passwords
2.Handwrite pass phrase into TIP 
3.Insert
4.Click OK

Actual Results:  
Password Required box reappears

Expected Results:  
Passwords list should open

If the same password (Phrase) is typed one letter at a time into the Password Required box, the password is accepted and the stored sites appear

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 (.NET CLR 3.5.30729)

Works for me on Vista.  I don't have a Windows 7 setup to test on.

Comment 2

[u88484]

Can you try,

1. Try to reproduce in Safe Mode. http://support.mozilla.com/en-US/kb/Safe+Mode
2. Disable all add-on and see what the minimal set of add-ons installed will
cause this problem to reproduce
3. Try to reproduce in a new profile.
http://support.mozilla.com/en-US/kb/Managing+profiles

Comment 3

[wisty]

I have tested as best I can. It doesn't seem to be related to add-ons. It is a problem with a set of passwords imported from an earlier release of Firefox. To that end it may be me not understanding the upgrade process.

If I create a clean profile and insert passwords manually ( by logging into various websites ) the master password works with handwriting as expected.

However if I import a password file (key3.db and signons3.txt) from my Firefox 3.0.11 installation, the master password dialogue works OK with the on screen keyboard, but not with handwriting.

The same behaviour occurs in the options/security/show passwords

Its almost as if the handwritten version of the master password is slightly different from that in the 3.0.11 database file, but the on screen keyboard version is the same.

Comment 4

[u88484]

Hmm, ok.  I have no clue what component this bug should live in then.  Requesting QA to take a look into this.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Justin, do you know what could be cause such a problem?

Comment 6

[Justin Dolske [:Dolske]]

Does handwriting input work in other parts of the browser? EG, a bugzilla text input, an HTTP auth prompt, or javascript:prompt()?

Comment 7

[Robert Strong (they/them - no direct email)]

It should just work which is indicated by comment #3. Felipe have a system this can be checked out on and will be in the office on Thursday.

Comment 8

[wisty]

For comment #6. Yes other handwriting works well. The problem only shows up when the input string is the master password.

Comment 9

[wisty]

Further experiment. I exported the passwords from 3.0.11, created a new profile and imported the passwords (forced compatibility on Password Exporter). The password system worked as expected. I created the new master password using handwriting.

However if I now use the new profile with version 3.0.11 the master password is unrecognised (either with handwriting or the keyboard).

In minefield I have discovered that if I create the master password with handwriting, it is recognised with handwriting. If I create it with a keyboard, it is not recognised in handwriting.

Further issue that might help. Creating the master password in handwriting the password quality bar gets to about 75%. Create the same phrase using the onscreen keyboard, the quality bar gets to 100% and the on screen keyboard "flicks" away and back for the last few letters.

If I get some time I will perform some more specific experiments.

Comment 10

[wisty]

Some detailed tests suggest that Minefield deals with handwritten passwords under Windows 7 differently from the way it sees passwords input from a keyboard. It does however seem to deal with usernames and other text correctly (trailing nulls?).

If the master password is created with handwriting then any input to the master password box in handwriting is successful. Any input using the keyboard fails. The inverse is also true.

I also discovered that using handwriting to enter the password to ANY website  using Minefield causes the password validation to fail. Using the keyboard works. The error is in the password field not the username. I can enter the username using handwriting, and the password using the keyboard, and the login validates OK. Enter the password in handwriting and validation fails. Once Minefield has stored the (correct keyboard generated) login credentials for a site they are reproduced faithfully. 

Firefox 3.0.11 using the same profile works fine, I can enter website passwords using handwriting and they validate OK. 

Below is a summary of the tests I ran. The password quality measure was estimated with a ruler against the moving bar. I tested against both my ADSL router's admin page and my E-bay account.

Master password = A Short (7 Characters)
Test Sequence
Create New Profile
Open Minefield
Create Master Password
Access Website
Login & Ask Minefield to Save Password
Enter Master Password <-Test A
Close Down
*Restart Minefield
Access Website
Enter Master Password <-Test B
Close Site
Access Tools/Options/Security/Saved Passwords <-Test C
Close Down
Go to *Restart

Test 1 Create master password keyboard (Password Quality 65%)
Test A keyboard - Success
Test B keyboard - Success
Test C keyboard - Success
Test B handwriting - Failed
Test C handwriting - Failed

Test 2 Create master password handwriting (Password Quality 75%)
Test A handwriting - Success
Test B handwriting - Success
Test C handwriting - Success
Test B keyboard - Failed
Test C keyboard - Failed

Comment 11

[wisty]

I have just realised that this bug was found and all the tests run with intl.enable_tsf_support;true

If intl.enable_tsf_support;false Then all the password systems work as expected. No problems. However disabling the TIP support functions make Firefox unusable with a Tablet.

However it looks as if this code, or its interaction is causing the problem. FF3.0.11 with the GessoV3 add-on does not exhibit the same problems.

Comment 12

[wisty]

I have reproduced this problem under XP

Comment 13

[u88484]

(In reply to comment #7)
> It should just work which is indicated by comment #3. Felipe have a system this
> can be checked out on and will be in the office on Thursday.

Any word?

Comment 14

[Robert Strong (they/them - no direct email)]

Reproduced by Felipe. Will ping dolske when he is available to take a look at this bug

Comment 15

[Robert Strong (they/them - no direct email)]

Felipe found that with intl.enable_tsf_support set to true each character in the password was set to 9679 (e.g. black circle).

Comment 16

[Robert Strong (they/them - no direct email)]

char code 9679 that is

Comment 17

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I can see this on Vista with intl.enable_tsf_support = true too.

Comment 18

[wisty]

Any progress on getting this fixed?

Comment 19

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #16)
> char code 9679 that is

That's the password placeholder character.  I did some digging around and it looks like the TSF ITfContextOwnerCompositionSink interface methods are grabbing onto the placeholder text instead of the password data.

Comment 20

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I have a patch for this.  I'm going to throw it at try and make sure I didn't break anything.

Comment 21

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

This actually goes a lot deeper than I thought.  None of the content event handlers (for nsQuerySelectionText, etc) handle passwords properly.  They all fall for editor's content node with placeholder characters.

Comment 22

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #21)
> This actually goes a lot deeper than I thought.  None of the content event
> handlers (for nsQuerySelectionText, etc) handle passwords properly.  They all
> fall for editor's content node with placeholder characters.

Luckily they all converge on one function.  I'm going to throw another patch at try.

Comment 23

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: WIP

This makes the TIP work correctly, but it breaks a bunch of other stuff (doesn't pass TSF tests).  I probably should QI for an input element instead of nsIDOMNsEditableElement since the problem seems to be that the hack is getting applied to non-input elements.  That or check and see if the length of the editor return string is equal to the length of the text content of nsIContent.

Comment 24

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Somebody who understands editor and TSF needs to take a look at this, as I'm out of my league.

Comment 25

[Justin Dolske [:Dolske]]

Ehsan, you're Mr. Editor these days, can you help out?

Comment 26

[(no longer active)]

(In reply to comment #23)
> Created an attachment (id=423427) [details]
> WIP
> 
> This makes the TIP work correctly, but it breaks a bunch of other stuff
> (doesn't pass TSF tests).  I probably should QI for an input element instead of
> nsIDOMNsEditableElement since the problem seems to be that the hack is getting
> applied to non-input elements.  That or check and see if the length of the
> editor return string is equal to the length of the text content of nsIContent.

You need to QI to nsIDOMHTMLInputElement and check the return value of GetType, and do this only if it returns NS_FORM_INPUT_PASSWORD.  Can you please try that?

That said, I think the approach of this code is kind of wrong.  Boris, is it OK for this code to randomly touch content nodes which can be inside anonymous subtrees?

Also, CCing TSF folks.

Comment 27

[Boris Zbarsky [:bzbarsky]]

I have no idea why this code is ending up inside anon content.  Why is it ending up there, exactly?

Comment 28

[(no longer active)]

The code seems to be just getting the selection start and end points, and passing them to the function in question.  For example:

<http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsContentEventHandler.cpp#446>

Comment 29

[Boris Zbarsky [:bzbarsky]]

Ah, I see.  That should be ok, then, as long as this code doesn't modify the DOM in question, right?

Comment 30

[(no longer active)]

(In reply to comment #29)
> Ah, I see.  That should be ok, then, as long as this code doesn't modify the
> DOM in question, right?

Well, is it?  I think most native anonymous content is supposed to be controlled by the owning frame, and modifying them directly shouldn't give desirable results.  For example, what if the code grabs the text node under the text box for an <input type=file>?  It would be able to effectively edit the text box contents, something that we've disabled on purpose on the higher level.

Comment 31

[Boris Zbarsky [:bzbarsky]]

> and modifying them directly shouldn't give desirable results

Right, hence "as long as this code doesn't modify the DOM in question"...

Comment 32

[(no longer active)]

(In reply to comment #31)
> > and modifying them directly shouldn't give desirable results
> 
> Right, hence "as long as this code doesn't modify the DOM in question"...

Well, that's exactly what this code is doing.  :-)  So what's the best course of action here?

Comment 33

[Boris Zbarsky [:bzbarsky]]

Fixing this code to not poke its nose where it doesn't belong?  ;)

Would having a way to go from a selection to the associated editor, if any, do the trick?

Comment 34

[(no longer active)]

Maybe.  What should happen if let's say the range start node is inside the anonymous div for a textbox, and the range end node is outside the textbox?

Also, providing that API wouldn't really help fix the problem at hand.  Like I said in comment 26, one doesn't need to use the editor here at all.

Comment 35

[Boris Zbarsky [:bzbarsky]]

> let's say the range start node is inside the anonymous div for a textbox, and
> the range end node is outside the textbox?

That usually can't happen.

> Also, providing that API wouldn't really help fix the problem at hand. 

OK.  What would?

Comment 36

[(no longer active)]

(In reply to comment #35)
> > Also, providing that API wouldn't really help fix the problem at hand. 
> 
> OK.  What would?

Querying the parent for nsIDOMHTMLInputElement, making sure GetValue() == NS_FORM_INPUT_PASSWORD, and then use GetValue() and SetValue() on the input element pointer.

Comment 37

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #36)
> (In reply to comment #35)
> > > Also, providing that API wouldn't really help fix the problem at hand. 
> > 
> > OK.  What would?
> 
> Querying the parent for nsIDOMHTMLInputElement, making sure GetValue() ==
> NS_FORM_INPUT_PASSWORD, and then use GetValue() and SetValue() on the input
> element pointer.

This doesn't fix the underlying problem though, does it (e.g. the scenario in comment 30)?

Comment 38

[wisty]

Are we likely to see any further progress on this Bug

Comment 39

[(inactive) Jim Chen [:jchen] [:darchons]]

wisty, khuey, others?: I can work on this if I can get help with testing (I don't have my tablet PC right now). There is a way to tell Windows that we are in a password field so it doesn't try to mess with its contents.

Comment 40

[:Felipe Gomes (needinfo for replies!)]

I can test

Comment 41

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Can you still reproduce this? I cannot reproduce this with current Nightly build.

I guess that this was fixed by InputScope support (bug 835738). Now, Gecko tells focused input context's type to TIP. Therefore, TIP knows if the focused editor is password. Then, for security, TIP should not try to get inputted text. I think that TIPs behave so.

The attached patch gives raw password data to TSF. I think that it's wrong approach for security.

FYI: Starting with Aurora (29), "intl.enable_tsf_support" is renamed to "intl.tsf.enable". If you set "intl.enable_tsf_support" true, it will be migrated to the new pref automatically.

Comment 42

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Marking as WFM since nobody replies to my question and I cannot reproduce this bug.

Reopen this bug with new STR if you can still reproduce this bug.