Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UserPasswordDataCodec.encode shortens Password to 16 bytes instead of padding to multiple. #12

Closed
ramberg opened this issue Sep 14, 2023 · 1 comment

Comments

@ramberg
Copy link

ramberg commented Sep 14, 2023

Hi Thomas,

I think your encode method for the password org.aaa4j.radius.core.attribute.UserPasswordDataCodec#encode makes a small math mistake. Your line is

Math.min(16, password.length + ((16 - password.length) % 16))

This line will always return 16. I think the min might be meant as a max (then it would work in Python), and then Java does not deal with negative values and modulo the same, as e.g. Python.

//Java
(16 - 19) % 16 = -3
#Python
>>> (16 - 19) % 16
13
>>> min(16,19 + ((16 - 19) % 16))
16
>>> max(16,19 + ((16 - 19) % 16))
32

In Java I think you need:

byte[] paddedPassword = new byte[password.length - ((password.length - 1) % 16) + 15];

Again, if I find the time, I will make this part of my contribution.

For completeness, the exception when using a 19 byte password was:

java.lang.ArrayIndexOutOfBoundsException: arraycopy: last destination index 19 out of bounds for byte[16]

	at java.base/java.lang.System.arraycopy(Native Method)
	at org.aaa4j.radius.core.attribute.UserPasswordDataCodec.encode(UserPasswordDataCodec.java:88)
	at org.aaa4j.radius.core.attribute.StandardAttribute$Codec.encode(StandardAttribute.java:91)
	at org.aaa4j.radius.core.packet.PacketCodec.encodeAttributes(PacketCodec.java:519)
	at org.aaa4j.radius.core.packet.PacketCodec.encodeRequest(PacketCodec.java:123)
	at org.aaa4j.radius.client.clients.UdpRadiusClient.send(UdpRadiusClient.java:104)
	at de.biotronik.pet.tools.crm.server.backend.RadiusClientService.authenticate(RadiusClientService.java:110)
	at RadiusIntegrationTest.testRadiusServer(RadiusIntegrationTest.java:50)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater$1.execute(IdeaTestRunner.java:38)
	at com.intellij.rt.execution.junit.TestsRepeater.repeat(TestsRepeater.java:11)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:35)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:235)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)

King Regards.

@tsyd
Copy link
Member

tsyd commented Aug 21, 2024

Hello Ronny,

Thank you for pointing out this issue with the User-Password encoding. This was fixed in v0.2.4.

@tsyd tsyd closed this as completed Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants