diff mbox series

[RFC,v3,4/4] ceph: replace base64url by the encoding used for mailbox names

Message ID 20220317154521.6615-5-lhenriques@suse.de (mailing list archive)
State New, archived
Headers show
Series ceph: add support for snapshot names encryption | expand

Commit Message

Luis Henriques March 17, 2022, 3:45 p.m. UTC
The base64url encoding includes the '_' character, which may cause problems
in snapshot names (if the name starts with '_').  Thus, use the base64
encoding defined for IMAP mailbox names (RFC 3501), which uses '+' and ','
instead of '-' and '_'.

Signed-off-by: Luís Henriques <lhenriques@suse.de>
---
 fs/ceph/crypto.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++--
 fs/ceph/crypto.h |  3 +++
 fs/ceph/dir.c    |  2 +-
 fs/ceph/inode.c  |  2 +-
 4 files changed, 54 insertions(+), 4 deletions(-)

Comments

Xiubo Li March 18, 2022, 8:32 a.m. UTC | #1
On 3/17/22 11:45 PM, Luís Henriques wrote:
> The base64url encoding includes the '_' character, which may cause problems
> in snapshot names (if the name starts with '_').  Thus, use the base64
> encoding defined for IMAP mailbox names (RFC 3501), which uses '+' and ','
> instead of '-' and '_'.
>
> Signed-off-by: Luís Henriques <lhenriques@suse.de>
> ---
>   fs/ceph/crypto.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++--
>   fs/ceph/crypto.h |  3 +++
>   fs/ceph/dir.c    |  2 +-
>   fs/ceph/inode.c  |  2 +-
>   4 files changed, 54 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
> index caa9863dee93..d6f1c444ce91 100644
> --- a/fs/ceph/crypto.c
> +++ b/fs/ceph/crypto.c
> @@ -7,6 +7,53 @@
>   #include "mds_client.h"
>   #include "crypto.h"
>   
> +static const char base64_table[65] =
> +        "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+,";
> +
> +int ceph_base64_encode(const u8 *src, int srclen, char *dst)
> +{
> +	u32 ac = 0;
> +	int bits = 0;
> +	int i;
> +	char *cp = dst;
> +
> +	for (i = 0; i < srclen; i++) {
> +		ac = (ac << 8) | src[i];
> +		bits += 8;
> +		do {
> +			bits -= 6;
> +			*cp++ = base64_table[(ac >> bits) & 0x3f];
> +		} while (bits >= 6);
> +	}
> +	if (bits)
> +		*cp++ = base64_table[(ac << (6 - bits)) & 0x3f];
> +	return cp - dst;
> +}
> +
> +int ceph_base64_decode(const char *src, int srclen, u8 *dst)
> +{
> +	u32 ac = 0;
> +	int bits = 0;
> +	int i;
> +	u8 *bp = dst;
> +
> +	for (i = 0; i < srclen; i++) {
> +		const char *p = strchr(base64_table, src[i]);
> +
> +		if (p == NULL || src[i] == 0)
> +			return -1;
> +		ac = (ac << 6) | (p - base64_table);
> +		bits += 6;
> +		if (bits >= 8) {
> +			bits -= 8;
> +			*bp++ = (u8)(ac >> bits);
> +		}
> +	}
> +	if (ac & ((1 << bits) - 1))
> +		return -1;
> +	return bp - dst;
> +}

Maybe this should be in fs/crypto.c ?

-- Xiubo

> +
>   static int ceph_crypt_get_context(struct inode *inode, void *ctx, size_t len)
>   {
>   	struct ceph_inode_info *ci = ceph_inode(inode);
> @@ -260,7 +307,7 @@ int ceph_encode_encrypted_dname(struct inode *parent, struct qstr *d_name, char
>   	}
>   
>   	/* base64 encode the encrypted name */
> -	elen = fscrypt_base64url_encode(cryptbuf, len, buf);
> +	elen = ceph_base64_encode(cryptbuf, len, buf);
>   	dout("base64-encoded ciphertext name = %.*s\n", elen, buf);
>   
>   	WARN_ON(elen > (CEPH_NOHASH_NAME_MAX + SHA256_DIGEST_SIZE));
> @@ -365,7 +412,7 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
>   			tname = &_tname;
>   		}
>   
> -		declen = fscrypt_base64url_decode(name, name_len, tname->name);
> +		declen = ceph_base64_decode(name, name_len, tname->name);
>   		if (declen <= 0) {
>   			ret = -EIO;
>   			goto out;
> diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
> index 3273d076a9e5..d22316011810 100644
> --- a/fs/ceph/crypto.h
> +++ b/fs/ceph/crypto.h
> @@ -93,6 +93,9 @@ static inline u32 ceph_fscrypt_auth_len(struct ceph_fscrypt_auth *fa)
>    */
>   #define CEPH_NOHASH_NAME_MAX (180 - SHA256_DIGEST_SIZE)
>   
> +int ceph_base64_encode(const u8 *src, int srclen, char *dst);
> +int ceph_base64_decode(const char *src, int srclen, u8 *dst);
> +
>   void ceph_fscrypt_set_ops(struct super_block *sb);
>   
>   void ceph_fscrypt_free_dummy_policy(struct ceph_fs_client *fsc);
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index 5ae5cb778389..417d8c3a7edd 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -960,7 +960,7 @@ static int prep_encrypted_symlink_target(struct ceph_mds_request *req, const cha
>   		goto out;
>   	}
>   
> -	len = fscrypt_base64url_encode(osd_link.name, osd_link.len, req->r_path2);
> +	len = ceph_base64_encode(osd_link.name, osd_link.len, req->r_path2);
>   	req->r_path2[len] = '\0';
>   out:
>   	fscrypt_fname_free_buffer(&osd_link);
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 359e29896f16..8fd493257e0b 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -875,7 +875,7 @@ static int decode_encrypted_symlink(const char *encsym, int enclen, u8 **decsym)
>   	if (!sym)
>   		return -ENOMEM;
>   
> -	declen = fscrypt_base64url_decode(encsym, enclen, sym);
> +	declen = ceph_base64_decode(encsym, enclen, sym);
>   	if (declen < 0) {
>   		pr_err("%s: can't decode symlink (%d). Content: %.*s\n", __func__, declen, enclen, encsym);
>   		kfree(sym);
>
Luis Henriques March 18, 2022, 11:01 a.m. UTC | #2
Xiubo Li <xiubli@redhat.com> writes:

> On 3/17/22 11:45 PM, Luís Henriques wrote:
>> The base64url encoding includes the '_' character, which may cause problems
>> in snapshot names (if the name starts with '_').  Thus, use the base64
>> encoding defined for IMAP mailbox names (RFC 3501), which uses '+' and ','
>> instead of '-' and '_'.
>>
>> Signed-off-by: Luís Henriques <lhenriques@suse.de>
>> ---
>>   fs/ceph/crypto.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++--
>>   fs/ceph/crypto.h |  3 +++
>>   fs/ceph/dir.c    |  2 +-
>>   fs/ceph/inode.c  |  2 +-
>>   4 files changed, 54 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
>> index caa9863dee93..d6f1c444ce91 100644
>> --- a/fs/ceph/crypto.c
>> +++ b/fs/ceph/crypto.c
>> @@ -7,6 +7,53 @@
>>   #include "mds_client.h"
>>   #include "crypto.h"
>>   +static const char base64_table[65] =
>> +        "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+,";
>> +
>> +int ceph_base64_encode(const u8 *src, int srclen, char *dst)
>> +{
>> +	u32 ac = 0;
>> +	int bits = 0;
>> +	int i;
>> +	char *cp = dst;
>> +
>> +	for (i = 0; i < srclen; i++) {
>> +		ac = (ac << 8) | src[i];
>> +		bits += 8;
>> +		do {
>> +			bits -= 6;
>> +			*cp++ = base64_table[(ac >> bits) & 0x3f];
>> +		} while (bits >= 6);
>> +	}
>> +	if (bits)
>> +		*cp++ = base64_table[(ac << (6 - bits)) & 0x3f];
>> +	return cp - dst;
>> +}
>> +
>> +int ceph_base64_decode(const char *src, int srclen, u8 *dst)
>> +{
>> +	u32 ac = 0;
>> +	int bits = 0;
>> +	int i;
>> +	u8 *bp = dst;
>> +
>> +	for (i = 0; i < srclen; i++) {
>> +		const char *p = strchr(base64_table, src[i]);
>> +
>> +		if (p == NULL || src[i] == 0)
>> +			return -1;
>> +		ac = (ac << 6) | (p - base64_table);
>> +		bits += 6;
>> +		if (bits >= 8) {
>> +			bits -= 8;
>> +			*bp++ = (u8)(ac >> bits);
>> +		}
>> +	}
>> +	if (ac & ((1 << bits) - 1))
>> +		return -1;
>> +	return bp - dst;
>> +}
>
> Maybe this should be in fs/crypto.c ?

Yeah, if the solution is to modify the base64 encoding, that's my
preference too (in the series cover-letter this would correspond to
alternative #3).

[ In fact, I've probably done something very wrong here, as I didn't even
  mentioned that this patch is basically a copy of someone else's code; I
  simply modified the table used.  So, please do *not* consider merging
  this patch. ]

Cheers,
diff mbox series

Patch

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index caa9863dee93..d6f1c444ce91 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -7,6 +7,53 @@ 
 #include "mds_client.h"
 #include "crypto.h"
 
+static const char base64_table[65] =
+        "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+,";
+
+int ceph_base64_encode(const u8 *src, int srclen, char *dst)
+{
+	u32 ac = 0;
+	int bits = 0;
+	int i;
+	char *cp = dst;
+
+	for (i = 0; i < srclen; i++) {
+		ac = (ac << 8) | src[i];
+		bits += 8;
+		do {
+			bits -= 6;
+			*cp++ = base64_table[(ac >> bits) & 0x3f];
+		} while (bits >= 6);
+	}
+	if (bits)
+		*cp++ = base64_table[(ac << (6 - bits)) & 0x3f];
+	return cp - dst;
+}
+
+int ceph_base64_decode(const char *src, int srclen, u8 *dst)
+{
+	u32 ac = 0;
+	int bits = 0;
+	int i;
+	u8 *bp = dst;
+
+	for (i = 0; i < srclen; i++) {
+		const char *p = strchr(base64_table, src[i]);
+
+		if (p == NULL || src[i] == 0)
+			return -1;
+		ac = (ac << 6) | (p - base64_table);
+		bits += 6;
+		if (bits >= 8) {
+			bits -= 8;
+			*bp++ = (u8)(ac >> bits);
+		}
+	}
+	if (ac & ((1 << bits) - 1))
+		return -1;
+	return bp - dst;
+}
+
 static int ceph_crypt_get_context(struct inode *inode, void *ctx, size_t len)
 {
 	struct ceph_inode_info *ci = ceph_inode(inode);
@@ -260,7 +307,7 @@  int ceph_encode_encrypted_dname(struct inode *parent, struct qstr *d_name, char
 	}
 
 	/* base64 encode the encrypted name */
-	elen = fscrypt_base64url_encode(cryptbuf, len, buf);
+	elen = ceph_base64_encode(cryptbuf, len, buf);
 	dout("base64-encoded ciphertext name = %.*s\n", elen, buf);
 
 	WARN_ON(elen > (CEPH_NOHASH_NAME_MAX + SHA256_DIGEST_SIZE));
@@ -365,7 +412,7 @@  int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
 			tname = &_tname;
 		}
 
-		declen = fscrypt_base64url_decode(name, name_len, tname->name);
+		declen = ceph_base64_decode(name, name_len, tname->name);
 		if (declen <= 0) {
 			ret = -EIO;
 			goto out;
diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
index 3273d076a9e5..d22316011810 100644
--- a/fs/ceph/crypto.h
+++ b/fs/ceph/crypto.h
@@ -93,6 +93,9 @@  static inline u32 ceph_fscrypt_auth_len(struct ceph_fscrypt_auth *fa)
  */
 #define CEPH_NOHASH_NAME_MAX (180 - SHA256_DIGEST_SIZE)
 
+int ceph_base64_encode(const u8 *src, int srclen, char *dst);
+int ceph_base64_decode(const char *src, int srclen, u8 *dst);
+
 void ceph_fscrypt_set_ops(struct super_block *sb);
 
 void ceph_fscrypt_free_dummy_policy(struct ceph_fs_client *fsc);
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 5ae5cb778389..417d8c3a7edd 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -960,7 +960,7 @@  static int prep_encrypted_symlink_target(struct ceph_mds_request *req, const cha
 		goto out;
 	}
 
-	len = fscrypt_base64url_encode(osd_link.name, osd_link.len, req->r_path2);
+	len = ceph_base64_encode(osd_link.name, osd_link.len, req->r_path2);
 	req->r_path2[len] = '\0';
 out:
 	fscrypt_fname_free_buffer(&osd_link);
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 359e29896f16..8fd493257e0b 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -875,7 +875,7 @@  static int decode_encrypted_symlink(const char *encsym, int enclen, u8 **decsym)
 	if (!sym)
 		return -ENOMEM;
 
-	declen = fscrypt_base64url_decode(encsym, enclen, sym);
+	declen = ceph_base64_decode(encsym, enclen, sym);
 	if (declen < 0) {
 		pr_err("%s: can't decode symlink (%d). Content: %.*s\n", __func__, declen, enclen, encsym);
 		kfree(sym);