On Tue, Nov 24, 2015 at 11:29:55AM -0700, Eric Blake wrote:
> > +static void
> > +qcrypto_secret_load_data(QCryptoSecret *secret,
> > + uint8_t **output,
> > + size_t *outputlen,
> > + Error **errp)
> > +{
> > + int fd;
> > + char *data = NULL;
> > + size_t offset = 0;
> > + size_t length = 0;
> > +
> > + *output = NULL;
> > + *outputlen = 0;
> > +
> > + if (secret->file) {
> > + if (secret->data) {
> > + error_setg(errp,
> > + "'file' and 'data' are mutually exclusive");
>
> Is it worth trying to use a qapi flat union to make the mutual exclusion
> inherent in the type definition, rather than something we have to
> enforce manually? (I've got more experience with qapi than with Object,
> so my question may be nonsensical)
Not ensure sure how you'd wire up a qapi type to a QOM property. It
is probably possible in some manner, but I not sure its particularly
compelling to do it for this.
>
> > + return;
> > + }
> > + fd = qemu_open(secret->file, O_RDONLY);
> > + if (fd < 0) {
> > + error_setg_errno(errp, errno,
> > + "Unable to open %s", secret->file);
>
> Using error_setg_file_open() makes for a consistent message on open()
> failure.
Yep
>
> > + return;
> > + }
> > + while (length < (1024 * 1024)) { /* Limit secrets to 1 MB */
> > + if ((length - offset) < 1024) {
> > + length += 1024;
> > + data = g_renew(char, data, length);
> > + }
> > + ssize_t ret = read(fd, data + offset, length - offset);
> > + if (ret == 0) {
> > + break;
> > + }
> > + if (ret < 0) {
> > + error_setg_errno(errp, errno,
> > + "Unable to read from %s", secret->file);
>
> Does glib have a convenience function for reading contents of a file?
Of course, I completely forgot about g_file_get_contents() which
works just fine.
> > +static void qcrypto_secret_decrypt(QCryptoSecret *secret,
> > + const uint8_t *input,
> > + size_t inputlen,
> > + uint8_t **output,
> > + size_t *outputlen,
> > + Error **errp)
> > +{
> > + uint8_t *key = NULL, *ciphertext = NULL, *iv = NULL;
> > + size_t keylen, ciphertextlen, ivlen;
> > + QCryptoCipher *aes = NULL;
> > + uint8_t *plaintext = NULL;
> > +
> > + *output = NULL;
> > + *outputlen = 0;
> > +
> > + if (qcrypto_secret_lookup(secret->keyid,
> > + &key, &keylen,
> > + errp) < 0) {
> > + goto cleanup;
> > + }
> > +
> > + if (keylen != 32) {
> > + error_setg(errp, "Key should be 32 bytes in length");
> > + goto cleanup;
> > + }
> > +
> > + if (!secret->iv) {
> > + error_setg(errp, "IV is required to decrypt secret");
> > + goto cleanup;
> > + }
> > +
> > + iv = (uint8_t *)g_base64_decode(secret->iv, &ivlen);
>
> Shouldn't this be using qbase64_decode()?
Yeah, it really should
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
