Skip to content

Conversation

@jbonofre
Copy link
Member

@jbonofre jbonofre commented May 8, 2025

As discussed within the Parquet community, this PR is more "restrictive":

  1. no serializable packages by default
  2. deprecate org.apache.avro.SERIALIZABLE_PACKAGES property
  3. instead of SERIALIZABLE_PACKAGES, users should use org.apache.avro.SERIALIZABLE_CLASSES

This is a much more "strict" security enforcement.

@github-actions github-actions bot added the Java Pull Requests for Java binding label May 8, 2025
@jbonofre jbonofre requested review from Fokko and martin-g May 8, 2025 16:41
@jbonofre
Copy link
Member Author

jbonofre commented May 8, 2025

@gszadovszky @Fokko @martin-g ^^

@jbonofre jbonofre force-pushed the improve-security-check branch 2 times, most recently from 78ca676 to 26294d9 Compare May 11, 2025 12:51
@github-actions github-actions bot added the build label May 11, 2025
@jbonofre jbonofre force-pushed the improve-security-check branch from 26294d9 to 5269613 Compare May 11, 2025 13:03
@jbonofre
Copy link
Member Author

@nandorKollar @martin-g @Fokko I updated the PR. Can you please do a new pass ?

Copy link
Contributor

@nandorKollar nandorKollar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM! Thanks for fixing this!

…to introduce org.apache.avro.SERIALIZABLE_CLASSES instead
@jbonofre jbonofre force-pushed the improve-security-check branch from 5269613 to e9bfef8 Compare May 14, 2025 11:31
@jbonofre
Copy link
Member Author

@martin-g @Fokko @nandorKollar I'm volunteer to do new Avro releases (1.11.5 and 1.12.1) including this. Thoughts ?

throw new SecurityException("Forbidden " + clazz
+ "! This class is not trusted to be included in Avro schema using java-class. Please set org.apache.avro.SERIALIZABLE_PACKAGES system property with the packages you trust.");

for (String trustedPackage : getTrustedPackages()) {

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation Note

Invoking
SpecificDatumReader.getTrustedPackages
should be avoided because it has been deprecated.
@nandorKollar
Copy link
Contributor

@martin-g @Fokko @nandorKollar I'm volunteer to do new Avro releases (1.11.5 and 1.12.1) including this. Thoughts ?

I'll wait a couple of more days for any objection, then I'll merge this PR. Would be great if you could help with the release!

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for waiting @nandorKollar, my mailbox is overflowing. This looks good to me, thanks @jbonofre for working on this 👍

@nandorKollar nandorKollar merged commit 90a937f into apache:main May 19, 2025
8 checks passed
@Fokko
Copy link
Contributor

Fokko commented May 19, 2025

@jbonofre I'm seeing some errors on main: https://github.com/apache/avro/actions/runs/15121164134/job/42503656318

Error:  Errors: 
Error:    TestReflect.nullableStringableField:1179->checkBinary:1206->checkBinary:1184->checkBinary:1200 » Security Forbidden java.math.BigDecimal! This class is not trusted to be included in Avro schema using java-class. Please set org.apache.avro.SERIALIZABLE_CLASSES system property with the class you trust or org.apache.avro.SERIALIZABLE_PACKAGES system property with the packages you trust.
Error:    TestReflect.r10:434->checkReadWrite:623 » Security Forbidden org.apache.avro.reflect.TestReflect$R10! This class is not trusted to be included in Avro schema using java-class. Please set org.apache.avro.SERIALIZABLE_CLASSES system property with the class you trust or org.apache.avro.SERIALIZABLE_PACKAGES system property with the packages you trust.
Error:    TestReflect.stringableMapKeys:1166->checkBinary:1184->checkBinary:1200 » Security Forbidden java.math.BigDecimal! This class is not trusted to be included in Avro schema using java-class. Please set org.apache.avro.SERIALIZABLE_CLASSES system property with the class you trust or org.apache.avro.SERIALIZABLE_PACKAGES system property with the packages you trust.
Error:    TestReflect.stringables:1126->checkStringable:1138->checkBinary:1206->checkBinary:1184->checkBinary:1200 » Security Forbidden java.math.BigDecimal! This class is not trusted to be included in Avro schema using java-class. Please set org.apache.avro.SERIALIZABLE_CLASSES system property with the class you trust or org.apache.avro.SERIALIZABLE_PACKAGES system property with the packages you trust.

@nandorKollar
Copy link
Contributor

Ooops, looks like setting org.apache.avro.SERIALIZABLE_CLASSES is missing from lang/java/avro/src/it/pom.xml

nandorKollar pushed a commit to nandorKollar/avro that referenced this pull request May 19, 2025
nandorKollar added a commit to nandorKollar/avro that referenced this pull request May 19, 2025
@jbonofre
Copy link
Member Author

@nandorKollar @Fokko i was a out to fix the IT as we are now much more restrictive than before. Thanks @nandorKollar for fixing that in the other PR !

martin-g pushed a commit that referenced this pull request May 20, 2025
opwvhk pushed a commit to opwvhk/avro that referenced this pull request Aug 10, 2025
…to introduce org.apache.avro.SERIALIZABLE_CLASSES instead (apache#3376)

(cherry picked from commit 90a937f)
opwvhk pushed a commit to opwvhk/avro that referenced this pull request Aug 18, 2025
…to introduce org.apache.avro.SERIALIZABLE_CLASSES instead (apache#3376)

(cherry picked from commit 90a937f)
opwvhk pushed a commit to opwvhk/avro that referenced this pull request Sep 5, 2025
…to introduce org.apache.avro.SERIALIZABLE_CLASSES instead (apache#3376)
opwvhk pushed a commit to opwvhk/avro that referenced this pull request Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Java Pull Requests for Java binding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants