Skip to content

Commit

Permalink
Remove NUL padding when reading secrets from stdin (#76)
Browse files Browse the repository at this point in the history
* Respect the limit when converting a ByteBuffer to an array

* Explicitly make use of 'position' + clean up intermediate storage

* Don't clear wrapped char array twice

* Move 'asBytes()' to 'SecretValueConverter' + add tests
  • Loading branch information
jwarlander authored and emilva committed Mar 19, 2018
1 parent f251a8f commit 1ad8392
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 10 deletions.
Expand Up @@ -144,7 +144,7 @@ private byte[] fromStdin() {
if (secretValue == null) {
throw new IllegalArgumentException("A secret value must be specified");
}
return asBytes(secretValue);
return SecretValueConverter.asBytes(secretValue);
} else {
// Piped in
return IOUtils.toByteArray(inputStream);
Expand All @@ -164,15 +164,6 @@ private static byte[] extractValueFromFile(String valueFile) {
}
}

private byte[] asBytes(char[] chars) {
CharBuffer charBuffer = CharBuffer.wrap(chars);
ByteBuffer byteBuffer = Charset.forName("UTF-8").encode(charBuffer);

BestEffortShredder.shred(chars);

return byteBuffer.array();
}

private static int booleanIfExists(String value) {
return (value != null) ? 1 : 0;
}
Expand Down
Expand Up @@ -8,6 +8,9 @@
import com.schibsted.security.strongbox.sdk.types.SecretType;
import com.schibsted.security.strongbox.sdk.types.SecretValue;

import java.nio.ByteBuffer;
import java.nio.CharBuffer;
import java.nio.charset.Charset;
import java.util.Arrays;

/**
Expand All @@ -28,4 +31,15 @@ public static SecretValue inferEncoding(byte[] value, SecretType secretType) {
return new SecretValue(value, secretType);
}
}

public static byte[] asBytes(char[] chars) {
CharBuffer charBuffer = CharBuffer.wrap(chars);
ByteBuffer byteBuffer = Charset.forName("UTF-8").encode(charBuffer);
byte[] bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit());

BestEffortShredder.shred(charBuffer.array());
BestEffortShredder.shred(byteBuffer.array());

return bytes;
}
}
@@ -0,0 +1,30 @@
/*
* Copyright (c) 2018 Schibsted Products & Technology AS. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
*/

package com.schibsted.security.strongbox.sdk;

import com.schibsted.security.strongbox.sdk.internal.converter.SecretValueConverter;
import org.testng.annotations.Test;

import java.nio.charset.Charset;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.Is.is;

/**
* @author jwarlander
*/
public class SecretValueConverterTest {
@Test
public void chars_to_bytes() {
String str = "beeboopfoobarblahblahthisisalongstringyeah";
char[] charsFromString = str.toCharArray();
byte[] bytesFromString = str.getBytes(Charset.forName("UTF-8"));
assertThat(SecretValueConverter.asBytes(charsFromString), is(bytesFromString));

// Our initial char array above should be shredded now; eg. only nulls
char[] emptyCharArray = new char[bytesFromString.length];
assertThat(charsFromString, is(emptyCharArray));
}
}

0 comments on commit 1ad8392

Please sign in to comment.