Skip to content

AVRO-3985: Add trusted packages support in SpecificData - #2934

Merged
Fokko merged 4 commits into
apache:mainfrom
jbonofre:AVRO-3985
Jun 24, 2024
Merged

AVRO-3985: Add trusted packages support in SpecificData#2934
Fokko merged 4 commits into
apache:mainfrom
jbonofre:AVRO-3985

Conversation

@jbonofre

@jbonofrejbonofre commented Jun 1, 2024•
edited
Loading

Copy link
Copy Markdown
Member

What is the purpose of the change

This change introduces the org.apache.avro.SERIALIZABLE_PACKAGES system property to enforce the security aspect of using java-class in a schema.

Verifying this change

This change is already covered by existing tests, using the default trusted packages. I can add an additional test specifically for non trusted packages.

Documentation

  • Does this pull request introduce a new feature? yes
  • If yes, how is the feature documented? The error message already explains how to use org.apache.avro.SERIALIZABLE_PACKAGES system property. Happy to add additional documentation if needed.

@github-actionsgithub-actionsBot added the Java Pull Requests for Java binding label Jun 1, 2024
@jbonofre
jbonofre requested a review from FokkoJune 1, 2024 07:29
Fokko
Fokko approved these changes Jun 2, 2024
View reviewed changes

@FokkoFokko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @jbonofre

Comment threadlang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java Outdated
martin-g
martin-g reviewed Jun 3, 2024
View reviewed changes
Comment threadlang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java Outdated
Comment threadlang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectData.java Outdated
@jbonofre
jbonofreforce-pushed the AVRO-3985 branch 4 times, most recently from 78142bb to 9991b78CompareJune 4, 2024 09:39
KalleOlaviNiemitalo
KalleOlaviNiemitalo reviewed Jun 4, 2024
View reviewed changes
Comment threadlang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java Outdated
@jbonofre

jbonofre commented Jun 13, 2024

Copy link
Copy Markdown
MemberAuthor

@Fokko@martin-g@KalleOlaviNiemitalo I updated the PR. Can you guys please take a look ? Thanks !

martin-g
martin-g reviewed Jun 14, 2024
View reviewed changes
Comment threadlang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java Outdated
Comment threadlang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java Outdated
Comment threadlang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java Outdated
Comment threadlang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java
KalleOlaviNiemitalo
KalleOlaviNiemitalo reviewed Jun 14, 2024
View reviewed changes
Comment threadlang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java Outdated
Fokko
Fokko reviewed Jun 19, 2024
View reviewed changes
Comment threadlang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java
Fokkoand others added 3 commits June 19, 2024 18:21
@Fokko@martin-g
Apply suggestions from code review
27b7afb
Co-authored-by: Martin Grigorov <martin-g@users.noreply.github.com>
@Fokko
Move to SecurityException
68df6e1
@Fokko
Remove redundant import
77f25ed
@Fokko
Fokko merged commit f6b3bd7 into apache:mainJun 24, 2024
@Fokko

Fokko commented Jun 24, 2024

Copy link
Copy Markdown
Contributor

Moving this forward, thanks @jbonofre for fixing this 🙌 and thanks @martin-g and @KalleOlaviNiemitalo for the reviews!

@jbonofre
jbonofre deleted the AVRO-3985 branch June 26, 2024 07:53
@jbonofre

jbonofre commented Jun 26, 2024

Copy link
Copy Markdown
MemberAuthor

I'm creating the 1.11.x backport PR.

@github-actionsgithub-actionsBot mentioned this pull request Oct 9, 2024
Closed
MichalFoksa
MichalFoksa reviewed Jan 6, 2025
View reviewed changes
Comment threadlang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java

boolean found = false;
Package thePackage = clazz.getPackage();
if (thePackage != null) {

@MichalFoksaMichalFoksaJan 6, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Fokko or @martin-g
When a class is not in any package clazz.getPackage() returns null and this condition renders that class trusty.

Move if (!found) throw new SecurityException out of the loop.
Move if (!found) throw new SecurityException behind if (thePackage != null) condition - or something :).

@jbonofrejbonofreJan 6, 2025

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I can take a look on this too as I'm the original author

@martin-gmartin-gFeb 10, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@MichalFoksa Would you like to send a Pull Request?

@jbonofrejbonofreFeb 10, 2025

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Else I can work on a PR tomorrow

@MichalFoksaMichalFoksaFeb 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@martin-g Here you are PR #3311

@wgtmacwgtmac mentioned this pull request Mar 5, 2025
Merged
StitchMl pushed a commit to StitchMl/avro that referenced this pull request Jul 13, 2026
@jbonofre@Fokko@martin-g
AVRO-3985: Add trusted packages support in SpecificData (apache#2934)
9b90663
* AVRO-3985: Add trusted packages support in SpecificData
* Apply suggestions from code review
Co-authored-by: Martin Grigorov <martin-g@users.noreply.github.com>
* Move to SecurityException
* Remove redundant import
---------
Co-authored-by: Fokko Driesprong <fokko@apache.org>
Co-authored-by: Martin Grigorov <martin-g@users.noreply.github.com>
StitchMl pushed a commit to StitchMl/avro that referenced this pull request Jul 13, 2026
@jbonofre@Fokko@martin-g
AVRO-3985: Add trusted packages support in SpecificData (apache#2934)
9eeca74
* AVRO-3985: Add trusted packages support in SpecificData
* Apply suggestions from code review
Co-authored-by: Martin Grigorov <martin-g@users.noreply.github.com>
* Move to SecurityException
* Remove redundant import
---------
Co-authored-by: Fokko Driesprong <fokko@apache.org>
Co-authored-by: Martin Grigorov <martin-g@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@martin-gmartin-gmartin-g approved these changes
@FokkoFokkoFokko approved these changes
@KalleOlaviNiemitaloKalleOlaviNiemitaloAwaiting requested review from KalleOlaviNiemitalo
+1 more reviewer
@MichalFoksaMichalFoksaMichalFoksa left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

JavaPull Requests for Java binding

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jbonofre@Fokko@martin-g@MichalFoksa@KalleOlaviNiemitalo