Skip to content

Commit 1ad8392

Browse files
jwarlanderemil
authored and
emil
committed
Remove NUL padding when reading secrets from stdin (#76)
* 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
1 parent f251a8f commit 1ad8392

File tree

3 files changed

+45
-10
lines changed

3 files changed

+45
-10
lines changed

cli/src/main/java/com/schibsted/security/strongbox/cli/viewmodel/SecretModel.java

+1-10
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ private byte[] fromStdin() {
144144
if (secretValue == null) {
145145
throw new IllegalArgumentException("A secret value must be specified");
146146
}
147-
return asBytes(secretValue);
147+
return SecretValueConverter.asBytes(secretValue);
148148
} else {
149149
// Piped in
150150
return IOUtils.toByteArray(inputStream);
@@ -164,15 +164,6 @@ private static byte[] extractValueFromFile(String valueFile) {
164164
}
165165
}
166166

167-
private byte[] asBytes(char[] chars) {
168-
CharBuffer charBuffer = CharBuffer.wrap(chars);
169-
ByteBuffer byteBuffer = Charset.forName("UTF-8").encode(charBuffer);
170-
171-
BestEffortShredder.shred(chars);
172-
173-
return byteBuffer.array();
174-
}
175-
176167
private static int booleanIfExists(String value) {
177168
return (value != null) ? 1 : 0;
178169
}

sdk/src/main/java/com/schibsted/security/strongbox/sdk/internal/converter/SecretValueConverter.java

+14
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88
import com.schibsted.security.strongbox.sdk.types.SecretType;
99
import com.schibsted.security.strongbox.sdk.types.SecretValue;
1010

11+
import java.nio.ByteBuffer;
12+
import java.nio.CharBuffer;
13+
import java.nio.charset.Charset;
1114
import java.util.Arrays;
1215

1316
/**
@@ -28,4 +31,15 @@ public static SecretValue inferEncoding(byte[] value, SecretType secretType) {
2831
return new SecretValue(value, secretType);
2932
}
3033
}
34+
35+
public static byte[] asBytes(char[] chars) {
36+
CharBuffer charBuffer = CharBuffer.wrap(chars);
37+
ByteBuffer byteBuffer = Charset.forName("UTF-8").encode(charBuffer);
38+
byte[] bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit());
39+
40+
BestEffortShredder.shred(charBuffer.array());
41+
BestEffortShredder.shred(byteBuffer.array());
42+
43+
return bytes;
44+
}
3145
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*
2+
* Copyright (c) 2018 Schibsted Products & Technology AS. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
3+
*/
4+
5+
package com.schibsted.security.strongbox.sdk;
6+
7+
import com.schibsted.security.strongbox.sdk.internal.converter.SecretValueConverter;
8+
import org.testng.annotations.Test;
9+
10+
import java.nio.charset.Charset;
11+
12+
import static org.hamcrest.MatcherAssert.assertThat;
13+
import static org.hamcrest.core.Is.is;
14+
15+
/**
16+
* @author jwarlander
17+
*/
18+
public class SecretValueConverterTest {
19+
@Test
20+
public void chars_to_bytes() {
21+
String str = "beeboopfoobarblahblahthisisalongstringyeah";
22+
char[] charsFromString = str.toCharArray();
23+
byte[] bytesFromString = str.getBytes(Charset.forName("UTF-8"));
24+
assertThat(SecretValueConverter.asBytes(charsFromString), is(bytesFromString));
25+
26+
// Our initial char array above should be shredded now; eg. only nulls
27+
char[] emptyCharArray = new char[bytesFromString.length];
28+
assertThat(charsFromString, is(emptyCharArray));
29+
}
30+
}

0 commit comments

Comments
 (0)