commons-codec
commons-codec copied to clipboard
Base64 should be open for different alphabets
Currently I´m trying to write a Linux shadow-file - the passwords are stored as Base64-ecoded (hashes). But the alphabet of this Base64 is different:
see www.linuxquestions.org/questions/linux-security-4/how-can-i-convert-a-sha-512-etc-shadow-hash-to-base64-4175477045/
A base 64 encoding is used to store password hashes computed with crypt in the /etc/passwd. Its alphabet starts with '.' for zero, then '/' for one, followed by 0-9, A-Z and a-z. Padding is not used.
So I opened up your Base64 class constructor to accept an alphabet.
Coverage decreased (-0.04%) to 92.74% when pulling c44f596cdd942ec73011581b708d752c54fa1ec8 on cljk:master into 47a55d21515bf2ec49d2c1b4f2f83bf66a09a7c5 on apache:master.
Coverage increased (+0.02%) to 92.803% when pulling 3e0d13c035021da7921cae9702194bb0699f5071 on cljk:master into 47a55d21515bf2ec49d2c1b4f2f83bf66a09a7c5 on apache:master.
You´re right. I added one unit test to check the encoding output of using a custom alphabet by comparing it to the default alphabet output.
I made the byte arrays of the alphabets public
. Not just because I needed it for the unit test but I think that the encoding table might be necessary elsewhere in other projects?!
If this is not wanted I´ll copy the array to the test-case...
When my personal project with the shadow table is finished I´d like to add the crypt table.
Hm.... I´ve no idea why the travis build fails.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 58.236 s
[INFO] Finished at: 2019-07-23T20:05:09Z
[INFO] Final Memory: 47M/160M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.1.0:javadoc (default-cli) on project commons-codec: An error has occurred in Javadoc report generation:
[ERROR] Exit code: 1 - javadoc: error - The code being documented uses modules but the packages defined in https://docs.oracle.com/javase/7/docs/api/ are in the unnamed module.
[ERROR] javadoc: error - The code being documented uses modules but the packages defined in https://docs.oracle.com/javaee/6/api/ are in the unnamed module.
[ERROR] javadoc: error - The code being documented uses modules but the packages defined in https://docs.oracle.com/javase/7/docs/api/ are in the unnamed module.
[ERROR]
[ERROR] Command line was: /home/travis/oraclejdk11/bin/javadoc @options @packages
[ERROR]
[ERROR] Refer to the generated Javadoc files in '/home/travis/build/apache/commons-codec/target/site/apidocs' dir.
[ERROR]
I work on Corretto JDK 11 and my maven build succeeds?!
mvn clean package site
works on my system.
Okay... can reproduce the error - but it´s no fault in my changes. Same error occurs in original code.
I think the build fails because the maven-javadoc-plugin configuration section in build does not have this:
<source>${maven.compiler.source}</source>
The plugin is configured in commons-parent. It has this tag for the reporting section and the release profile but not for the build section.
@cljk Can you confirm that you cannot do this with Corretto JDK 11:
mvn javadoc:javadoc
or default goal
mvn
This should work:
mvn javadoc:javadoc -Dsource=1.7
On the code change, do not make the array public. The values are mutable. Copy the array to the unit test or provide a package private level static getter which returns a clone of the array.
You should also change the public constructor that accepts an input alphabet to check it has 64 characters. I'm not sure where we are with defensive copying of input arrays so I'll leave that judgement for someone else.
@aherbert Yes, I can confirm:
mvn javadoc:javadoc
fails, mvn
too and
mvn javadoc:javadoc -Dsource=1.7
works...
But perhaps we should open an issue for that since that´s not caused by my pull request?!
- added check for encodeTable length ... and a unit test for this check
- encode tables
private
again - reintruduced(?) usage of local variable
decodeTable
- needed for successful decoding of data encoded with custom encoding table- for
STANDARD_ENCODE_TABLE
orURL_SAFE_ENCODE_TABLE
it uses the precalculatedDEFAULT_DECODE_TABLE
as before - for custom encode tables: added a calculation method for decode tables ... and an encode/decode test for a modified encode table
- for
" defensive copying of input arrays" - If you think making a duplicate of the encodeTable improves security in any way (?)... adding this would be a one-liner?!