841569 - Cannot install image-uploader and filatours packaged apps with reviewer certs - receive INVALID_SIGNATURE on install

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Jason Smith [:jsmith]]

Build:

Device: Unagi
Build: B2G 18 2/14/2013
Reviewer Certs Included

Steps:

1. Try installing https://marketplace.firefox.com/reviewers/apps/review/image-uploader or https://marketplace.firefox.com/reviewers/apps/review/filatours

Expected:

The app should install successfully.

Actual:

The app fails to install with INVALID_SIGNATURE.

Additional Notes:

The investigation I've done so far hasn't shown any obvious evidence that this is a client-side bug. Could be a signing bug on the marketplace side.

Comment 1

[Jason Smith [:jsmith]]

Mentioned this to Ryan in IRC a couple of days ago to see if he could investigate why this is failing. Have you found why this is failing?

Comment 2

[:gerard-majax]

ping?

Comment 3

[Ryan Tilder [:rtilder]]

Per my conversation with Jason on IRC last week, I can't access those Marketplace URLs since I am not a reviewer.  He was going to send me the necessary data.

Comment 4

[Jason Smith [:jsmith]]

(In reply to Ryan Tilder [:rtilder] from comment #3)
> Per my conversation with Jason on IRC last week, I can't access those
> Marketplace URLs since I am not a reviewer.  He was going to send me the
> necessary data.

I just emailed you the zip files from reviewer tools.

Comment 5

[Wil Clouser [:clouserw]]

-> rtilder I think.  Let me know if you need something

Comment 6

[Ryan Tilder [:rtilder]]

Is it possible for you to re-submit the packages for review and thereby have them re-signed, Jason?  I'm curious to see if a newer signature attempt will have the same problem.

Comment 7

[Jason Smith [:jsmith]]

(In reply to Ryan Tilder [:rtilder] from comment #6)
> Is it possible for you to re-submit the packages for review and thereby have
> them re-signed, Jason?  I'm curious to see if a newer signature attempt will
> have the same problem.

Redirecting to Lisa - can we try resubmitting these apps and seeing if we can install it after we resubmit?

Comment 8

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Please check the expiration date of the certificates used for the signing.

Over the weekend, the test certificates used for the Gecko unit tests expired because I put "expires in 90 days" in those certificates. I don't remember what expiration date we picked for the test certs used on the servers, but I wouldn't be surprised if there is a similar problem with them.

Attach one of the signed apps to this bug if you are still having trouble after confirming the test certs aren't expired.

Comment 9

[:gerard-majax]

As asked by lisa, I've resubmitted both the applications.

Comment 10

[Lisa Brewster [:adora]]

Still unable to install.  Alexandre, by chance did you delete the old versions and resubmit with the same version number?  If so, we're also seeing Bug 843419, and you'll need to change the version number.

Comment 11

[:gerard-majax]

Yesterday, I've resubmitted as version 1.1. I did not deleted the old versions.

Comment 12

[:gerard-majax]

I've deleted old 1.0 that were marked as « Disabled by Mozilla ».

Comment 14

[Jason Smith [:jsmith]]

So we're stuck on resolution in this bug.

bsmith - Can help shed some light on what you think we need to do here to diagnose and resolve this bug?

Comment 15

[Jason Smith [:jsmith]]

Working with rtilder on this bug, the signatures for packaged apps that work vs. not work don't have distinct differences.

Comment 16

[Lisa Brewster [:adora]]

Updated to new certs, still unable to install these apps.

Comment 17

[Wil Clouser [:clouserw]]

I'm removing the dependency on the packaged apps bug and closing that bug out.  Packaged apps are live, I'm treating this as a separate bug.

Comment 18

[Lisa Brewster [:adora]]

Attachment: Package for Image Uploader

Comment 19

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Camilo, can you take a look at this?

Probably the fastest way to figure out what to do would be to replace the privileged-app-test-1.0.zip file with the image-uploader ZIP, make the marketplace reviewer root cert (root-ca-reviewers-marketplace.der in the github repo below) trusted, and then run the test_signed_apps-marketplace.js or similar test in a debugger to find out what the problem with the package is.

In order to test it out end-to-end, you would have to use B2G Desktop (not Firefox) or a B2G device and do this craziness:

https://github.com/briansmith/marketplace-certs

And, I guess in order for cviecco to be able to test everything, he needs to have a reviewer account on marketplace.firefox.com so that these things can be accessible.

https://marketplace.firefox.com/reviewers/apps/review/image-uploader or https://marketplace.firefox.com/reviewers/apps/review/filatours

Ask cgalimidi who you need to ask for help for the marketplace.firefox.com account.

Comment 20

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Nominating tef+ because this is a potential bug in the client that could affect app installation.

Comment 21

[Jason Smith [:jsmith]]

Bumping over to core under the assumption that this is now a client-side bug. Will bump back if it turns out this isn't right.

Comment 22

[Johnny Stenback (:jst)]

Over to Camilio to explore this a bit further. If it turns out that this can be worked around w/o a client side change, we wouldn't need to block on this, but until we know that, we'll block on this.

Comment 23

[Lisa Brewster [:adora]]

Camilo, send eviljeff ([e-mail]) the email address you're using for Marketplace and he can get you reviewer privs.

Comment 24

[Alex Keybl [:akeybl]]

We want this so that we don't have to provide marketplace reviewers with a custom build. Marking as tef+ for now.

Camilo - if simple fixes can be made to the submitted apps, please mark this as [NPOTB].

Comment 25

[Camilo Viecco (:cviecco)]

Got access to the marketplace. Now tring to solve.

Comment 26

[David Scravaglieri [:scravag]]

Camilo, any estimation for this bug ?

Comment 27

[Camilo Viecco (:cviecco)]

Internal error->  NS_FILE_CORRUPTED.

Besides those two apps, any other apps with the problem?

ETA, tomorrow with p=.90
today: p=0.3

Comment 28

[Jason Smith [:jsmith]]

(In reply to Camilo Viecco (:cviecco) from comment #27)
> Internal error->  NS_FILE_CORRUPTED.
> 
> Besides those two apps, any other apps with the problem?

IQ FitFun Lite is another app. I think I'm also hitting the problem with WP Photos now too.

Comment 29

[Camilo Viecco (:cviecco)]

The manifests are invalid. The maximum data lenght for each line cannot exceed 72 bytes,

For the app the initiated the problem this is the problem line:
Name: style/bb/menus-dialogs/valueselector/date/images/ui/shadow-invert.png



http://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/src/JARSignatureVerification.cpp#248

Comment 30

[Jason Smith [:jsmith]]

I'm assuming you are referring to the manifest.mf file in the META-INF directory with that line you are referring, right?

Does that imply then this is no longer a client-side bug then, since we're following the spec here?

Comment 31

[Camilo Viecco (:cviecco)]

Correct it is still a bug, but not a client side bug.

Comment 32

[Jason Smith [:jsmith]]

Thanks Camilo. I think we've reached the conclusion here that this now back to being a signing bug. Sending this back over to marketplace to rtilder.

Comment 33

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #32)
> Thanks Camilo. I think we've reached the conclusion here that this now back
> to being a signing bug. Sending this back over to marketplace to rtilder.

When I meant "signing bug," I meant a bug on the signing service on the marketplace side.

Comment 34

[Jason Smith [:jsmith]]

Ryan - Can you take a look?

Comment 35

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Good work!

See http://docs.oracle.com/javase/7/docs/technotes/guides/jar/jar.html

"Line length: No line may be longer than 72 bytes (not characters), in its UTF8-encoded form. If a value would make the initial line longer than this, it should be continued on extra lines (each starting with a single SPACE)."

For example:

Name: style/bb/menus-dialogs/valueselector/date/images/ui/shadow-invert
 .png

Comment 36

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Camilo Viecco (:cviecco) from comment #27)
> Internal error->  NS_FILE_CORRUPTED.

Camilo, why did the openSignedJarFileAsync return NS_FILE_CORRUPTED instead of NS_ERROR_SIGNED_JAR_MANIFEST_INVALID?

In ParseAttribute in JARSignatireVerification.cpp:

  if (len > 72) {
    // The spec says "No line may be longer than 72 bytes (not characters)"
    // in its UTF8-encoded form. This check also ensures that len < INT32_MAX,
    // which is required below.
    return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
  }

and that error code seems to be propogated all the way back. Is nsJAR.cpp also doing its own parsing/checking of the manifest? If so, then that might be a bug in the client. The intent is that the nsJAR.cpp code should ignore all the stuff in META-INF because I re-implemented all that code in JARSignatureVerification.cpp in a better way.

Comment 37

[Camilo Viecco (:cviecco)]

Was a bad test file during testing (Ctrl + C) during copy.

Comment 38

[Jason Smith [:jsmith]]

Talked with Ryan in IRC - he said he would take a look into this today.

Comment 39

[:gerard-majax]

Still no news, we are now nearly at 4 months since I submitted my apps ... and no idea of the issue ?

Comment 40

[:gerard-majax]

I did work on the application this weekend, tried to take a chance and submit a new version on the Marketplace ...

After uploading this v1.2 (and removing any previous version), I'm able to download the manifest which is at https://marketplace.firefox.com/app/bf191091-abef-4443-ae1b-f955068d1875/manifest.webapp (this URL is listed in the edit listing page).

However, this manifest is empty, and contains only "{}".

Comment 41

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #40)
> I did work on the application this weekend, tried to take a chance and
> submit a new version on the Marketplace ...
> 
> After uploading this v1.2 (and removing any previous version), I'm able to
> download the manifest which is at
> https://marketplace.firefox.com/app/bf191091-abef-4443-ae1b-f955068d1875/
> manifest.webapp (this URL is listed in the edit listing page).
> 
> However, this manifest is empty, and contains only "{}".

Further attemps to download the file results in either 404 or 500 errors.

Comment 42

[:gerard-majax]

Here is logcat matching this:

I/Gecko   ( 5684): -*-*- Webapps.jsm : Got http status=500 for https://marketplace.firefox.com/app/bf191091-abef-4443-ae1b-f955068d1875/manifest.webapp
E/GeckoConsole( 5684): [JavaScript Error: "Error installing packaged app from: https://marketplace.firefox.com: MANIFEST_URL_ERROR" {file: "resource://gre/modules/Webapps.jsm" line: 1687}]
E/GeckoConsole( 5982): Content JS LOG at https://marketplace.cdn.mozilla.net/media/js/mkt/consumer-min.js?build=b8f21b0:5 in anonymous: Error code: MANIFEST_URL_ERROR

Comment 43

[Andrew Williamson [:eviljeff]]

(In reply to Alexandre LISSY :gerard-majax from comment #42)
> Here is logcat matching this:
> 
> I/Gecko   ( 5684): -*-*- Webapps.jsm : Got http status=500 for
> https://marketplace.firefox.com/app/bf191091-abef-4443-ae1b-f955068d1875/
> manifest.webapp
> E/GeckoConsole( 5684): [JavaScript Error: "Error installing packaged app
> from: https://marketplace.firefox.com: MANIFEST_URL_ERROR" {file:
> "resource://gre/modules/Webapps.jsm" line: 1687}]
> E/GeckoConsole( 5982): Content JS LOG at
> https://marketplace.cdn.mozilla.net/media/js/mkt/consumer-min.
> js?build=b8f21b0:5 in anonymous: Error code: MANIFEST_URL_ERROR

You can't install a packaged app from the public listing page until its been approved.  The bug is about being able to install via our reviewer tools before approval.

Comment 44

[:gerard-majax]

(In reply to Andrew Williamson [:eviljeff] from comment #43)
> (In reply to Alexandre LISSY :gerard-majax from comment #42)
> > Here is logcat matching this:
> > 
> > I/Gecko   ( 5684): -*-*- Webapps.jsm : Got http status=500 for
> > https://marketplace.firefox.com/app/bf191091-abef-4443-ae1b-f955068d1875/
> > manifest.webapp
> > E/GeckoConsole( 5684): [JavaScript Error: "Error installing packaged app
> > from: https://marketplace.firefox.com: MANIFEST_URL_ERROR" {file:
> > "resource://gre/modules/Webapps.jsm" line: 1687}]
> > E/GeckoConsole( 5982): Content JS LOG at
> > https://marketplace.cdn.mozilla.net/media/js/mkt/consumer-min.
> > js?build=b8f21b0:5 in anonymous: Error code: MANIFEST_URL_ERROR
> 
> You can't install a packaged app from the public listing page until its been
> approved.  The bug is about being able to install via our reviewer tools
> before approval.

I'm not talking about public listing, I'm talking about application for which I'm developper, thus I have access even if it's not yet approved.

Comment 45

[Andrew Williamson [:eviljeff]]

You /are/ talking about the public listing page, even though the app isn't publicly installable yet.  A packaged app is only signed on approval.  The app isn't approved so isn't signed so isn't installable.
This bug is about a separate file; only available from the reviewer pages, with an account with appropriate permissions; which is signed with a different certificate that isn't present on FirefoxOS by default.  This /is/ available before approval.

Comment 46

[:gerard-majax]

(In reply to Andrew Williamson [:eviljeff] from comment #45)
> You /are/ talking about the public listing page, even though the app isn't
> publicly installable yet.  A packaged app is only signed on approval.  The
> app isn't approved so isn't signed so isn't installable.
> This bug is about a separate file; only available from the reviewer pages,
> with an account with appropriate permissions; which is signed with a
> different certificate that isn't present on FirefoxOS by default.  This /is/
> available before approval.

Then there is another bug here: that I'm being able to see the app with an « Install » button.

Comment 47

[:gerard-majax]

Attachment: The application is installable while it should not ?

Please find attached a screenshot showing that I can actually install the app from my Nexus S, after adding the Marketplace account used on the device as a owner team member and accepting the developper agreement.

Comment 48

[Ryan Tilder [:rtilder]]

Patched and will work with ops to make sure it gets deployed as soon as possible. https://github.com/mozilla/signing-clients/pull/5

Comment 49

[michbier]

Just for information:
I am the author of app "IQ Fit Fun Lite" and got my Peak today.
I was able to install the app using simulator's new push feature.
Macbook Retina -> FF 20.0 -> simulator 3.0pre8

Comment 50

[:gerard-majax]

(In reply to michbier from comment #49)
> Just for information:
> I am the author of app "IQ Fit Fun Lite" and got my Peak today.
> I was able to install the app using simulator's new push feature.
> Macbook Retina -> FF 20.0 -> simulator 3.0pre8

Seems consistent since the issue is with signing on marketplace.

Comment 51

[:gerard-majax]

(In reply to Ryan Tilder [:rtilder] from comment #48)
> Patched and will work with ops to make sure it gets deployed as soon as
> possible. https://github.com/mozilla/signing-clients/pull/5

I see this has been merged, any idea when it will be live ?

Comment 52

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Alexandre LISSY :gerard-majax from comment #50)
> (In reply to michbier from comment #49)
> > Just for information:
> > I am the author of app "IQ Fit Fun Lite" and got my Peak today.
> > I was able to install the app using simulator's new push feature.
> > Macbook Retina -> FF 20.0 -> simulator 3.0pre8
> 
> Seems consistent since the issue is with signing on marketplace.

Right. The push feature skips all the JAR integrity and signature checking, so signature-related errors will never be caught with it.

Comment 53

[Ryan Tilder [:rtilder]]

(In reply to Alexandre LISSY :gerard-majax from comment #51)
> I see this has been merged, any idea when it will be live ?
>

It should be pushed to prod with tomorrow regularly scheduled release.  At that point affected packages can be re-signed with the production key.

--Ryan

Comment 54

[michbier]

I am a little bit confused, do the authors of the apps like mine (IQ FitFun) need to upload a new app version or just wait until approval will continue?

Comment 55

[Jason Smith [:jsmith]]

Wil - Should this be closed, given that a fix has been pushed and merged?

Comment 56

[Andrew Williamson [:eviljeff]]

btw, I've just tried to install filatour, and IQ Fitfun Lite, and installing is failing for both of them.

Comment 57

[Jason Smith [:jsmith]]

(In reply to Andrew Williamson [:eviljeff] from comment #56)
> btw, I've just tried to install filatour, and IQ Fitfun Lite, and installing
> is failing for both of them.

Well, I guess that answers my needinfo request. This isn't fixed.

Comment 58

[Andrew Williamson [:eviljeff]]

I'm not sure if the reviewer cert signing is done on upload, on first use, or on-demand each time.  If its anything other than on-demand something might need to be run to force re-signing on existing apps.

Comment 59

[Andrew Williamson [:eviljeff]]

Okay, all the cached reviewer cert signed packaged apps have been cleared off the disk now so Marketplace should be signing with the new signing code.

They're still failing though for me though.

Ryan, do they work for you?  (You now have reviewer access on Marketplace)

Comment 60

[Wil Clouser [:clouserw]]

(In reply to Andrew Williamson [:eviljeff] from comment #58)
> I'm not sure if the reviewer cert signing is done on upload, on first use,
> or on-demand each time.  If its anything other than on-demand something
> might need to be run to force re-signing on existing apps.

For the record, they are currently signed on demand and then saved on disk permanently.  Another bug is filed to gc the cached versions on disk once a day.

Comment 61

[Ryan Tilder [:rtilder]]

The re-signed packages look correct for the 72 byte line length limit.  I fear we're going to need Brian to see what the error being generated is now.

Comment 62

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Camilo, you investigated this before. Do you remember anything else wrong other than the 72-byte line issue? Is there any way you can investigate this again?

Comment 63

[Camilo Viecco (:cviecco)]

Sure, but it will have to wait until tomorrow morning. Wait for more info.

Comment 64

[Camilo Viecco (:cviecco)]

it is possible to get a raw copy of the new app?

Comment 65

[Lisa Brewster [:adora]]

Camilo, what do you mean by "raw copy of the new app?"

Btw, the same INVALID_SIGNATURE error is also showing in logcat when trying to install the packaged version of the preinstalled Notes app.

https://marketplace.firefox.com/reviewers/apps/review/notes-packaged

Comment 66

[Jason Smith [:jsmith]]

FYI - with those meta bugs like bug 869212, I was only intending to track app-specific issues there, not other issues such as this bug.

Comment 67

[Andrew Williamson [:eviljeff]]

Alexandre: can you attempt a possible workaround for us to get your app listed?  If this bug is still caused by long file paths, shorter ones will not be affected.  So can you rename some files/folders so no file path is longer than 67 characters?

Comment 68

[michbier]

IQ Fit Fun manifest looks like this:

Any recommendation how to change it would be great:

{
  "name": "IQ FitFunLite",
  "version": "1.2.6.1",

  "description": "Have fun with 43 free brain teasers for young and old (lite version).",
  "icons": {
     "16": "/IQFitFun/res/drawable/iqfitfun_lite_icon_round_16.png",
    "128": "/IQFitFun/res/drawable/iqfitfun_lite_icon_round_128.png"
  },
  "developer": {
    "name": "DsignMatters, Michael Biermann",
    "url": "http://sites.google.com/site/dsignmatters/"
  },
  "default_locale": "en",
  "launch_path": "/IQFitFun/index.html",
  "content_security_policy": "script-src 'self' 'unsafe-eval'; object-src 'self'",
  "installs_allowed_from": ["*"]
}

Comment 69

[Andrew Williamson [:eviljeff]]

(In reply to michbier from comment #68)
> IQ Fit Fun manifest looks like this:
> 
> Any recommendation how to change it would be great:

There isn't any issue in the app manifest.  Its the file paths - i.e. folders+filename.  From a quick look I think its the filenames of the image assets e.g. IQFitFun/res/drawable/background_blackboard_desc_164x59_wo_shadow.png

Comment 70

[Camilo Viecco (:cviecco)]

Line size restrictions where being enforced the decoded line not on a per line basis. Patch coming soon.

Comment 71

[Jason Smith [:jsmith]]

(In reply to Camilo Viecco (:cviecco) from comment #70)
> Line size restrictions where being enforced the decoded line not on a per
> line basis. Patch coming soon.

This needs a gecko patch I take it, right?

If so, I'd close this bug for the work rtilder did and open a new bug in the core to track the gecko work. Nom that also for tef.

Comment 72

[Camilo Viecco (:cviecco)]

Attachment: move size check from parseline to readline

Comment 73

[Jason Smith [:jsmith]]

tef? since we now know there's a client change required here. Marketplace piece is finished per an earlier comment.

Comment 74

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 749140 [details] [diff] [review]
move size check from parseline to readline

Review of attachment 749140 [details] [diff] [review]:
-----------------------------------------------------------------

Please add a test for this to test_signed_apps.js (*not* test_signed_apps-marketplace.js). It should be easy to modify the existing test case to include an entry with a path that causes the manifest entry to wrap.

::: security/manager/ssl/src/JARSignatureVerification.cpp
@@ +197,5 @@
>  // On input, nextLineStart is the start of the current line. On output,
>  // nextLineStart is the start of the next line.
>  nsresult
>  ReadLine(/*in/out*/ const char* & nextLineStart, /*out*/ nsCString & line,
> +         bool allowContinuations = true, size_t lineLimit = 72 )

Why is lineLimit a parameter? AFAICT, it should be a local static constant, or even just use a literal "72" below with a comment that says this restriction is from the spec.

@@ +222,5 @@
> +    }
> +    
> +    // We can probably be more strict as the spec says: "Implementations 
> +    // should support 65535-byte (not character) header values..."
> +    if (currentLength >  INT32_MAX) {

whitespace around ">". Also, I suggest we compare against 65535. Even 65535 is ridiculously long for our needs. Also, IIRC, elsewhere in JarSignatureVerification.cpp we limit the total size of the manifest file to be fairly small to prevent OOM since we read the entire file into memory.

Comment 75

[Camilo Viecco (:cviecco)]

Attachment: move size check (v2)

Comment 76

[Camilo Viecco (:cviecco)]

Attachment: move size check (v2.1)

Comment 77

[Camilo Viecco (:cviecco)]

Comment on attachment 749566 [details] [diff] [review]
move size check (v2.1)

Review of attachment 749566 [details] [diff] [review]:
-----------------------------------------------------------------

previous review comments addressed

Comment 78

[Alex Keybl [:akeybl]]

tef+ since this is blocking marketplace testing.

Comment 79

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 749566 [details] [diff] [review]
move size check (v2.1)

Review of attachment 749566 [details] [diff] [review]:
-----------------------------------------------------------------

The patch looks good. Please add the test.

::: security/manager/ssl/src/JARSignatureVerification.cpp
@@ +215,5 @@
> +    currentLength = line.Length();
> +
> +    // The spec says "No line may be longer than 72 bytes (not characters)"
> +    // in its UTF8-encoded form.
> +    const size_t lineLimit = 72;

nit: make this static.

@@ +726,5 @@
>      NS_ENSURE_TRUE(signerCert, NS_ERROR_OUT_OF_MEMORY);
>  
>      signerCert.forget(aSignerCert);
>    }
> + 

Undo this whitespace change.

Comment 80

[Camilo Viecco (:cviecco)]

push to try with tests before cleanup:
https://tbpl.mozilla.org/?tree=Try&rev=fc00df66c7cb

Comment 81

[Camilo Viecco (:cviecco)]

Attachment: move size check (v3)

Comment 82

[Camilo Viecco (:cviecco)]

Attachment: move size check (v3.1)

now with tests!

Comment 83

[Camilo Viecco (:cviecco)]

Attachment: move size check (v3.1)

Comment 84

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 750100 [details] [diff] [review]
move size check (v3.1)

Review of attachment 750100 [details] [diff] [review]:
-----------------------------------------------------------------

There is no test that verifies that we reject a line longer than 72 bytes. However, that isn't the important thing here; I don't actually care whether we accept lines that are too long or not, as long as we correctly accept the continuation form of lines, which is what this new test *does* test.

::: security/manager/ssl/tests/unit/test_signed_apps/sign_b2g_app.py
@@ +14,5 @@
>  max_entry_count = 100 * 1000
>  max_entry_filename_len = 1024
>  max_mf_len = max_entry_count * 50
>  max_sf_len = 1024
>  

Add a comment along the lines of "Lines in a JAR manifest file are required to be 72 bytes or less."

@@ +18,5 @@
>  
> +def reformat_line_to_JAR_line(inline):
> +  newline = inline[0:72]
> +  leftover = inline[72:]
> +  while len(leftover) > 0 :

s/ :/:/

Add a comment along the lines of "On continuation lines, the leading space allows only 71 bytes from the original line to be used."

Nit: The words "newline" and "inline" both have completely different meanings from what you are using them here and those names don't match the naming convention used throughout the file. I suggest using "input_line" and "output_line".

Comment 85

[Camilo Viecco (:cviecco)]

Attachment: move size check (v4)

comments addressed. keeping r+ from bsmith

Comment 86

[Camilo Viecco (:cviecco)]

v4 push to try

https://tbpl.mozilla.org/?tree=Try&rev=53c8c83a85bc

Comment 87

[Alex Keybl [:akeybl]]

(In reply to Camilo Viecco (:cviecco) from comment #86)
> v4 push to try
> 
> https://tbpl.mozilla.org/?tree=Try&rev=53c8c83a85bc

Please land or add checkin-needed if the try results look good.

Comment 88

[Camilo Viecco (:cviecco)]

try failed on win32 due to filename length limitations. Since this only affects the test there will be a new bug on the tests and land the fix. ASAP

try: https://tbpl.mozilla.org/?tree=Try&rev=c641101def5c

Comment 89

[Camilo Viecco (:cviecco)]

Attachment: move size check (v5)

removal of tests so tha we can land this.
keeping r+ from bsmith

Comment 90

[Daniel Coloma:dcoloma]

(In reply to Camilo Viecco (:cviecco) from comment #89)
> Created attachment 750813 [details] [diff] [review]
> move size check (v5)
> 
> removal of tests so tha we can land this.
> keeping r+ from bsmith

Are we good to land this now? If so I think we just need to request check-in

Comment 91

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cb242a1cccb2

Comment 92

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/b87559cef261
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/67ac7c9e74fb

Comment 93

[Jason Smith [:jsmith]]

So Andrew ended up testing this today on a 5/21 build and this isn't working for him. I'm following up with Rob & Ryan to find out why.

Comment 94

[Jason Smith [:jsmith]]

Looks like comment 93 is a false alarm. We didn't have the reviewer certs installed when we tested this. The two apps in question here can now be installed on 1.01. Marking as verified.

Comment 95

[:gerard-majax]

My application has now been validated :)

Comment 96

[michbier]

My app "IQ FitFun Lite" still pending review "Estimated waiting time: 1 working day", but nothing seems to happen for more than 40 days.

Comment 97

[michbier]

(In reply to michbier from comment #96)
> My app "IQ FitFun Lite" still pending review "Estimated waiting time: 1
> working day", but nothing seems to happen for more than 40 days.

My app "IQ FitFun Lite" has been approved today and is listed on the Firefox Marketplace.
Many thx
    Michael

Comment 98

[Andrew Williamson [:eviljeff]]

(In reply to michbier from comment #96)
> My app "IQ FitFun Lite" still pending review "Estimated waiting time: 1
> working day", but nothing seems to happen for more than 40 days.

bug 845263 was tracking that app - but it seems to have slipped through the cracks.  I've reviewed it now.