Skip to content

Remove the default serializable packages and deprecated the property to introduce org.apache.avro.SERIALIZABLE_CLASSES instead#3376

Merged
nandorKollar merged 1 commit intoapache:mainfrom
jbonofre:improve-security-check
May 19, 2025
Merged

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

jbonofre commented May 11, 2025

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

nandorKollar
nandorKollar approved these changes May 11, 2025
View reviewed changes
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!

martin-g
martin-g approved these changes May 12, 2025
View reviewed changes
@jbonofre
Remove the default serializable packages and deprecated the property …
e9bfef8 
…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

jbonofre commented May 14, 2025

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

github-advanced-security[bot]
github-advanced-security bot found potential problems May 14, 2025
View reviewed changes
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java
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
Loading
should be avoided because it has been deprecated.
@nandorKollar
Copy link
Contributor

nandorKollar commented May 15, 2025

@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!

Fokko
Fokko approved these changes May 18, 2025
View reviewed changes
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

nandorKollar commented May 19, 2025

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
@nkollar
fix broken build after merging apache#3376
e3a18f9
@nandorKollar nandorKollar mentioned this pull request May 19, 2025
Merged
nandorKollar added a commit to nandorKollar/avro that referenced this pull request May 19, 2025
@nandorKollar
fix broken tests after merging apache#3376
a693c41
@jbonofre
Copy link
Member Author

jbonofre commented May 20, 2025

@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
@nandorKollar
fix broken tests after merging #3376 (#3383)
ac09c2c
opwvhk pushed a commit to opwvhk/avro that referenced this pull request Aug 10, 2025
@jbonofre @opwvhk
Remove the default serializable packages and deprecated the property …
4682541 
…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
@jbonofre @opwvhk
Remove the default serializable packages and deprecated the property …
35389bd 
…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
@jbonofre @opwvhk
Remove the default serializable packages and deprecated the property …
a5bf3fc 
…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
@nandorKollar @opwvhk
fix broken tests after merging apache#3376 (apache#3383)
90b48b8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@martin-g martin-g martin-g approved these changes

@Fokko Fokko Fokko approved these changes

@nandorKollar nandorKollar nandorKollar approved these changes

Assignees

No one assigned

Labels

build Java Pull Requests for Java binding

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jbonofre @nandorKollar @Fokko @martin-g