Opened 7 months ago

Closed 7 months ago

#30280 closed defect (fixed)

Wrong SHA-256 sum for j2objc-annotations-1.1.jar

Reported by: gk Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-mobile, tbb-rbm, TorBrowserTeam201904R, tbb-8.5-must
Cc: sisbell, boklm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

While testing #29981 it turns out that j2objc-annotations-1.1.jar got changed recently on the mirror we download it from, resulting in a non-matching SHA-256 sum and a broken build.

The diff shows two different .class files, ReflectionSupport.class and WeakOuter.class and

diff -r test/META-INF/MANIFEST.MF test_later/META-INF/MANIFEST.MF
3c3
< Built-By: kstanger
---
> Built-By: tball
5c5
< Build-Jdk: 1.8.0_91
---
> Build-Jdk: 1.8.0_112
diff -r test/META-INF/maven/com.google.j2objc/j2objc-annotations/pom.properties test_later/META-INF/maven/com.google.j2objc/j2objc-annotations/pom.properties
2c2
< #Tue Jul 26 15:38:55 EDT 2016
---
> #Wed Jan 18 15:06:49 PST 2017

It seems, someone recompiled the Java code and just replaced the file on the mirror.

The ReflectionSupport.class diff is:

--- /dev/fd/63	2019-04-24 09:44:07.004119666 +0200
+++ /dev/fd/62	2019-04-24 09:44:07.008119696 +0200
@@ -19,20 +19,20 @@
 00000120: 6e2f 5265 7465 6e74 696f 6e3b 0100 264c  n/Retention;..&L
 00000130: 6a61 7661 2f6c 616e 672f 616e 6e6f 7461  java/lang/annota
 00000140: 7469 6f6e 2f52 6574 656e 7469 6f6e 506f  tion/RetentionPo
-00000150: 6c69 6379 3b01 0006 534f 5552 4345 0100  licy;...SOURCE..
-00000160: 2f63 6f6d 2f67 6f6f 676c 652f 6a32 6f62  /com/google/j2ob
-00000170: 6a63 2f61 6e6e 6f74 6174 696f 6e73 2f52  jc/annotations/R
-00000180: 6566 6c65 6374 696f 6e53 7570 706f 7274  eflectionSupport
-00000190: 0100 106a 6176 612f 6c61 6e67 2f4f 626a  ...java/lang/Obj
-000001a0: 6563 7401 001f 6a61 7661 2f6c 616e 672f  ect...java/lang/
-000001b0: 616e 6e6f 7461 7469 6f6e 2f41 6e6e 6f74  annotation/Annot
-000001c0: 6174 696f 6e01 0035 636f 6d2f 676f 6f67  ation..5com/goog
-000001d0: 6c65 2f6a 326f 626a 632f 616e 6e6f 7461  le/j2objc/annota
-000001e0: 7469 6f6e 732f 5265 666c 6563 7469 6f6e  tions/Reflection
-000001f0: 5375 7070 6f72 7424 4c65 7665 6c26 0100  Support$Level&..
-00000200: 0100 0200 0100 0300 0000 0104 0100 0700  ................
-00000210: 0800 0000 0300 0900 0000 0200 0a00 0b00  ................
-00000220: 0000 2000 0200 0c00 0100 075b 0002 6500  .. ........[..e.
-00000230: 0d00 0e65 000d 000f 0010 0001 0007 6500  ...e..........e.
-00000240: 1100 1200 0600 0000 0a00 0100 0400 0100  ................
-00000250: 0540 19                                  .@.
+00000150: 6c69 6379 3b01 0005 434c 4153 5301 002f  licy;...CLASS../
+00000160: 636f 6d2f 676f 6f67 6c65 2f6a 326f 626a  com/google/j2obj
+00000170: 632f 616e 6e6f 7461 7469 6f6e 732f 5265  c/annotations/Re
+00000180: 666c 6563 7469 6f6e 5375 7070 6f72 7401  flectionSupport.
+00000190: 0010 6a61 7661 2f6c 616e 672f 4f62 6a65  ..java/lang/Obje
+000001a0: 6374 0100 1f6a 6176 612f 6c61 6e67 2f61  ct...java/lang/a
+000001b0: 6e6e 6f74 6174 696f 6e2f 416e 6e6f 7461  nnotation/Annota
+000001c0: 7469 6f6e 0100 3563 6f6d 2f67 6f6f 676c  tion..5com/googl
+000001d0: 652f 6a32 6f62 6a63 2f61 6e6e 6f74 6174  e/j2objc/annotat
+000001e0: 696f 6e73 2f52 6566 6c65 6374 696f 6e53  ions/ReflectionS
+000001f0: 7570 706f 7274 244c 6576 656c 2601 0001  upport$Level&...
+00000200: 0002 0001 0003 0000 0001 0401 0007 0008  ................
+00000210: 0000 0003 0009 0000 0002 000a 000b 0000  ................
+00000220: 0020 0002 000c 0001 0007 5b00 0265 000d  . ........[..e..
+00000230: 000e 6500 0d00 0f00 1000 0100 0765 0011  ..e..........e..
+00000240: 0012 0006 0000 000a 0001 0004 0001 0005  ................
+00000250: 4019                                     @.

and the WeakOuter.class one is

--- /dev/fd/63	2019-04-24 09:44:12.840162976 +0200
+++ /dev/fd/62	2019-04-24 09:44:12.840162976 +0200
@@ -1,5 +1,5 @@
-00000000: cafe babe 0000 0031 0011 0700 0e07 000f  .......1........
-00000010: 0700 1001 000a 536f 7572 6365 4669 6c65  ......SourceFile
+00000000: cafe babe 0000 0031 0012 0700 0f07 0010  .......1........
+00000010: 0700 1101 000a 536f 7572 6365 4669 6c65  ......SourceFile
 00000020: 0100 0e57 6561 6b4f 7574 6572 2e6a 6176  ...WeakOuter.jav
 00000030: 6101 0019 5275 6e74 696d 6556 6973 6962  a...RuntimeVisib
 00000040: 6c65 416e 6e6f 7461 7469 6f6e 7301 001d  leAnnotations...
@@ -8,18 +8,20 @@
 00000070: 7661 6c75 6501 0022 4c6a 6176 612f 6c61  value.."Ljava/la
 00000080: 6e67 2f61 6e6e 6f74 6174 696f 6e2f 456c  ng/annotation/El
 00000090: 656d 656e 7454 7970 653b 0100 0454 5950  ementType;...TYP
-000000a0: 4501 0020 4c6a 6176 612f 6c61 6e67 2f61  E.. Ljava/lang/a
-000000b0: 6e6e 6f74 6174 696f 6e2f 5265 7465 6e74  nnotation/Retent
-000000c0: 696f 6e3b 0100 264c 6a61 7661 2f6c 616e  ion;..&Ljava/lan
-000000d0: 672f 616e 6e6f 7461 7469 6f6e 2f52 6574  g/annotation/Ret
-000000e0: 656e 7469 6f6e 506f 6c69 6379 3b01 0006  entionPolicy;...
-000000f0: 534f 5552 4345 0100 2763 6f6d 2f67 6f6f  SOURCE..'com/goo
-00000100: 676c 652f 6a32 6f62 6a63 2f61 6e6e 6f74  gle/j2objc/annot
-00000110: 6174 696f 6e73 2f57 6561 6b4f 7574 6572  ations/WeakOuter
-00000120: 0100 106a 6176 612f 6c61 6e67 2f4f 626a  ...java/lang/Obj
-00000130: 6563 7401 001f 6a61 7661 2f6c 616e 672f  ect...java/lang/
-00000140: 616e 6e6f 7461 7469 6f6e 2f41 6e6e 6f74  annotation/Annot
-00000150: 6174 696f 6e26 0100 0100 0200 0100 0300  ation&..........
-00000160: 0000 0000 0200 0400 0000 0200 0500 0600  ................
-00000170: 0000 1b00 0200 0700 0100 085b 0001 6500  ...........[..e.
-00000180: 0900 0a00 0b00 0100 0865 000c 000d       .........e....
+000000a0: 4501 000e 4c4f 4341 4c5f 5641 5249 4142  E...LOCAL_VARIAB
+000000b0: 4c45 0100 204c 6a61 7661 2f6c 616e 672f  LE.. Ljava/lang/
+000000c0: 616e 6e6f 7461 7469 6f6e 2f52 6574 656e  annotation/Reten
+000000d0: 7469 6f6e 3b01 0026 4c6a 6176 612f 6c61  tion;..&Ljava/la
+000000e0: 6e67 2f61 6e6e 6f74 6174 696f 6e2f 5265  ng/annotation/Re
+000000f0: 7465 6e74 696f 6e50 6f6c 6963 793b 0100  tentionPolicy;..
+00000100: 0653 4f55 5243 4501 0027 636f 6d2f 676f  .SOURCE..'com/go
+00000110: 6f67 6c65 2f6a 326f 626a 632f 616e 6e6f  ogle/j2objc/anno
+00000120: 7461 7469 6f6e 732f 5765 616b 4f75 7465  tations/WeakOute
+00000130: 7201 0010 6a61 7661 2f6c 616e 672f 4f62  r...java/lang/Ob
+00000140: 6a65 6374 0100 1f6a 6176 612f 6c61 6e67  ject...java/lang
+00000150: 2f61 6e6e 6f74 6174 696f 6e2f 416e 6e6f  /annotation/Anno
+00000160: 7461 7469 6f6e 2601 0001 0002 0001 0003  tation&.........
+00000170: 0000 0000 0002 0004 0000 0002 0005 0006  ................
+00000180: 0000 0020 0002 0007 0001 0008 5b00 0265  ... ........[..e
+00000190: 0009 000a 6500 0900 0b00 0c00 0100 0865  ....e..........e
+000001a0: 000d 000e                                ....

Child Tickets

Change History (10)

comment:1 Changed 7 months ago by gk

So, I guess the immediate reaction to this bug would be to add the new SHA-256 sum where needed and bump the gradle dependencies versions respectively?

However, I'd like to think about other thing we might want to do. For instance, I think we should try to catch this kind of issue way earlier and/or avoid it right from the beginning.

Previously, when using Gitian for our reproducible builds our nightly builds built everything from scratch every day making sure someone who just set up our reproducible builds environment would very likely get a working one AND we would get notified about a broken set up fast.

Now, that might not be doable anymore given the complexity of our current setup and how long it takes to build everything from scratch, but maybe it would be okay if we, say, build only armv7 from scratch, plus maybe a desktop arch to have a reasonable coverage?

We could think as well about mirroring the gradle dependencies we use ourselves to avoid this kind of bug (both in addition to the previous idea or instead of it).

comment:2 Changed 7 months ago by sisbell

If we look at maven central, we see the later 2017 version

http://central.maven.org/maven2/com/google/j2objc/j2objc-annotations/1.1

If we go to ibiblio, we see the earlier 2016 version
http://maven.ibiblio.org/maven2/com/google/j2objc/j2objc-annotations/1.1/

So it does look like bintray pulled from ibiblio and then later from maven central. We don't have any assurances bintray wouldn't switch back at some point.

My suggestion at this point, is to dump all uses of bintray. There is nothing stopping someone from overriding artifacts, using this as a back door. We can point all references directly to maven central and then to ibiblio in the (unlikely) situation that central doesn't host the artifact.

comment:3 in reply to:  1 ; Changed 7 months ago by boklm

Replying to gk:

Previously, when using Gitian for our reproducible builds our nightly builds built everything from scratch every day making sure someone who just set up our reproducible builds environment would very likely get a working one AND we would get notified about a broken set up fast.

Now, that might not be doable anymore given the complexity of our current setup and how long it takes to build everything from scratch, but maybe it would be okay if we, say, build only armv7 from scratch, plus maybe a desktop arch to have a reasonable coverage?

An other possibility is to rebuild everything from scratch (removing the out/ directory) once a week. However it might take more than 24 hours to rebuild everything, so we would skip the next nightly. Maybe that can be done during the weekend, when we usually push less commits.

comment:4 in reply to:  2 ; Changed 7 months ago by gk

Replying to sisbell:

If we look at maven central, we see the later 2017 version

http://central.maven.org/maven2/com/google/j2objc/j2objc-annotations/1.1

If we go to ibiblio, we see the earlier 2016 version
http://maven.ibiblio.org/maven2/com/google/j2objc/j2objc-annotations/1.1/

So it does look like bintray pulled from ibiblio and then later from maven central. We don't have any assurances bintray wouldn't switch back at some point.

My suggestion at this point, is to dump all uses of bintray. There is nothing stopping someone from overriding artifacts, using this as a back door. We can point all references directly to maven central and then to ibiblio in the (unlikely) situation that central doesn't host the artifact.

Works for me. Could you come up with a patch for that?

comment:5 Changed 7 months ago by gk

Keywords: tbb-8.5-must added

comment:6 in reply to:  3 Changed 7 months ago by gk

Replying to boklm:

Replying to gk:

Previously, when using Gitian for our reproducible builds our nightly builds built everything from scratch every day making sure someone who just set up our reproducible builds environment would very likely get a working one AND we would get notified about a broken set up fast.

Now, that might not be doable anymore given the complexity of our current setup and how long it takes to build everything from scratch, but maybe it would be okay if we, say, build only armv7 from scratch, plus maybe a desktop arch to have a reasonable coverage?

An other possibility is to rebuild everything from scratch (removing the out/ directory) once a week. However it might take more than 24 hours to rebuild everything, so we would skip the next nightly. Maybe that can be done during the weekend, when we usually push less commits.

Great idea! I opened #30283 for that.

comment:7 in reply to:  4 Changed 7 months ago by sisbell

Replying to gk:

Replying to sisbell:

If we look at maven central, we see the later 2017 version

http://central.maven.org/maven2/com/google/j2objc/j2objc-annotations/1.1

If we go to ibiblio, we see the earlier 2016 version
http://maven.ibiblio.org/maven2/com/google/j2objc/j2objc-annotations/1.1/

So it does look like bintray pulled from ibiblio and then later from maven central. We don't have any assurances bintray wouldn't switch back at some point.

My suggestion at this point, is to dump all uses of bintray. There is nothing stopping someone from overriding artifacts, using this as a back door. We can point all references directly to maven central and then to ibiblio in the (unlikely) situation that central doesn't host the artifact.

Works for me. Could you come up with a patch for that?

Yes working on that now. Almost all artifacts from bintray are located in maven central. But it looks like a few artifacts are only located in https://repo.spring.io/plugins-release/ so that will be another repo we will directly point at. I'll also need to update the documentation since this is an extra step we will need to take when generating dependencies.

comment:8 Changed 7 months ago by sisbell

I removed jcenter from all the dependencies. We just use maven central and spring.io now. We won't have any more changing SHA-256 sums.

https://github.com/sisbell/tor-browser-build/tree/0424

I didn't add documentation since I've opened these issues.

https://github.com/sisbell/tor-android-service/issues/23
https://github.com/thaliproject/Tor_Onion_Proxy_Library/issues/109

I'll remove jcenter from the gradle builds and add in maven repo and spring.io repo. So no special documentation will be needed on how to handle these cases when processing dependencies.

comment:9 Changed 7 months ago by gk

Keywords: TorBrowserTeam201904R added; TorBrowserTeam201904 removed
Status: newneeds_review

comment:10 Changed 7 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

This looks mostly good to me. The commit change to the rbm submodule is wrong, though (it reverts to the previous commit). I've fixed that up to save a round trip and amended the commit message slightly. Pushed to master (commit 6535b7c5b83ea7da7c5d57e92ff957e2e43e2ff6) and maint-8.5 (commit 5d764e950d6783820b7ab0d78db91000b0df817d).

sisbell: if you think a ticket is ready for review, please set the state accordingly (state to needs_review and ideally making sure there is the "TorBrowserTeamYYYYMMR" keyword set). I almost lost track of it.

Note: See TracTickets for help on using tickets.